From bb1f6f29bc9b914fa5e98e67cce68866b43e30d1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Jun 2023 22:42:15 -0500 Subject: [PATCH] fix(embedded): Don't pollute script dir with lockfile This puts the lockfile back into a target directory in the users home, like before #12268. Another idea that came up was to move the workspace root to be in the target directory (which would effectively be like pre-#12268) but I think that is a bit hacky / misleading. This does mean that the lockfile is buried away from the user and they can't pass it along with their script. In most cases I've dealt with, this would be fine. When the lockfile is needed, they will also most likely have a workspace, so it shoud have a local lockfile in that case. The inbetween case is something that needs further evaluation for whether we should handle it and how. --- src/cargo/core/workspace.rs | 3 ++- src/cargo/ops/lockfile.rs | 35 ++++++++++++++++++++++++----------- tests/testsuite/script.rs | 2 +- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index c13163af8b9..db9c1801073 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1599,7 +1599,8 @@ impl MaybePackage { } } - fn is_embedded(&self) -> bool { + /// Has an embedded manifest (single-file package) + pub fn is_embedded(&self) -> bool { match self { MaybePackage::Package(p) => p.manifest().is_embedded(), MaybePackage::Virtual(_) => false, diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs index ece0701426a..b27a7e742ff 100644 --- a/src/cargo/ops/lockfile.rs +++ b/src/cargo/ops/lockfile.rs @@ -8,12 +8,12 @@ use crate::util::Filesystem; use anyhow::Context as _; pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult> { - if !ws.root().join("Cargo.lock").exists() { + let lock_root = lock_root(ws); + if !lock_root.as_path_unlocked().join("Cargo.lock").exists() { return Ok(None); } - let root = Filesystem::new(ws.root().to_path_buf()); - let mut f = root.open_ro("Cargo.lock", ws.config(), "Cargo.lock file")?; + let mut f = lock_root.open_ro("Cargo.lock", ws.config(), "Cargo.lock file")?; let mut s = String::new(); f.read_to_string(&mut s) @@ -30,12 +30,12 @@ pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult> { /// Generate a toml String of Cargo.lock from a Resolve. pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoResult { - let (_orig, out, _ws_root) = resolve_to_string_orig(ws, resolve); + let (_orig, out, _lock_root) = resolve_to_string_orig(ws, resolve); Ok(out) } pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoResult<()> { - let (orig, mut out, ws_root) = resolve_to_string_orig(ws, resolve); + let (orig, mut out, lock_root) = resolve_to_string_orig(ws, resolve); // If the lock file contents haven't changed so don't rewrite it. This is // helpful on read-only filesystems. @@ -55,7 +55,7 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoRes "the lock file {} needs to be updated but {} was passed to prevent this\n\ If you want to try to generate the lock file without accessing the network, \ remove the {} flag and use --offline instead.", - ws.root().to_path_buf().join("Cargo.lock").display(), + lock_root.as_path_unlocked().join("Cargo.lock").display(), flag, flag ); @@ -80,14 +80,19 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoRes } // Ok, if that didn't work just write it out - ws_root + lock_root .open_rw("Cargo.lock", ws.config(), "Cargo.lock file") .and_then(|mut f| { f.file().set_len(0)?; f.write_all(out.as_bytes())?; Ok(()) }) - .with_context(|| format!("failed to write {}", ws.root().join("Cargo.lock").display()))?; + .with_context(|| { + format!( + "failed to write {}", + lock_root.as_path_unlocked().join("Cargo.lock").display() + ) + })?; Ok(()) } @@ -96,15 +101,15 @@ fn resolve_to_string_orig( resolve: &mut Resolve, ) -> (Option, String, Filesystem) { // Load the original lock file if it exists. - let ws_root = Filesystem::new(ws.root().to_path_buf()); - let orig = ws_root.open_ro("Cargo.lock", ws.config(), "Cargo.lock file"); + let lock_root = lock_root(ws); + let orig = lock_root.open_ro("Cargo.lock", ws.config(), "Cargo.lock file"); let orig = orig.and_then(|mut f| { let mut s = String::new(); f.read_to_string(&mut s)?; Ok(s) }); let out = serialize_resolve(resolve, orig.as_deref().ok()); - (orig.ok(), out, ws_root) + (orig.ok(), out, lock_root) } fn serialize_resolve(resolve: &Resolve, orig: Option<&str>) -> String { @@ -235,3 +240,11 @@ fn emit_package(dep: &toml::Table, out: &mut String) { out.push_str(&format!("replace = {}\n\n", &dep["replace"])); } } + +fn lock_root(ws: &Workspace<'_>) -> Filesystem { + if ws.root_maybe().is_embedded() { + ws.target_dir() + } else { + Filesystem::new(ws.root().to_owned()) + } +} diff --git a/tests/testsuite/script.rs b/tests/testsuite/script.rs index 1eb01158be5..638677bc4b3 100644 --- a/tests/testsuite/script.rs +++ b/tests/testsuite/script.rs @@ -619,5 +619,5 @@ args: [] ) .run(); - assert!(local_lockfile_path.exists()); + assert!(!local_lockfile_path.exists()); }