Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: lockfile path implies --locked on cargo install #14556

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/bin/cargo/commands/install.rs
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
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 rust_version(&self) -> Option<&RustVersion> {
Expand Down
51 changes: 39 additions & 12 deletions src/cargo/ops/cargo_install.rs
Ifropc marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
Ifropc marked this conversation as resolved.
Show resolved Hide resolved
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();

Ifropc marked this conversation as resolved.
Show resolved Hide resolved
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