-
Notifications
You must be signed in to change notification settings - Fork 760
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
remove source
and version
from dependencies in lock file when unambiguous
#4513
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
Sweet! |
BurntSushi
force-pushed
the
ag/lock-dependency-trim
branch
from
June 25, 2024 17:24
153484b
to
72e5ebf
Compare
ibraheemdev
approved these changes
Jun 25, 2024
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.
Nice! This cleans up the lockfile quite a bit.
charliermarsh
approved these changes
Jun 26, 2024
konstin
approved these changes
Jun 26, 2024
BurntSushi
force-pushed
the
ag/lock-dependency-trim
branch
from
June 26, 2024 11:58
72e5ebf
to
a99bf25
Compare
This commit prepares to make the `source` and `version` fields optional in a `distribution.dependency` based on whether they have an unambiguous value. e.g., When there is exactly one distribution with a matching package name. This refactor effectively defines "wire" types for most of the lock data types (repeating the `WheelWire` and `LockWire` pattern) with one key difference: we don't use serde's `TryFrom` integration. In this refactor, we could have, and it would have worked. But in a subsequent commit, we're going to be adding state to the `unwire()` calls that is impossible to thread through a `TryFrom` implementation. This state will tell us how to populate the `source` and `version` values on a `Dependency` when they're missing. The duplication of types here is unfortunate, but compiler should catch any deviations. And the wire types are unexported, so they have a limited blast radius on complexity.
When there is only one distribution for a particular package name, any dependencies (the edges in the resolution graph) that reference that package name are completely unambiguous. Therefore, we can actually omit their version and source information and instead derive it from the distribution entry. We add some tests to check the success and error cases. That is, when `source` or `version` are omitted and there are more than one corresponding distribution for the package name (i.e., it's ambiguous), then lock deserialization should fail.
This update follows from the removal of of `source` and `version` from `distribution.dependency` entries in the lock file when the package name unambiguously refers to a single distribution.
BurntSushi
force-pushed
the
ag/lock-dependency-trim
branch
from
June 26, 2024 11:59
a99bf25
to
c9a918f
Compare
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.
When there is only one distribution for a particular package name, any
dependencies (the edges in the resolution graph) that reference that
package name are completely unambiguous. Therefore, we can actually
omit their version and source information and instead derive it from
the distribution entry.
We add some tests to check the success and error cases. That is,
when
source
orversion
are omitted and there are more than onecorresponding distribution for the package name (i.e., it's ambiguous),
then lock deserialization should fail.
Since there can be a lot of
distribution.dependency
blocks, thischange trims a lot of fat from a lock file.
Cargo does a similar thing here where it omits the source and version
from dependencies when their value is unambiguous.
Ref #3611