uv-resolver: include 'sdist' entry for direct URL dependencies

In the case of a direct URL sdist, it includes a hash, and this hash is
not (and probably should not) be part of the `source`. The URL is part
of the source because it permits uniquely identifying this particular
package as distinct from any other package with the same name. But, we
should still include the hash.

So in this commit, we rejigger what we did previously to make it so the
`SourceDist` value isn't even constructed at all when it isn't needed.
This also in turn lets us make the hash field required (which we will do
in a subsequent commit).

This does mean the URL is stored twice for direct URL dependencies in
the lock file. This seems non-ideal. We could make the URL for the sdist
optional, but this seems like a bridge too far? Another choice is to add
a new key to `distribution` that is just `direct-url-hash`, but that
also seems mucky.

Maybe the duplication here is okay given the relative rarity of direct
URL dependencies.
This commit is contained in:
Andrew Gallant 2024-06-25 14:19:51 -04:00 committed by Andrew Gallant
parent 9d4681cf19
commit 7c71aec68c

View file

@ -421,12 +421,7 @@ impl Lock {
table.insert("source", value(dist.id.source.to_string()));
if let Some(ref sdist) = dist.sdist {
// An actual sdist entry in the lock file is only required when
// it's from a registry. Otherwise, it's strictly redundant
// with the information in all other kinds of `source`.
if matches!(dist.id.source, Source::Registry(_)) {
table.insert("sdist", value(sdist.to_toml()?));
}
table.insert("sdist", value(sdist.to_toml()?));
}
if !dist.dependencies.is_empty() {
@ -1449,32 +1444,27 @@ impl SourceDist {
SourceDist::from_registry_dist(sdist).map(Some)
}
Dist::Built(_) => Ok(None),
Dist::Source(ref source_dist) => {
SourceDist::from_source_dist(source_dist, hashes).map(Some)
}
Dist::Source(ref source_dist) => SourceDist::from_source_dist(source_dist, hashes),
}
}
fn from_source_dist(
source_dist: &distribution_types::SourceDist,
hashes: &[HashDigest],
) -> Result<SourceDist, LockError> {
) -> Result<Option<SourceDist>, LockError> {
match *source_dist {
distribution_types::SourceDist::Registry(ref reg_dist) => {
SourceDist::from_registry_dist(reg_dist)
SourceDist::from_registry_dist(reg_dist).map(Some)
}
distribution_types::SourceDist::DirectUrl(ref direct_dist) => {
Ok(SourceDist::from_direct_dist(direct_dist, hashes))
}
distribution_types::SourceDist::Git(ref git_dist) => {
Ok(SourceDist::from_git_dist(git_dist, hashes))
}
distribution_types::SourceDist::Path(ref path_dist) => {
Ok(SourceDist::from_path_dist(path_dist, hashes))
}
distribution_types::SourceDist::Directory(ref directory_dist) => {
Ok(SourceDist::from_directory_dist(directory_dist, hashes))
Ok(Some(SourceDist::from_direct_dist(direct_dist, hashes)))
}
// An actual sdist entry in the lock file is only required when
// it's from a registry or a direct URL. Otherwise, it's strictly
// redundant with the information in all other kinds of `source`.
distribution_types::SourceDist::Git(_)
| distribution_types::SourceDist::Path(_)
| distribution_types::SourceDist::Directory(_) => Ok(None),
}
}
@ -1502,39 +1492,6 @@ impl SourceDist {
},
}
}
fn from_git_dist(git_dist: &GitSourceDist, hashes: &[HashDigest]) -> SourceDist {
SourceDist::Url {
url: locked_git_url(git_dist),
metadata: SourceDistMetadata {
hash: hashes.first().cloned().map(Hash::from),
size: None,
},
}
}
fn from_path_dist(path_dist: &PathSourceDist, hashes: &[HashDigest]) -> SourceDist {
SourceDist::Path {
path: path_dist.lock_path.clone(),
metadata: SourceDistMetadata {
hash: hashes.first().cloned().map(Hash::from),
size: None,
},
}
}
fn from_directory_dist(
directory_dist: &DirectorySourceDist,
hashes: &[HashDigest],
) -> SourceDist {
SourceDist::Path {
path: directory_dist.lock_path.clone(),
metadata: SourceDistMetadata {
hash: hashes.first().cloned().map(Hash::from),
size: None,
},
}
}
}
impl From<GitReference> for GitSourceKind {