Skip to content

Commit

Permalink
Auto merge of #14556 - Ifropc:lockfile-path-install, r=weihanglo
Browse files Browse the repository at this point in the history 
feat: lockfile path implies --locked on cargo install

Follow-up of #14421
Resolving one of the items
> cargo install should make --lockfile-path imply --locked

Simply mirrored behavior as if `--locked` was provided (on creating the workspace)
  • Loading branch information
@bors
bors committed Sep 27, 2024
2 parents b396f2c + bf37cf7 commit 1fad770
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 41 deletions.
9 changes: 9 additions & 0 deletions src/bin/cargo/commands/install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ pub fn cli() -> Command {
.arg_target_triple("Build for the target triple")
.arg_target_dir()
.arg_timings()
.arg_lockfile_path()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help install</>` for more detailed information.\n"
))
Expand Down Expand Up @@ -204,6 +205,13 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
if args.dry_run() {
gctx.cli_unstable().fail_if_stable_opt("--dry-run", 11123)?;
}

let requested_lockfile_path = args.lockfile_path(gctx)?;
// 14421: lockfile path should imply --locked on running `install`
if requested_lockfile_path.is_some() {
gctx.set_locked(true);
}

if args.flag("list") {
ops::install_list(root, gctx)?;
} else {
Expand All @@ -217,6 +225,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
args.flag("force"),
args.flag("no-track"),
args.dry_run(),
requested_lockfile_path.as_deref(),
)?;
}
Ok(())
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/core/workspace.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,10 @@ impl<'gctx> Workspace<'gctx> {
self.requested_lockfile_path = path;
}

pub fn requested_lockfile_path(&self) -> Option<&Path> {
self.requested_lockfile_path.as_deref()
}

/// Get the lowest-common denominator `package.rust-version` within the workspace, if specified
/// anywhere
pub fn lowest_rust_version(&self) -> Option<&RustVersion> {
Expand Down
51 changes: 39 additions & 12 deletions src/cargo/ops/cargo_install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ struct InstallablePackage<'gctx> {
vers: Option<VersionReq>,
force: bool,
no_track: bool,

pkg: Package,
ws: Workspace<'gctx>,
rustc: Rustc,
Expand All @@ -68,6 +67,7 @@ impl<'gctx> InstallablePackage<'gctx> {
no_track: bool,
needs_update_if_source_is_index: bool,
current_rust_version: Option<&PartialVersion>,
lockfile_path: Option<&Path>,
) -> CargoResult<Option<Self>> {
if let Some(name) = krate {
if name == "." {
Expand Down Expand Up @@ -155,6 +155,7 @@ impl<'gctx> InstallablePackage<'gctx> {
&root,
&dst,
force,
lockfile_path,
) {
let msg = format!(
"package `{}` is already installed, use --force to override",
Expand All @@ -179,15 +180,32 @@ impl<'gctx> InstallablePackage<'gctx> {
}
};

let (ws, rustc, target) =
make_ws_rustc_target(gctx, &original_opts, &source_id, pkg.clone())?;
// If we're installing in --locked mode and there's no `Cargo.lock` published
// ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026
if gctx.locked() && !ws.root().join("Cargo.lock").exists() {
gctx.shell().warn(format!(
"no Cargo.lock file published in {}",
pkg.to_string()
))?;
let (ws, rustc, target) = make_ws_rustc_target(
gctx,
&original_opts,
&source_id,
pkg.clone(),
lockfile_path.clone(),
)?;

if gctx.locked() {
// When --lockfile-path is set, check that passed lock file exists
// (unlike the usual flag behavior, lockfile won't be created as we imply --locked)
if let Some(requested_lockfile_path) = ws.requested_lockfile_path() {
if !requested_lockfile_path.is_file() {
bail!(
"no Cargo.lock file found in the requested path {}",
requested_lockfile_path.display()
);
}
// If we're installing in --locked mode and there's no `Cargo.lock` published
// ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026
} else if !ws.root().join("Cargo.lock").exists() {
gctx.shell().warn(format!(
"no Cargo.lock file published in {}",
pkg.to_string()
))?;
}
}
let pkg = if source_id.is_git() {
// Don't use ws.current() in order to keep the package source as a git source so that
Expand Down Expand Up @@ -246,7 +264,6 @@ impl<'gctx> InstallablePackage<'gctx> {
vers: vers.cloned(),
force,
no_track,

pkg,
ws,
rustc,
Expand Down Expand Up @@ -636,6 +653,7 @@ pub fn install(
force: bool,
no_track: bool,
dry_run: bool,
lockfile_path: Option<&Path>,
) -> CargoResult<()> {
let root = resolve_root(root, gctx)?;
let dst = root.join("bin").into_path_unlocked();
Expand Down Expand Up @@ -667,6 +685,7 @@ pub fn install(
no_track,
true,
current_rust_version.as_ref(),
lockfile_path,
)?;
let mut installed_anything = true;
if let Some(installable_pkg) = installable_pkg {
Expand Down Expand Up @@ -698,6 +717,7 @@ pub fn install(
no_track,
!did_update,
current_rust_version.as_ref(),
lockfile_path,
) {
Ok(Some(installable_pkg)) => {
did_update = true;
Expand Down Expand Up @@ -804,6 +824,7 @@ fn installed_exact_package<T>(
root: &Filesystem,
dst: &Path,
force: bool,
lockfile_path: Option<&Path>,
) -> CargoResult<Option<Package>>
where
T: Source,
Expand All @@ -819,7 +840,7 @@ where
// best-effort check to see if we can avoid hitting the network.
if let Ok(pkg) = select_dep_pkg(source, dep, gctx, false, None) {
let (_ws, rustc, target) =
make_ws_rustc_target(gctx, opts, &source.source_id(), pkg.clone())?;
make_ws_rustc_target(gctx, opts, &source.source_id(), pkg.clone(), lockfile_path)?;
if let Ok(true) = is_installed(&pkg, gctx, opts, &rustc, &target, root, dst, force) {
return Ok(Some(pkg));
}
Expand All @@ -832,6 +853,7 @@ fn make_ws_rustc_target<'gctx>(
opts: &ops::CompileOptions,
source_id: &SourceId,
pkg: Package,
lockfile_path: Option<&Path>,
) -> CargoResult<(Workspace<'gctx>, Rustc, String)> {
let mut ws = if source_id.is_git() || source_id.is_path() {
Workspace::new(pkg.manifest_path(), gctx)?
Expand All @@ -841,6 +863,11 @@ fn make_ws_rustc_target<'gctx>(
ws
};
ws.set_ignore_lock(gctx.lock_update_allowed());
ws.set_requested_lockfile_path(lockfile_path.map(|p| p.to_path_buf()));
// if --lockfile-path is set, imply --locked
if ws.requested_lockfile_path().is_some() {
ws.set_ignore_lock(false);
}
ws.set_require_optional_deps(false);

let rustc = gctx.load_global_rustc(Some(&ws))?;
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/util/context/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,10 @@ impl GlobalContext {
self.locked
}

pub fn set_locked(&mut self, locked: bool) {
self.locked = locked;
}

pub fn lock_update_allowed(&self) -> bool {
!self.frozen && !self.locked
}
Expand Down
56 changes: 29 additions & 27 deletions tests/testsuite/cargo_install/help/stdout.term.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
98 changes: 96 additions & 2 deletions tests/testsuite/lockfile_path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ use std::fs;
use snapbox::str;

use cargo_test_support::compare::assert_e2e;
use cargo_test_support::install::assert_has_installed_exe;
use cargo_test_support::registry::{Package, RegistryBuilder};
use cargo_test_support::{
basic_bin_manifest, cargo_test, project, symlink_supported, ProjectBuilder,
basic_bin_manifest, cargo_process, cargo_test, paths, project, symlink_supported,
ProjectBuilder,
};

///////////////////////////////
//// Unstable feature tests start
///////////////////////////////
Expand Down Expand Up @@ -400,6 +401,99 @@ bar = "0.1.0"
assert_e2e().eq(contents, lockfile_original);
}

#[cargo_test]
fn install_respects_lock_file_path() {
// `cargo install` will imply --locked when lockfile path is provided
Package::new("bar", "0.1.0").publish();
Package::new("bar", "0.1.1")
.file("src/lib.rs", "not rust")
.publish();
// Publish with lockfile containing bad version of `bar` (0.1.1)
Package::new("foo", "0.1.0")
.dep("bar", "0.1")
.file("src/lib.rs", "")
.file(
"src/main.rs",
"extern crate foo; extern crate bar; fn main() {}",
)
.file(
"Cargo.lock",
r#"
[[package]]
name = "bar"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "foo"
version = "0.1.0"
dependencies = [
"bar 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
]
"#,
)
.publish();

cargo_process("install foo --locked")
.with_stderr_data(str![[r#"
...
[..]not rust[..]
...
"#]])
.with_status(101)
.run();

// Create lockfile with the good `bar` version (0.1.0) and use it for install
project()
.file(
"Cargo.lock",
r#"
[[package]]
name = "bar"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "foo"
version = "0.1.0"
dependencies = [
"bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
"#,
)
.build();
cargo_process("install foo -Zunstable-options --lockfile-path foo/Cargo.lock")
.masquerade_as_nightly_cargo(&["lockfile-path"])
.run();

assert!(paths::root().join("foo/Cargo.lock").is_file());
assert_has_installed_exe(paths::cargo_home(), "foo");
}

#[cargo_test]
fn install_lock_file_path_must_present() {
// `cargo install` will imply --locked when lockfile path is provided
Package::new("bar", "0.1.0").publish();
Package::new("foo", "0.1.0")
.dep("bar", "0.1")
.file("src/lib.rs", "")
.file(
"src/main.rs",
"extern crate foo; extern crate bar; fn main() {}",
)
.publish();

cargo_process("install foo -Zunstable-options --lockfile-path lockfile_dir/Cargo.lock")
.masquerade_as_nightly_cargo(&["lockfile-path"])
.with_stderr_data(str![[r#"
...
[ERROR] no Cargo.lock file found in the requested path [ROOT]/lockfile_dir/Cargo.lock
...
"#]])
.with_status(101)
.run();
}

#[cargo_test]
fn run_embed() {
let lockfile_path = "mylockfile/Cargo.lock";
Expand Down

0 comments on commit 1fad770

Please sign in to comment.