Skip to content

Commit

Permalink
fix(add): Respect --locked
Browse files Browse the repository at this point in the history
This is prep for rust-lang#10901 to avoid the most common failure case for the
lock file.  We are assuming if the users action caused a change in the
manifes, then it will cause a change in the lock file.  This isn't
entirely true but close enough and I think these semantics can make
sense.
  • Loading branch information
epage authored and Hezuikn committed Sep 22, 2022
1 parent df3a2bb commit 32f80eb
Show file tree
Hide file tree
Showing 14 changed files with 97 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/cargo/ops/cargo_add/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,7 @@ impl str::FromStr for Manifest {

impl std::fmt::Display for Manifest {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let s = self.data.to_string();
s.fmt(f)
self.data.fmt(f)
}
}

Expand Down Expand Up @@ -433,6 +432,12 @@ impl LocalManifest {
}
}

impl std::fmt::Display for LocalManifest {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.manifest.fmt(f)
}
}

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
enum DependencyStatus {
None,
Expand Down
11 changes: 11 additions & 0 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(

let manifest_path = options.spec.manifest_path().to_path_buf();
let mut manifest = LocalManifest::try_new(&manifest_path)?;
let original_raw_manifest = manifest.to_string();
let legacy = manifest.get_legacy_sections();
if !legacy.is_empty() {
anyhow::bail!(
Expand Down Expand Up @@ -142,6 +143,16 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
}
}

if options.config.locked() {
let new_raw_manifest = manifest.to_string();
if original_raw_manifest != new_raw_manifest {
anyhow::bail!(
"the manifest file {} needs to be updated but --locked was passed to prevent this",
manifest.path.display()
);
}
}

if options.dry_run {
options.config.shell().warn("aborting add due to dry run")?;
} else {
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/locked_changed/in
25 changes: 25 additions & 0 deletions tests/testsuite/cargo_add/locked_changed/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

use crate::cargo_add::init_registry;
use cargo_test_support::curr_dir;

#[cargo_test]
fn locked_changed() {
init_registry();
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("my-package --locked")
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
5 changes: 5 additions & 0 deletions tests/testsuite/cargo_add/locked_changed/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_add/locked_changed/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Updating `dummy-registry` index
Adding my-package v99999.0.0 to dependencies.
error: the manifest file [ROOT]/case/Cargo.toml needs to be updated but --locked was passed to prevent this
Empty file.
8 changes: 8 additions & 0 deletions tests/testsuite/cargo_add/locked_unchanged/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"

[dependencies]
my-package = "99999.0.0"
Empty file.
25 changes: 25 additions & 0 deletions tests/testsuite/cargo_add/locked_unchanged/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

use crate::cargo_add::init_registry;
use cargo_test_support::curr_dir;

#[cargo_test]
fn locked_unchanged() {
init_registry();
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("my-package --locked")
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
8 changes: 8 additions & 0 deletions tests/testsuite/cargo_add/locked_unchanged/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"

[dependencies]
my-package = "99999.0.0"
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_add/locked_unchanged/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Updating `dummy-registry` index
Adding my-package v99999.0.0 to dependencies.
Empty file.
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ mod invalid_vers;
mod list_features;
mod list_features_path;
mod list_features_path_no_default;
mod locked_changed;
mod locked_unchanged;
mod manifest_path_package;
mod merge_activated_features;
mod multiple_conflicts_with_features;
Expand Down

0 comments on commit 32f80eb

Please sign in to comment.