Skip to content

Commit dd56d79

Browse files
committed
refactor(toml): Only collect nested paths when needed
This simplifies the interface for `toml/mod.rs` and reduces the work we do.
1 parent 954142e commit dd56d79

File tree

5 files changed

+60
-32
lines changed

5 files changed

+60
-32
lines changed

src/cargo/core/workspace.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,6 @@ impl<'gctx> Workspace<'gctx> {
420420
let source = SourceId::for_path(self.root())?;
421421

422422
let mut warnings = Vec::new();
423-
let mut nested_paths = Vec::new();
424423

425424
let mut patch = HashMap::new();
426425
for (url, deps) in config_patch.into_iter().flatten() {
@@ -442,7 +441,6 @@ impl<'gctx> Workspace<'gctx> {
442441
dep,
443442
name,
444443
source,
445-
&mut nested_paths,
446444
self.gctx,
447445
&mut warnings,
448446
/* platform */ None,
@@ -1063,7 +1061,7 @@ impl<'gctx> Workspace<'gctx> {
10631061
return Ok(p);
10641062
}
10651063
let source_id = SourceId::for_path(manifest_path.parent().unwrap())?;
1066-
let (package, _nested_paths) = ops::read_package(manifest_path, source_id, self.gctx)?;
1064+
let package = ops::read_package(manifest_path, source_id, self.gctx)?;
10671065
loaded.insert(manifest_path.to_path_buf(), package.clone());
10681066
Ok(package)
10691067
}
@@ -1596,7 +1594,7 @@ impl<'gctx> Packages<'gctx> {
15961594
Entry::Occupied(e) => Ok(e.into_mut()),
15971595
Entry::Vacant(v) => {
15981596
let source_id = SourceId::for_path(key)?;
1599-
let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, self.gctx)?;
1597+
let manifest = read_manifest(manifest_path, source_id, self.gctx)?;
16001598
Ok(v.insert(match manifest {
16011599
EitherManifest::Real(manifest) => {
16021600
MaybePackage::Package(Package::new(manifest, manifest_path))
@@ -1746,7 +1744,7 @@ pub fn find_workspace_root(
17461744
find_workspace_root_with_loader(manifest_path, gctx, |self_path| {
17471745
let key = self_path.parent().unwrap();
17481746
let source_id = SourceId::for_path(key)?;
1749-
let (manifest, _nested_paths) = read_manifest(self_path, source_id, gctx)?;
1747+
let manifest = read_manifest(self_path, source_id, gctx)?;
17501748
Ok(manifest
17511749
.workspace_config()
17521750
.get_ws_root(self_path, manifest_path))

src/cargo/ops/cargo_package.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,7 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
456456
let toml_manifest =
457457
prepare_for_publish(orig_pkg.manifest().resolved_toml(), ws, orig_pkg.root())?;
458458
let source_id = orig_pkg.package_id().source_id();
459-
let (manifest, _nested_paths) =
460-
to_real_manifest(toml_manifest, source_id, orig_pkg.manifest_path(), gctx)?;
459+
let manifest = to_real_manifest(toml_manifest, source_id, orig_pkg.manifest_path(), gctx)?;
461460
let new_pkg = Package::new(manifest, orig_pkg.manifest_path());
462461

463462
// Regenerate Cargo.lock using the old one as a guide.

src/cargo/ops/cargo_read_manifest.rs

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::fs;
33
use std::io;
44
use std::path::{Path, PathBuf};
55

6-
use crate::core::{EitherManifest, Package, PackageId, SourceId};
6+
use crate::core::{EitherManifest, Manifest, Package, PackageId, SourceId};
77
use crate::util::errors::CargoResult;
88
use crate::util::important_paths::find_project_manifest_exact;
99
use crate::util::toml::read_manifest;
@@ -15,13 +15,13 @@ pub fn read_package(
1515
path: &Path,
1616
source_id: SourceId,
1717
gctx: &GlobalContext,
18-
) -> CargoResult<(Package, Vec<PathBuf>)> {
18+
) -> CargoResult<Package> {
1919
trace!(
2020
"read_package; path={}; source-id={}",
2121
path.display(),
2222
source_id
2323
);
24-
let (manifest, nested) = read_manifest(path, source_id, gctx)?;
24+
let manifest = read_manifest(path, source_id, gctx)?;
2525
let manifest = match manifest {
2626
EitherManifest::Real(manifest) => manifest,
2727
EitherManifest::Virtual(..) => anyhow::bail!(
@@ -31,7 +31,7 @@ pub fn read_package(
3131
),
3232
};
3333

34-
Ok((Package::new(manifest, path), nested))
34+
Ok(Package::new(manifest, path))
3535
}
3636

3737
pub fn read_packages(
@@ -107,6 +107,44 @@ pub fn read_packages(
107107
}
108108
}
109109

110+
fn nested_paths(manifest: &Manifest) -> Vec<PathBuf> {
111+
let mut nested_paths = Vec::new();
112+
let resolved = manifest.resolved_toml();
113+
let dependencies = resolved
114+
.dependencies
115+
.iter()
116+
.chain(resolved.build_dependencies())
117+
.chain(resolved.dev_dependencies())
118+
.chain(
119+
resolved
120+
.target
121+
.as_ref()
122+
.into_iter()
123+
.flat_map(|t| t.values())
124+
.flat_map(|t| {
125+
t.dependencies
126+
.iter()
127+
.chain(t.build_dependencies())
128+
.chain(t.dev_dependencies())
129+
}),
130+
);
131+
for dep_table in dependencies {
132+
for dep in dep_table.values() {
133+
let cargo_util_schemas::manifest::InheritableDependency::Value(dep) = dep else {
134+
continue;
135+
};
136+
let cargo_util_schemas::manifest::TomlDependency::Detailed(dep) = dep else {
137+
continue;
138+
};
139+
let Some(path) = dep.path.as_ref() else {
140+
continue;
141+
};
142+
nested_paths.push(PathBuf::from(path.as_str()));
143+
}
144+
}
145+
nested_paths
146+
}
147+
110148
fn walk(path: &Path, callback: &mut dyn FnMut(&Path) -> CargoResult<bool>) -> CargoResult<()> {
111149
if !callback(path)? {
112150
trace!("not processing {}", path.display());
@@ -151,7 +189,7 @@ fn read_nested_packages(
151189

152190
let manifest_path = find_project_manifest_exact(path, "Cargo.toml")?;
153191

154-
let (manifest, nested) = match read_manifest(&manifest_path, source_id, gctx) {
192+
let manifest = match read_manifest(&manifest_path, source_id, gctx) {
155193
Err(err) => {
156194
// Ignore malformed manifests found on git repositories
157195
//
@@ -174,6 +212,7 @@ fn read_nested_packages(
174212
EitherManifest::Real(manifest) => manifest,
175213
EitherManifest::Virtual(..) => return Ok(()),
176214
};
215+
let nested = nested_paths(&manifest);
177216
let pkg = Package::new(manifest, &manifest_path);
178217

179218
let pkg_id = pkg.package_id();

src/cargo/sources/path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl<'gctx> PathSource<'gctx> {
110110
ops::read_packages(&self.path, self.source_id, self.gctx)
111111
} else {
112112
let path = self.path.join("Cargo.toml");
113-
let (pkg, _) = ops::read_package(&path, self.source_id, self.gctx)?;
113+
let pkg = ops::read_package(&path, self.source_id, self.gctx)?;
114114
Ok(vec![pkg])
115115
}
116116
}

src/cargo/util/toml/mod.rs

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub fn read_manifest(
4747
path: &Path,
4848
source_id: SourceId,
4949
gctx: &GlobalContext,
50-
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
50+
) -> CargoResult<EitherManifest> {
5151
let contents =
5252
read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?;
5353
let document =
@@ -174,13 +174,13 @@ fn convert_toml(
174174
manifest_file: &Path,
175175
source_id: SourceId,
176176
gctx: &GlobalContext,
177-
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
177+
) -> CargoResult<EitherManifest> {
178178
return if manifest.package().is_some() {
179-
let (manifest, paths) = to_real_manifest(manifest, source_id, manifest_file, gctx)?;
180-
Ok((EitherManifest::Real(manifest), paths))
179+
let manifest = to_real_manifest(manifest, source_id, manifest_file, gctx)?;
180+
Ok(EitherManifest::Real(manifest))
181181
} else {
182-
let (m, paths) = to_virtual_manifest(manifest, source_id, manifest_file, gctx)?;
183-
Ok((EitherManifest::Virtual(m), paths))
182+
let manifest = to_virtual_manifest(manifest, source_id, manifest_file, gctx)?;
183+
Ok(EitherManifest::Virtual(manifest))
184184
};
185185
}
186186

@@ -464,7 +464,7 @@ pub fn to_real_manifest(
464464
source_id: SourceId,
465465
manifest_file: &Path,
466466
gctx: &GlobalContext,
467-
) -> CargoResult<(Manifest, Vec<PathBuf>)> {
467+
) -> CargoResult<Manifest> {
468468
fn get_ws(
469469
gctx: &GlobalContext,
470470
resolved_path: &Path,
@@ -516,7 +516,6 @@ pub fn to_real_manifest(
516516
}
517517
}
518518

519-
let mut nested_paths = vec![];
520519
let mut warnings = vec![];
521520
let mut errors = vec![];
522521

@@ -770,7 +769,6 @@ pub fn to_real_manifest(
770769
let mut manifest_ctx = ManifestContext {
771770
deps: &mut deps,
772771
source_id,
773-
nested_paths: &mut nested_paths,
774772
gctx,
775773
warnings: &mut warnings,
776774
features: &features,
@@ -1272,15 +1270,15 @@ pub fn to_real_manifest(
12721270

12731271
manifest.feature_gate()?;
12741272

1275-
Ok((manifest, nested_paths))
1273+
Ok(manifest)
12761274
}
12771275

12781276
fn to_virtual_manifest(
12791277
me: manifest::TomlManifest,
12801278
source_id: SourceId,
12811279
manifest_file: &Path,
12821280
gctx: &GlobalContext,
1283-
) -> CargoResult<(VirtualManifest, Vec<PathBuf>)> {
1281+
) -> CargoResult<VirtualManifest> {
12841282
let root = manifest_file.parent().unwrap();
12851283

12861284
if let Some(deps) = me
@@ -1302,7 +1300,6 @@ fn to_virtual_manifest(
13021300
bail!("this virtual manifest specifies a `{field}` section, which is not allowed");
13031301
}
13041302

1305-
let mut nested_paths = Vec::new();
13061303
let mut warnings = Vec::new();
13071304
let mut deps = Vec::new();
13081305
let empty = Vec::new();
@@ -1315,7 +1312,6 @@ fn to_virtual_manifest(
13151312
let mut manifest_ctx = ManifestContext {
13161313
deps: &mut deps,
13171314
source_id,
1318-
nested_paths: &mut nested_paths,
13191315
gctx,
13201316
warnings: &mut warnings,
13211317
platform: None,
@@ -1376,7 +1372,7 @@ fn to_virtual_manifest(
13761372
manifest.warnings_mut().add_warning(warning);
13771373
}
13781374

1379-
Ok((manifest, nested_paths))
1375+
Ok(manifest)
13801376
}
13811377

13821378
fn replace(
@@ -1467,7 +1463,6 @@ fn patch(
14671463
struct ManifestContext<'a, 'b> {
14681464
deps: &'a mut Vec<Dependency>,
14691465
source_id: SourceId,
1470-
nested_paths: &'a mut Vec<PathBuf>,
14711466
gctx: &'b GlobalContext,
14721467
warnings: &'a mut Vec<String>,
14731468
platform: Option<Platform>,
@@ -1563,7 +1558,7 @@ fn inheritable_from_path(
15631558
};
15641559

15651560
let source_id = SourceId::for_path(workspace_path_root)?;
1566-
let (man, _) = read_manifest(&workspace_path, source_id, gctx)?;
1561+
let man = read_manifest(&workspace_path, source_id, gctx)?;
15671562
match man.workspace_config() {
15681563
WorkspaceConfig::Root(root) => {
15691564
gctx.ws_roots
@@ -1885,7 +1880,6 @@ pub(crate) fn to_dependency<P: ResolveToPath + Clone>(
18851880
dep: &manifest::TomlDependency<P>,
18861881
name: &str,
18871882
source_id: SourceId,
1888-
nested_paths: &mut Vec<PathBuf>,
18891883
gctx: &GlobalContext,
18901884
warnings: &mut Vec<String>,
18911885
platform: Option<Platform>,
@@ -1899,7 +1893,6 @@ pub(crate) fn to_dependency<P: ResolveToPath + Clone>(
18991893
&mut ManifestContext {
19001894
deps: &mut Vec::new(),
19011895
source_id,
1902-
nested_paths,
19031896
gctx,
19041897
warnings,
19051898
platform,
@@ -2068,7 +2061,6 @@ fn detailed_dep_to_dependency<P: ResolveToPath + Clone>(
20682061
}
20692062
(None, Some(path), _, _) => {
20702063
let path = path.resolve(manifest_ctx.gctx);
2071-
manifest_ctx.nested_paths.push(path.clone());
20722064
// If the source ID for the package we're parsing is a path
20732065
// source, then we normalize the path here to get rid of
20742066
// components like `..`.

0 commit comments

Comments
 (0)