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

Respect sentinels in prioritization #10443

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Respect sentinels in prioritization #10443

merged 1 commit into from
Jan 9, 2025

Conversation

charliermarsh
Copy link
Member

Summary

If a user provides a constraint like flask==3.0.0, that gets expanded to [3.0.0, 3.0.0+[max]). So it's not a singleton, but it should be treated as such for the purposes of prioritization, since in practice it will almost always map to a single version.

@charliermarsh charliermarsh added the performance Potential performance improvement label Jan 9, 2025
@charliermarsh charliermarsh marked this pull request as ready for review January 9, 2025 20:43
@@ -162,8 +162,6 @@ impl VersionMap {
range: &Ranges<Version>,
) -> impl DoubleEndedIterator<Item = (&Version, VersionMapDistHandle)> {
// Performance optimization: If we only have a single version, return that version directly.
//
// TODO(charlie): Now that we use local version sentinels, does this ever trigger?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does... Proxy packages use Ranges::singleton, which hits this path.

@charliermarsh charliermarsh force-pushed the charlie/sentinel branch 2 times, most recently from 6335f75 to 6ac2113 Compare January 9, 2025 20:57
@charliermarsh charliermarsh enabled auto-merge (squash) January 9, 2025 20:58
@charliermarsh charliermarsh merged commit 7096e83 into main Jan 9, 2025
64 checks passed
@charliermarsh charliermarsh deleted the charlie/sentinel branch January 9, 2025 21:19
@@ -178,9 +178,7 @@ impl PubGrubPriorities {
// The ConflictEarly` match avoids infinite loops.
if matches!(
entry.get(),
PubGrubPriority::ConflictLate(_)
| PubGrubPriority::ConflictEarly(_)
| PubGrubPriority::Singleton(_)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konstin -- Heads up, I removed this... Otherwise, the wrong_backtracking_basic test failed. I think it succeeded by accident before? Since all the conflicts are in singletons.

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

Successfully merging this pull request may close these issues.

2 participants