Skip to content

Commit

Permalink
Auto merge of #14326 - Ifropc:5707-lock-path, r=weihanglo
Browse files Browse the repository at this point in the history
Add `--lockfile-path` flag

This change implements a new `--lockfile-path` proposed in #5707 .

Functionality added:
- Add `--lockfile-path <PATH>` to all commands that support `manifest-path` with exception of:
   - `locate-project` (doesn't use lock file)
   - `verify-project` (is deprecated)
   - `read-manifest` (doesn't use lock file)
- Behind -Zunstable-options and docs
   - The flag's docs / `--help` has (unstable) in them
- `<PATH>` must end with `Cargo.lock`. If specified path doesn't exist (or parent director(ies), create all the parent directories and the lockfile itself

Implementation TLDR: add `requested_lockfile_path` into `Workspace` and set it on `workspace(gctx)` call (setting from the context)
Update `lockfile.lock_root()` to respect `requested_lockfile_path` (if set)
Add test cases covering all affected commands. Tested creating lockfile, reading lockfile, overriding default (`./Cargo.lock`) lockfile, symlink tests. Extra tests for package to make sure pinned versions from path's lockfile are respected (i.e. double check correct lockfile is used)
I doubt this flag will be used for any command that's not read-only, but I tried to cover all the commands.
  • Loading branch information
bors committed Aug 16, 2024
2 parents 9a170d7 + 04f9e2d commit 3293d22
Show file tree
Hide file tree
Showing 134 changed files with 1,650 additions and 224 deletions.
1 change: 1 addition & 0 deletions src/bin/cargo/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ Example uses:
- Depend on crates with the same name from different registries"),
])
.arg_manifest_path_without_unsupported_path_tip()
.arg_lockfile_path()
.arg_package("Package to modify")
.arg_ignore_rust_version()
.arg_dry_run("Don't actually write the manifest")
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help bench</>` for more detailed information.\n"
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help build</>` for more detailed information.\n"
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help check</>` for more detailed information.\n"
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub fn cli() -> Command {
.arg_target_triple("Target triple to clean output for")
.arg_target_dir()
.arg_manifest_path()
.arg_lockfile_path()
.arg_dry_run("Display what would be deleted without deleting anything")
.args_conflicts_with_subcommands(true)
.subcommand(
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help doc</>` for more detailed information.\n"
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub fn cli() -> Command {
.arg_silent_suggestion()
.arg_target_triple("Fetch dependencies for the target triple")
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help fetch</>` for more detailed information.\n"
))
Expand Down
9 changes: 8 additions & 1 deletion src/bin/cargo/commands/fix.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::command_prelude::*;

use cargo::core::Workspace;

use cargo::ops;

pub fn cli() -> Command {
Expand Down Expand Up @@ -54,6 +54,7 @@ pub fn cli() -> Command {
.arg_target_dir()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help fix</>` for more detailed information.\n"
Expand All @@ -71,8 +72,13 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
// Unlike other commands default `cargo fix` to all targets to fix as much
// code as we can.
let root_manifest = args.root_manifest(gctx)?;

// Can't use workspace() to avoid using -Zavoid-dev-deps (if passed)
let mut ws = Workspace::new(&root_manifest, gctx)?;
ws.set_resolve_honors_rust_version(args.honor_rust_version());
let lockfile_path = args.lockfile_path(gctx)?;
ws.set_requested_lockfile_path(lockfile_path.clone());

let mut opts = args.compile_options(gctx, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?;

if !opts.filter.is_specific() {
Expand All @@ -92,6 +98,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
allow_no_vcs: args.flag("allow-no-vcs"),
allow_staged: args.flag("allow-staged"),
broken_code: args.flag("broken-code"),
requested_lockfile_path: lockfile_path,
},
)?;
Ok(())
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub fn cli() -> Command {
.about("Generate the lockfile for a package")
.arg_silent_suggestion()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version_with_help(
"Ignore `rust-version` specification in packages (unstable)",
)
Expand Down
4 changes: 3 additions & 1 deletion src/bin/cargo/commands/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::command_prelude::*;
use cargo::ops::{self, OutputMetadataOptions};

use crate::command_prelude::*;

pub fn cli() -> Command {
subcommand("metadata")
.about(
Expand All @@ -26,6 +27,7 @@ pub fn cli() -> Command {
.arg_silent_suggestion()
.arg_features()
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help metadata</>` for more detailed information.\n"
))
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub fn cli() -> Command {
.arg_target_dir()
.arg_parallel()
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help package</>` for more detailed information.\n"
))
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub fn cli() -> Command {
.arg_silent_suggestion()
.arg_package("Argument to get the package ID specifier for")
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help pkgid</>` for more detailed information.\n"
))
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub fn cli() -> Command {
.arg_target_triple("Build for the target triple")
.arg_target_dir()
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help publish</>` for more detailed information.\n"
))
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub fn cli() -> clap::Command {
])
.arg_package("Package to remove from")
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help remove</>` for more detailed information.\n"
))
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub fn cli() -> Command {
.arg_target_triple("Build for the target triple")
.arg_target_dir()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.arg_unit_graph()
.arg_timings()
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help rustc</>` for more detailed information.\n"
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help rustdoc</>` for more detailed information.\n"
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ pub fn cli() -> Command {
.arg_unit_graph()
.arg_timings()
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help test</>` for more detailed information.\n\
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub fn cli() -> Command {
Pass `all` to include all targets.",
)
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help tree</>` for more detailed information.\n"
))
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub fn cli() -> Command {
.help_heading(heading::PACKAGE_SELECTION),
)
.arg_manifest_path()
.arg_lockfile_path()
.arg_ignore_rust_version_with_help(
"Ignore `rust-version` specification in packages (unstable)",
)
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub fn cli() -> Command {
.arg(unsupported("only-git-deps"))
.arg(unsupported("disallow-duplicates"))
.arg_manifest_path()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help vendor</>` for more detailed information.\n"
))
Expand Down
29 changes: 29 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ pub struct Workspace<'gctx> {
/// file. This is set for `cargo install` without `--locked`.
ignore_lock: bool,

/// Requested path of the lockfile (i.e. passed as the cli flag)
requested_lockfile_path: Option<PathBuf>,

/// The resolver behavior specified with the `resolver` field.
resolve_behavior: ResolveBehavior,
/// If `true`, then workspace `rust_version` would be used in `cargo resolve`
Expand Down Expand Up @@ -242,6 +245,7 @@ impl<'gctx> Workspace<'gctx> {
require_optional_deps: true,
loaded_packages: RefCell::new(HashMap::new()),
ignore_lock: false,
requested_lockfile_path: None,
resolve_behavior: ResolveBehavior::V1,
resolve_honors_rust_version: false,
custom_metadata: None,
Expand Down Expand Up @@ -633,6 +637,31 @@ impl<'gctx> Workspace<'gctx> {
self
}

/// Returns the directory where the lockfile is in.
pub fn lock_root(&self) -> Filesystem {
if let Some(requested) = self.requested_lockfile_path.as_ref() {
return Filesystem::new(
requested
.parent()
.expect("Lockfile path can't be root")
.to_owned(),
);
}
self.default_lock_root()
}

fn default_lock_root(&self) -> Filesystem {
if self.root_maybe().is_embedded() {
self.target_dir()
} else {
Filesystem::new(self.root().to_owned())
}
}

pub fn set_requested_lockfile_path(&mut self, path: Option<PathBuf>) {
self.requested_lockfile_path = path;
}

/// Get the lowest-common denominator `package.rust-version` within the workspace, if specified
/// anywhere
pub fn rust_version(&self) -> Option<&RustVersion> {
Expand Down
11 changes: 8 additions & 3 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::task::Poll;

use super::RegistryOrIndex;
use crate::core::compiler::{BuildConfig, CompileMode, DefaultExecutor, Executor};
use crate::core::dependency::DepKind;
use crate::core::manifest::Target;
use crate::core::resolver::CliFeatures;
use crate::core::resolver::HasDevUnits;
use crate::core::{Feature, PackageIdSpecQuery, Shell, Verbosity, Workspace};
use crate::core::{Package, PackageId, PackageSet, Resolve, SourceId};
use crate::ops::lockfile::LOCKFILE_NAME;
use crate::sources::registry::index::{IndexPackage, RegistryDependency};
use crate::sources::{PathSource, SourceConfigMap, CRATES_IO_REGISTRY};
use crate::util::cache_lock::CacheLockMode;
Expand All @@ -32,8 +34,6 @@ use tar::{Archive, Builder, EntryType, Header, HeaderMode};
use tracing::debug;
use unicase::Ascii as UncasedAscii;

use super::RegistryOrIndex;

#[derive(Clone)]
pub struct PackageOpts<'gctx> {
pub gctx: &'gctx GlobalContext,
Expand Down Expand Up @@ -280,7 +280,12 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Vec<Fi
}
let pkgs = ws.members_with_features(specs, &opts.cli_features)?;

if ws.root().join("Cargo.lock").exists() {
if ws
.lock_root()
.as_path_unlocked()
.join(LOCKFILE_NAME)
.exists()
{
// Make sure the Cargo.lock is up-to-date and valid.
let dry_run = false;
let _ = ops::resolve_ws(ws, dry_run)?;
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub struct FixOptions {
pub allow_no_vcs: bool,
pub allow_staged: bool,
pub broken_code: bool,
pub requested_lockfile_path: Option<PathBuf>,
}

pub fn fix(
Expand All @@ -121,6 +122,7 @@ pub fn fix(
}
let mut ws = Workspace::new(&root_manifest, gctx)?;
ws.set_resolve_honors_rust_version(Some(original_ws.resolve_honors_rust_version()));
ws.set_requested_lockfile_path(opts.requested_lockfile_path.clone());

// Spin up our lock server, which our subprocesses will use to synchronize fixes.
let lock_server = LockServer::new()?;
Expand Down
30 changes: 14 additions & 16 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ use crate::util::Filesystem;

use anyhow::Context as _;

pub const LOCKFILE_NAME: &str = "Cargo.lock";

#[tracing::instrument(skip_all)]
pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {
let lock_root = lock_root(ws);
if !lock_root.as_path_unlocked().join("Cargo.lock").exists() {
let lock_root = ws.lock_root();
if !lock_root.as_path_unlocked().join(LOCKFILE_NAME).exists() {
return Ok(None);
}

let mut f = lock_root.open_ro_shared("Cargo.lock", ws.gctx(), "Cargo.lock file")?;
let mut f = lock_root.open_ro_shared(LOCKFILE_NAME, ws.gctx(), "Cargo.lock file")?;

let mut s = String::new();
f.read_to_string(&mut s)
Expand Down Expand Up @@ -58,7 +60,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.",
lock_root.as_path_unlocked().join("Cargo.lock").display(),
lock_root.as_path_unlocked().join(LOCKFILE_NAME).display(),
flag,
flag
);
Expand All @@ -82,9 +84,13 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoRes
anyhow::bail!("lock file version `{current_version:?}` requires `-Znext-lockfile-bump`")
}

if !lock_root.as_path_unlocked().exists() {
lock_root.create_dir()?;
}

// Ok, if that didn't work just write it out
lock_root
.open_rw_exclusive_create("Cargo.lock", ws.gctx(), "Cargo.lock file")
.open_rw_exclusive_create(LOCKFILE_NAME, ws.gctx(), "Cargo.lock file")
.and_then(|mut f| {
f.file().set_len(0)?;
f.write_all(out.as_bytes())?;
Expand All @@ -93,7 +99,7 @@ pub fn write_pkg_lockfile(ws: &Workspace<'_>, resolve: &mut Resolve) -> CargoRes
.with_context(|| {
format!(
"failed to write {}",
lock_root.as_path_unlocked().join("Cargo.lock").display()
lock_root.as_path_unlocked().join(LOCKFILE_NAME).display()
)
})?;
Ok(true)
Expand All @@ -104,8 +110,8 @@ fn resolve_to_string_orig(
resolve: &Resolve,
) -> (Option<String>, String, Filesystem) {
// Load the original lock file if it exists.
let lock_root = lock_root(ws);
let orig = lock_root.open_ro_shared("Cargo.lock", ws.gctx(), "Cargo.lock file");
let lock_root = ws.lock_root();
let orig = lock_root.open_ro_shared(LOCKFILE_NAME, ws.gctx(), "Cargo.lock file");
let orig = orig.and_then(|mut f| {
let mut s = String::new();
f.read_to_string(&mut s)?;
Expand Down Expand Up @@ -245,11 +251,3 @@ 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())
}
}
Loading

0 comments on commit 3293d22

Please sign in to comment.