Skip to content

Commit

Permalink
fix(embedded): Don't pollute script dir with lockfile
Browse files Browse the repository at this point in the history
This puts the lockfile back into a target directory in the users home,
like before rust-lang#12268.

Another idea that came up was to move the workspace root to be in the
target directory (which would effectively be like pre-rust-lang#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.
  • Loading branch information
epage committed Jun 17, 2023
1 parent 53a63b2 commit bb1f6f2
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 13 deletions.
3 changes: 2 additions & 1 deletion src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 24 additions & 11 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ use crate::util::Filesystem;
use anyhow::Context as _;

pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {
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)
Expand All @@ -30,12 +30,12 @@ pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {

/// Generate a toml String of Cargo.lock from a Resolve.
pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoResult<String> {
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.
Expand All @@ -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
);
Expand All @@ -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(())
}

Expand All @@ -96,15 +101,15 @@ fn resolve_to_string_orig(
resolve: &mut Resolve,
) -> (Option<String>, 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 {
Expand Down Expand Up @@ -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())
}
}
2 changes: 1 addition & 1 deletion tests/testsuite/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,5 +619,5 @@ args: []
)
.run();

assert!(local_lockfile_path.exists());
assert!(!local_lockfile_path.exists());
}

0 comments on commit bb1f6f2

Please sign in to comment.