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

req: fix possible exception in install_req_from_req #5893

Closed
wants to merge 2 commits into from

Conversation

benoit-pierre
Copy link
Member

  • allow calling it with comes_from=None (easier for testing)
  • handle the case where comes_from.link is None (which can happen if a package using a PEP 508 URL requirement is already installed)

Fix #5889.

Note: this depends on #5892 for the test.

* allow calling it with `comes_from=None` (easier for testing)
* handle the case where `comes_from.link is None` (which can happen
  if a package using a PEP 508 URL requirement is already installed)
@cjerdonek
Copy link
Member

cjerdonek commented Oct 21, 2018

@benoit-pierre Is there a reason the fix for #5889 has to be combined with other code? Is it possible to submit a PR just with the comes_from check and some simple unit tests of install_req_from_req()? It seems that would make the patch smaller and more focused and easier to review, and the test more specific to the particular issue, etc.

@benoit-pierre
Copy link
Member Author

Yes, as mentioned in the note I need #5892 for the test: pkg @ file:///path/to/wheel cannot be used because of pypa/packaging#120, and pkg @ file://localhost/path/to/wheel does not without #5892 on Linux because pip tries to use a UNC path...

@cjerdonek
Copy link
Member

What about a simple unit test of install_req_from_req() that exercises that code path? I don't see one in the PR. I only see a higher-level end-to-end test.

@pradyunsg pradyunsg added C: tests Testing and related things S: awaiting response Waiting for a response/more information labels Nov 10, 2018
@lock
Copy link

lock bot commented May 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 31, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: tests Testing and related things S: awaiting response Waiting for a response/more information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: 'NoneType' object has no attribute 'netloc'
3 participants