-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fix requirements parsing with index URL option #1251
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This round of changes was tested and appears to function as intended...but the understanding is that more changes are coming to better satisfy CLI and API...by accounting for these types of dependencies as third party.
CHANGELOG entry needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes were tested and work for the specific test case provided, but alternate inputs result in failure.
This reverts commit 2f0660a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes were tested locally and appear to work as intended. There is still the possibility that a -f
/--find-links
option is included in the requirements file, which should similarly be treated as a third party dependency source. However, that case may be able to wait until it is clear that it causes issues for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes were tested locally and work as intended.
// primary one. | ||
if let Some(index_url) = line | ||
.strip_prefix("--index-url") | ||
.and_then(|line| line.strip_prefix(['=', ' '])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure, I'd assume --index-url = https://whatever.com
is not valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other weird edge cases... But I don't want to waste a bunch of time on them...
> cat req.txt
--index-url https://pypi.org/simple AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
pyyaml
> pip install --dry-run -r req.txt
Collecting pyyaml
Downloading PyYAML-6.0.1-cp310-cp310-macosx_11_0_arm64.whl (169 kB)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 169.3/169.3 kB 2.1 MB/s eta 0:00:00
Would install PyYAML-6.0.1
No description provided.