Fix extra indexes #5220
Fix extra indexes #5220
Conversation
aronbierbaum
force-pushed
the
fix_extra_indexes
branch
from
cc5b397
to
7dfb30e
Compare
|
This seems like a good improvement, but I still would like to get a second opinion from @matteius . |
aronbierbaum
force-pushed
the
fix_extra_indexes
branch
2 times, most recently
from
85030e6
to
f718642
Compare
| source_names = {src.get("name") for src in sources} | ||
| if not search_all_sources and requirement.index in source_names: |
There was a problem hiding this comment.
I think this change might have broken some tests. The proposed change only shows that the previous code would never find requirement.index in sources because the types didn't match. str and List[TSource].
I don't know enough about the desired behavior, but an alternative would be to remove the conditional completely since it would never be True.
There was a problem hiding this comment.
I see the problem, thank you for your analysis. I'd like a little bit of time to consider what I was trying to accomplish with this code, given what I know now.
There was a problem hiding this comment.
@aronbierbaum This patch fixes the other test that does fail:
diff --git a/tests/integration/test_lock.py b/tests/integration/test_lock.py
index 28e7f21f..d571a8d0 100644
--- a/tests/integration/test_lock.py
+++ b/tests/integration/test_lock.py
@@ -335,10 +335,11 @@ name = "testpypi"
[packages]
pipenv-test-private-package = {version = "*", index = "testpypi"}
+pipenv-test-public-package = {version = "*", index = "pypi"}
requests = "*"
""".strip()
f.write(contents)
There was a problem hiding this comment.
The reason that is important is because it doesn't search all sources and pipenv-test-private-package is in testpypi but requires pipenv-test-public-package is in pypi. When we initially switched to index restricted packages, there were some users that had to do similar things for their Pipfile index restrictions to work.
|
@aronbierbaum I am not sure why earlier I could not get that second test failure, because I was able to reproduce it now. I think this would be a good way the fix the other test, since we showed both methods here, one where we rely on index restrictions, and this test where we allow all indexes to be searched: |
The else clause of a for loop is executed unless a break statement is executed to terminate the for loop. https://docs.python.org/3/reference/compound_stmts.html#the-for-statement
aronbierbaum
force-pushed
the
fix_extra_indexes
branch
from
c8316ab
to
5edcec9
Compare
|
@aronbierbaum Thanks for this fix! 👍 I've been pulling hairs for hours trying to understand why pipenv is installing the public "utils" package instead of my private one. You're awesome man! 🥇 |
The issue
When installing a requirement that explicitly references an index all indexes are searched even though
install_search_all_sources = False.The fix
There were two different locations where the code appears to not have the expected behavior.
The checklist
news/directory to describe this fix with the extension.bugfix,.feature,.behavior,.doc..vendor. or.trivial(this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.