Skip to content

Commit 9ef6002

Browse files
sadmac7000sadmac7000
committed
Consider public dependencies when choosing a version in cargo add (#13038)
1 parent 1194cda commit 9ef6002

File tree

8 files changed

+252
-15
lines changed

8 files changed

+252
-15
lines changed

‎src/cargo/ops/cargo_add/mod.rs‎

Lines changed: 125 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use toml_edit::Item as TomlItem;
2020

2121
use crate::CargoResult;
2222
use crate::GlobalContext;
23+
use crate::core::Feature;
2324
use crate::core::FeatureValue;
2425
use crate::core::Features;
2526
use crate::core::Package;
@@ -29,6 +30,7 @@ use crate::core::Summary;
2930
use crate::core::Workspace;
3031
use crate::core::dependency::DepKind;
3132
use crate::core::registry::PackageRegistry;
33+
use crate::ops::resolve_ws;
3234
use crate::sources::source::QueryKind;
3335
use crate::util::cache_lock::CacheLockMode;
3436
use crate::util::edit_distance;
@@ -38,6 +40,7 @@ use crate::util::toml_mut::dependency::Dependency;
3840
use crate::util::toml_mut::dependency::GitSource;
3941
use crate::util::toml_mut::dependency::MaybeWorkspace;
4042
use crate::util::toml_mut::dependency::PathSource;
43+
use crate::util::toml_mut::dependency::RegistrySource;
4144
use crate::util::toml_mut::dependency::Source;
4245
use crate::util::toml_mut::dependency::WorkspaceSource;
4346
use crate::util::toml_mut::manifest::DepTable;
@@ -472,17 +475,44 @@ fn resolve_dependency(
472475
}
473476
dependency = dependency.set_source(src);
474477
} else {
475-
let latest =
476-
get_latest_dependency(spec, &dependency, honor_rust_version, gctx, registry)?;
477-
478-
if dependency.name != latest.name {
479-
gctx.shell().warn(format!(
480-
"translating `{}` to `{}`",
481-
dependency.name, latest.name,
482-
))?;
483-
dependency.name = latest.name; // Normalize the name
478+
let (package_set, resolve) = resolve_ws(ws, true)?;
479+
let public_source = if spec
480+
.manifest()
481+
.unstable_features()
482+
.require(Feature::public_dependency())
483+
.is_ok()
484+
{
485+
get_public_dependency(
486+
manifest,
487+
ws,
488+
section,
489+
gctx,
490+
&dependency,
491+
package_set,
492+
resolve,
493+
)
494+
} else {
495+
None
496+
};
497+
if let Some((registry, public_source)) = public_source {
498+
if let Some(registry) = registry {
499+
dependency = dependency.set_registry(registry);
500+
}
501+
dependency = dependency.set_source(public_source);
502+
} else {
503+
let latest =
504+
get_latest_dependency(spec, &dependency, honor_rust_version, gctx, registry)?;
505+
506+
if dependency.name != latest.name {
507+
gctx.shell().warn(format!(
508+
"translating `{}` to `{}`",
509+
dependency.name, latest.name,
510+
))?;
511+
dependency.name = latest.name; // Normalize the name
512+
}
513+
dependency =
514+
dependency.set_source(latest.source.expect("latest always has a source"));
484515
}
485-
dependency = dependency.set_source(latest.source.expect("latest always has a source"));
486516
}
487517
}
488518

@@ -501,6 +531,89 @@ fn resolve_dependency(
501531
dependency = dependency.clear_version();
502532
}
503533

534+
let query = query_dependency(ws, gctx, &mut dependency)?;
535+
let dependency = populate_available_features(dependency, &query, registry)?;
536+
537+
Ok(dependency)
538+
}
539+
540+
fn get_public_dependency(
541+
manifest: &LocalManifest,
542+
ws: &Workspace<'_>,
543+
section: &DepTable,
544+
gctx: &GlobalContext,
545+
dependency: &Dependency,
546+
package_set: crate::core::PackageSet<'_>,
547+
resolve: crate::core::Resolve,
548+
) -> Option<(Option<String>, Source)> {
549+
manifest
550+
.get_dependency_versions_all(ws, ws.unstable_features())
551+
.filter_map(|(path, dep)| if path == *section { dep.ok() } else { None })
552+
.filter_map(|mut dep| {
553+
query_dependency(ws, gctx, &mut dep)
554+
.map(|dep| {
555+
package_set
556+
.package_ids()
557+
.filter(|package_id| {
558+
package_id.name() == dep.package_name()
559+
&& dep.version_req().matches(package_id.version())
560+
})
561+
.max_by_key(|x| x.version())
562+
})
563+
.transpose()
564+
})
565+
.map(|dep_pkgid| {
566+
let Ok(dep_pkgid) = dep_pkgid else {
567+
todo!();
568+
};
569+
resolve.deps(dep_pkgid).filter_map(|(id, deps)| {
570+
deps.iter()
571+
.find(|dep| {
572+
dep.is_public()
573+
&& dep.kind() == DepKind::Normal
574+
&& dep.package_name() == dependency.name.as_str()
575+
})
576+
.map(|dep| (id, dep.version_req().clone()))
577+
})
578+
})
579+
.flatten()
580+
.max_by_key(|(pkg_id, _)| pkg_id.version())
581+
.map(|(pkg_id, version_req)| {
582+
let source = pkg_id.source_id();
583+
if source.is_git() {
584+
(
585+
Option::<String>::None,
586+
Source::Git(GitSource::new(source.as_encoded_url().to_string())),
587+
)
588+
} else if let Some(path) = source.local_path() {
589+
(None, Source::Path(PathSource::new(path)))
590+
} else {
591+
let toml_source = match version_req {
592+
crate::util::OptVersionReq::Any => Source::Registry(RegistrySource::new(
593+
format!("={}", pkg_id.version().to_string()),
594+
)),
595+
crate::util::OptVersionReq::Req(version_req)
596+
| crate::util::OptVersionReq::Locked(_, version_req)
597+
| crate::util::OptVersionReq::Precise(_, version_req) => {
598+
Source::Registry(RegistrySource::new(version_req.to_string()))
599+
}
600+
};
601+
(
602+
source
603+
.alt_registry_key()
604+
.map(|x| x.to_owned())
605+
.filter(|_| !source.is_crates_io()),
606+
toml_source,
607+
)
608+
}
609+
})
610+
}
611+
612+
fn query_dependency(
613+
ws: &Workspace<'_>,
614+
gctx: &GlobalContext,
615+
dependency: &mut Dependency,
616+
) -> CargoResult<crate::core::Dependency> {
504617
let query = dependency.query(gctx)?;
505618
let query = match query {
506619
MaybeWorkspace::Workspace(_workspace) => {
@@ -511,7 +624,7 @@ fn resolve_dependency(
511624
ws.unstable_features(),
512625
)?;
513626
if let Some(features) = dep.features.clone() {
514-
dependency = dependency.set_inherited_features(features);
627+
*dependency = dependency.clone().set_inherited_features(features);
515628
}
516629
let query = dep.query(gctx)?;
517630
match query {
@@ -523,10 +636,7 @@ fn resolve_dependency(
523636
}
524637
MaybeWorkspace::Other(query) => query,
525638
};
526-
527-
let dependency = populate_available_features(dependency, &query, registry)?;
528-
529-
Ok(dependency)
639+
Ok(query)
530640
}
531641

532642
fn fuzzy_lookup(

‎src/cargo/util/toml_mut/manifest.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,37 @@ impl LocalManifest {
326326
cargo_util::paths::write_atomic(&self.path, new_contents_bytes)
327327
}
328328

329+
/// Lookup all dependencies.
330+
pub fn get_dependency_versions_all<'s>(
331+
&'s self,
332+
ws: &'s Workspace<'_>,
333+
unstable_features: &'s Features,
334+
) -> impl Iterator<Item = (DepTable, CargoResult<Dependency>)> + 's {
335+
let crate_root = self.path.parent().expect("manifest path is absolute");
336+
self.get_sections()
337+
.into_iter()
338+
.filter_map(move |(table_path, table)| {
339+
let table = table.into_table().ok()?;
340+
Some(
341+
table
342+
.into_iter()
343+
.map(|(key, item)| (table_path.clone(), key, item))
344+
.collect::<Vec<_>>(),
345+
)
346+
})
347+
.flatten()
348+
.map(move |(table_path, dep_key, dep_item)| {
349+
let dep = Dependency::from_toml(
350+
ws.gctx(),
351+
ws.root(),
352+
crate_root,
353+
unstable_features,
354+
&dep_key,
355+
&dep_item,
356+
);
357+
(table_path, dep)
358+
})
359+
}
329360
/// Lookup a dependency.
330361
pub fn get_dependency_versions<'s>(
331362
&'s self,

‎tests/testsuite/cargo_add/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ mod preserve_features_unsorted;
134134
mod preserve_sorted;
135135
mod preserve_unsorted;
136136
mod public;
137+
mod public_common_version;
137138
mod quiet;
138139
mod registry;
139140
mod rename;

‎tests/testsuite/cargo_add/public_common_version/in/Cargo.toml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
cargo-features = ["public-dependency"]
2+
[workspace]
3+
4+
[package]
5+
name = "cargo-list-test-fixture"
6+
version = "0.0.0"
7+
edition = "2015"
8+
9+
[dependencies]
10+
my-package = "0.1.0"

‎tests/testsuite/cargo_add/public_common_version/in/src/lib.rs‎

Whitespace-only changes.

‎tests/testsuite/cargo_add/public_common_version/mod.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
use crate::prelude::*;
2+
use cargo_test_support::Project;
3+
use cargo_test_support::compare::assert_ui;
4+
use cargo_test_support::current_dir;
5+
use cargo_test_support::file;
6+
use cargo_test_support::registry::Dependency;
7+
use cargo_test_support::str;
8+
9+
#[cargo_test]
10+
fn case() {
11+
cargo_test_support::registry::init();
12+
cargo_test_support::registry::Package::new("my-package-dep", "0.1.0").publish();
13+
cargo_test_support::registry::Package::new("my-package-dep", "0.2.0").publish();
14+
cargo_test_support::registry::Package::new("my-package", "0.1.0")
15+
.add_dep(Dependency::new("my-package-dep", "0.1.0").public(true))
16+
.publish();
17+
cargo_test_support::registry::Package::new("my-package", "0.2.0")
18+
.add_dep(Dependency::new("my-package-dep", "0.2.0").public(true))
19+
.publish();
20+
let project = Project::from_template(current_dir!().join("in"));
21+
let project_root = project.root();
22+
let cwd = &project_root;
23+
24+
snapbox::cmd::Command::cargo_ui()
25+
.arg("add")
26+
.arg_line("my-package-dep")
27+
.current_dir(cwd)
28+
.masquerade_as_nightly_cargo(&["public-dependency"])
29+
.assert()
30+
.success()
31+
.stdout_eq(str![""])
32+
.stderr_eq(file!["stderr.term.svg"]);
33+
34+
assert_ui().subset_matches(current_dir!().join("out"), &project_root);
35+
}

‎tests/testsuite/cargo_add/public_common_version/out/Cargo.toml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
cargo-features = ["public-dependency"]
2+
[workspace]
3+
4+
[package]
5+
name = "cargo-list-test-fixture"
6+
version = "0.0.0"
7+
edition = "2015"
8+
9+
[dependencies]
10+
my-package = "0.1.0"
11+
my-package-dep = "^0.1.0"

‎tests/testsuite/cargo_add/public_common_version/stderr.term.svg‎

Lines changed: 39 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)