Skip to content

fix incorrect resolution of aliased git dependencies #391

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 74 additions & 14 deletions crate2nix/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -773,27 +773,58 @@ impl<'a> ResolvedDependencies<'a> {
let resolved = resolved_packages_by_crate_name
.get(&name)
.and_then(|packages| {
let exact_match = packages
let matches = packages
.iter()
.find(|p| package_dep.req.matches(&p.version));
.filter(|p| package_dep.req.matches(&p.version))
.collect::<Vec<_>>();

// Strip prerelease/build info from versions if we
// did not find an exact match.
//
// E.g. "*" does not match a prerelease version in this
// library but cargo thinks differently.

exact_match.or_else(|| {
packages.iter().find(|p| {
let without_metadata = {
let mut version = p.version.clone();
version.pre = semver::Prerelease::EMPTY;
version.build = semver::BuildMetadata::EMPTY;
version
};
package_dep.req.matches(&without_metadata)
})
})
let matches = if matches.is_empty() {
packages
.iter()
.filter(|p| {
let without_metadata = {
let mut version = p.version.clone();
version.pre = semver::Prerelease::EMPTY;
version.build = semver::BuildMetadata::EMPTY;
version
};
package_dep.req.matches(&without_metadata)
})
.collect()
} else {
matches
};

// It is possible to have multiple packages that match the name and version
// requirement of the dependency. In particular if there are multiple
// dependencies on the same package via git at different revisions - in
// that case `package_dep.req` is set to `*` so we can't use the version
// requirement to match the appropriate locked package with the dependency.
// Instead it's necessary to compare by source instead.
let matches = if matches.len() > 1 {
matches
.into_iter()
.filter(|p| {
sources_match(package_dep.source.as_deref(), p.source.as_ref())
.unwrap_or(false)
})
.collect()
} else {
matches
};

if matches.len() == 1 {
Some(matches[0])
} else if matches.is_empty() {
None
} else {
panic!("Could not find an unambiguous package match for dependency, {}. Candidates are: {}", &package_dep.name, matches.iter().map(|p| &p.id).join(", "));
}
});

let dep_package = resolved?;
Expand All @@ -815,6 +846,35 @@ impl<'a> ResolvedDependencies<'a> {
}
}

fn sources_match(
dependency_source: Option<&str>,
package_source: Option<&Source>,
) -> Result<bool, anyhow::Error> {
let Some(dependency_source) = dependency_source else {
return Ok(package_source.is_none());
};
let Some(package_source) = package_source else {
return Ok(false); // fail if dependency has a source, but package does not
};

let dependency = Url::parse(dependency_source)?;
let package = Url::parse(&package_source.repr)?;

let scheme_matches = dependency.scheme() == package.scheme();
let domain_matches = dependency.domain() == package.domain();
let path_matches = dependency.path() == package.path();
let query_matches = {
let package_query = package.query_pairs().collect::<HashMap<_, _>>();
dependency.query_pairs().all(|(key, dep_value)| {
package_query
.get(&key)
.is_some_and(|pkg_value| &dep_value == pkg_value)
})
};

Ok(scheme_matches && domain_matches && path_matches && query_matches)
}

/// Converts one type into another by serializing/deserializing it.
///
/// Therefore, the output json of `I` must be deserializable to `O`.
Expand Down
26 changes: 26 additions & 0 deletions sample_projects/aliased-dependencies/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions sample_projects/aliased-dependencies/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "aliased-dependencies"
version = "0.1.0"
edition = "2021"

[dependencies]
hello = { package = "ludndev-hello-world", git = "https://github.com/ludndev/rustlang-hello-world-lib.git", tag = "v0.1.1" }
hello_v010 = { package = "ludndev-hello-world", git = "https://github.com/ludndev/rustlang-hello-world-lib", tag = "v0.1.0" }
hello_path = { package = "ludndev-hello-world", path = "./hello" }
7 changes: 7 additions & 0 deletions sample_projects/aliased-dependencies/hello/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions sample_projects/aliased-dependencies/hello/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[package]
name = "ludndev-hello-world"
version = "0.0.1"
edition = "2021"
3 changes: 3 additions & 0 deletions sample_projects/aliased-dependencies/hello/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn greet() -> String {
"hello".to_string()
}
5 changes: 5 additions & 0 deletions sample_projects/aliased-dependencies/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub fn main() {
println!("{}", hello::greet(""));
println!("{}", hello_v010::greet(""));
println!("{}", hello_path::greet());
}
6 changes: 6 additions & 0 deletions tests.nix
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,12 @@ let
# # FIXME: https://github.com/nix-community/crate2nix/issues/319
skip = true;
}

{
name = "aliased_dependencies";
src = ./sample_projects/aliased-dependencies;
expectedOutput = "Hello World !\nHello World !";
}
];
buildTestDerivationAttrSet =
let
Expand Down