Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better support for packages with files outside site-packages #482

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dan-blanchard
Copy link

This should fix #455. I slightly modified the patch suggested by @jaraco in #455 (comment) so that we don't have to call self.locate_file('').resolve() for name in text.splitlines(), and fixed one type complaint I was getting locally.

I'm not sure if the test I added works properly (which is one of the reasons I made this PR), because I'm running into a weird tox issue locally.

@dan-blanchard dan-blanchard changed the title Add support for egg packages with files outside site-packages Add better support for packages with files outside site-packages Jan 10, 2024
@dan-blanchard
Copy link
Author

The "weird tox issue" turns out to be an issue with pytest-perf, I think.

I keep getting errors like:

E           subprocess.CalledProcessError: Command '('/opt/homebrew/Caskroom/mambaforge/base/envs/py312importlib/bin/python3.12', '-m', 'pip', 'install', '-t', PosixPath('/var/folders/vz/khc_rx815vv9rhbclb1bl2hc0000gn/T/pip-run-x6yeau2d'), 'importlib_metadata.git[perf]@git+git@github.com:dan-blanchard/importlib_metadata.git')' returned non-zero exit status 1.

/opt/homebrew/Caskroom/mambaforge/base/envs/py312importlib/lib/python3.12/subprocess.py:413: CalledProcessError
---------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------
ERROR: Invalid requirement: 'importlib_metadata.git[perf]@git+git@github.com:dan-blanchard/importlib_metadata.git'
Hint: It looks like a path. File 'importlib_metadata.git[perf]@git+git@github.com:dan-blanchard/importlib_metadata.git' does not exist.

when trying to run tests locally.

@jaraco
Copy link
Member

jaraco commented Mar 7, 2024

Invalid requirement: 'importlib_metadata.git[perf]@git+git@github.com:dan-blanchard/importlib_metadata.git'

You may want to report this issue to pytest-perf. I notice that importlib_metadata.git[perf] should be importlib_metadata[perf], and probably what's happening is the plugin is inferring the package name from the upstream URL, but since you're using the SSH form which may (or may not) expect the .git suffix.

That's one of the reasons I avoid the SSH protocol - because it tends to add noise like .git to URLs.

Trying to install it without the package name, I still get the same error:

 @ pip-run git+git@github.com:dan-blanchard/importlib_metadata.git
ERROR: Invalid requirement: 'git+git@github.com:dan-blanchard/importlib_metadata.git'
Hint: It looks like a path. File 'git+git@github.com:dan-blanchard/importlib_metadata.git' does not exist.

Perhaps it's the case that the syntax pip expects is different than what git supplies. Indeed, the pip docs indicate that the URL should be git+ssh://, so probably pytest-perf needs to add support for translating a git remote to a pip URL for SSH remotes.

If you want to work around the issue, you can use an https remote (e.g. git clone https://github.com/$(username)/importlib_metadata).

Let me see if I can resolve the conflicts and get the tests running again.

@jaraco
Copy link
Member

jaraco commented Mar 7, 2024

Thanks for this. Looks like it's pretty close.

I'm a little concerned that this change introduces more reliance on the assumption that the package is on disk, when it could be in a zip file or elsewhere.

@dan-blanchard
Copy link
Author

I believe all the test failures were mypy related so I added a cast() to fix/silence it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught Exception when reading Egg information
2 participants