Use impl IntoIterator to avoid clones
#24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
\cc @zanieb for visibility. |
charliermarsh
force-pushed
the
charlie/into
branch
from
c641910
to
247eb8b
Compare
charliermarsh
force-pushed
the
charlie/into
branch
from
247eb8b
to
5d0270c
Compare
zanieb
reviewed
Feb 21, 2024
src/lib.rs
Outdated
| //! unimplemented!(); | ||
| //! Ok(Dependencies::Available([])) |
zanieb
reviewed
Feb 21, 2024
Comment on lines
-170
to
175
| Dependencies::Available(x) if x.contains_key(p) => { | ||
| Dependencies::Available(x) if x.clone().into_iter().any(|(d, _)| &d == p) => { | ||
| return Err(PubGrubError::SelfDependency { | ||
| package: p.clone(), | ||
| version: v, | ||
| version: v.clone(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Are any of these new clones problematic?
There was a problem hiding this comment.
Not for us, because we already use a vector here IIUC.
zanieb
approved these changes
Feb 21, 2024
charliermarsh
force-pushed
the
charlie/into
branch
from
5d0270c
to
a7fa962
Compare
charliermarsh
force-pushed
the
charlie/into
branch
from
a7fa962
to
e95523a
Compare
charliermarsh
added a commit
to astral-sh/uv
that referenced
this pull request
Feb 22, 2024
## Summary This revives a PR from long ago (#383 and astral-sh/pubgrub#24) that modifies how we deal with dependencies that are declared multiple times within a single package. To quote from the originating PR: > Uses an experimental pubgrub branch (#370) that allows us to handle multiple version ranges for a single dependency to the solver which results in better error messages because the derivation tree contains all of the relevant versions. Previously, the version ranges were merged (by us) in the resolver before handing them to pubgrub since only one range could be provided per package. Since we don't merge the versions anymore, we no longer give the solver an empty range for conflicting requirements; instead the solver comes to that conclusion from the provided versions. You can see the improved error message for direct dependencies in [this snapshot](https://github.com/astral-sh/puffin/pull/383/files#diff-a0437f2c20cde5e2f15199a3bf81a102b92580063268417847ec9c793a115bd0). The main issue with that PR was around its handling of URL dependencies, so this PR _also_ refactors how we handle those. Previously, we stored URL dependencies on `PubGrubPackage`, but they were omitted from the hash and equality implementations of `PubGrubPackage`. This led to some really careful codepaths wherein we had to ensure that we always visited URLs before non-URL packages, so that the URL-inclusive versions were included in any hashmaps, etc. I considered preserving this approach, but it would require us to rely on lots of internal details of PubGrub (since we'd now be relying on PubGrub to merge those packages in the "right" order). So, instead, we now _always_ set the URL on a given package, whenever that package was _given_ a URL upfront. I think this is easier to reason about: if the user provided a URL for `flask`, then we should just always add the URL for `flask`. If we see some other URL for `flask`, we error, like before. If we see some unknown URL for `flask`, we error, like before. Closes #1522. Closes #1821. Closes #1615.
konstin
pushed a commit
that referenced
this pull request
Mar 6, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Mar 12, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Mar 12, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Mar 13, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Mar 22, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
May 2, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
May 7, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
May 8, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
May 21, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Jun 1, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Jun 1, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Jun 1, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Jun 3, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
konstin
pushed a commit
that referenced
this pull request
Jun 3, 2024
Co-authored-by: Jacob Finkelman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviving Jacob's PR to see if we can improve our error messages for conflicting dependencies within a single package.