Skip to content

Commit 015143c

Browse files
borsbors
committed
Auto merge of #10902 - epage:lock, r=ehuss
fix(add): Update the lock file This is done in the command, rather than in the op, - To consistently construct the `Workspace` - It is more composable as an API A downside is we update the git dependencies a second time and any sources we didn't initially update. Unlike the proposal in the attached issue, this does not roll back on error. - For some errors, the user might want to debug what went wrong. Losing the intermediate state makes that difficult - Rollback adds its own complications and risks, including since its non-atomic We've already tried to address most potential errors during `cargo add`s processing. To meet this desire, we now error if `--locked` is passed in and we would change the manifest. See that individual commit's message for more details. Fixes #10901
2 parents 85b500c + 5789c12 commit 015143c

File tree

56 files changed

+273
-32
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+273
-32
lines changed

‎src/bin/cargo/commands/add.rs

+7
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use cargo::ops::cargo_add::add;
77
use cargo::ops::cargo_add::AddOptions;
88
use cargo::ops::cargo_add::DepOp;
99
use cargo::ops::cargo_add::DepTable;
10+
use cargo::ops::resolve_ws;
1011
use cargo::util::command_prelude::*;
1112
use cargo::util::interning::InternedString;
1213
use cargo::CargoResult;
@@ -193,6 +194,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
193194
};
194195
add(&ws, &options)?;
195196

197+
if !dry_run {
198+
// Reload the workspace since we've changed dependencies
199+
let ws = args.workspace(config)?;
200+
resolve_ws(&ws)?;
201+
}
202+
196203
Ok(())
197204
}
198205

‎src/cargo/ops/cargo_add/manifest.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,7 @@ impl str::FromStr for Manifest {
238238

239239
impl std::fmt::Display for Manifest {
240240
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
241-
let s = self.data.to_string();
242-
s.fmt(f)
241+
self.data.fmt(f)
243242
}
244243
}
245244

@@ -433,6 +432,12 @@ impl LocalManifest {
433432
}
434433
}
435434

435+
impl std::fmt::Display for LocalManifest {
436+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
437+
self.manifest.fmt(f)
438+
}
439+
}
440+
436441
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
437442
enum DependencyStatus {
438443
None,

‎src/cargo/ops/cargo_add/mod.rs

+11
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
6262

6363
let manifest_path = options.spec.manifest_path().to_path_buf();
6464
let mut manifest = LocalManifest::try_new(&manifest_path)?;
65+
let original_raw_manifest = manifest.to_string();
6566
let legacy = manifest.get_legacy_sections();
6667
if !legacy.is_empty() {
6768
anyhow::bail!(
@@ -142,6 +143,16 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
142143
}
143144
}
144145

146+
if options.config.locked() {
147+
let new_raw_manifest = manifest.to_string();
148+
if original_raw_manifest != new_raw_manifest {
149+
anyhow::bail!(
150+
"the manifest file {} needs to be updated but --locked was passed to prevent this",
151+
manifest.path.display()
152+
);
153+
}
154+
}
155+
145156
if options.dry_run {
146157
options.config.shell().warn("aborting add due to dry run")?;
147158
} else {

‎tests/testsuite/cargo_add/build_prefer_existing_version/in/dependency/Cargo.toml

+4
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
[package]
44
name = "cargo-list-test-fixture-dependency"
55
version = "0.0.0"
6+
7+
[features]
8+
one = []
9+
two = []

‎tests/testsuite/cargo_add/build_prefer_existing_version/out/dependency/Cargo.toml

+4
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
[package]
44
name = "cargo-list-test-fixture-dependency"
55
version = "0.0.0"
6+
7+
[features]
8+
one = []
9+
two = []

‎tests/testsuite/cargo_add/build_prefer_existing_version/stderr.log

+3
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
11
Adding cargo-list-test-fixture-dependency (local) to build-dependencies.
2+
Features:
3+
- one
4+
- two

‎tests/testsuite/cargo_add/dev_prefer_existing_version/in/dependency/Cargo.toml

+4
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
[package]
44
name = "cargo-list-test-fixture-dependency"
55
version = "0.0.0"
6+
7+
[features]
8+
one = []
9+
two = []

‎tests/testsuite/cargo_add/dev_prefer_existing_version/out/dependency/Cargo.toml

+4
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
[package]
44
name = "cargo-list-test-fixture-dependency"
55
version = "0.0.0"
6+
7+
[features]
8+
one = []
9+
two = []

‎tests/testsuite/cargo_add/dev_prefer_existing_version/stderr.log

+3
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
11
Adding cargo-list-test-fixture-dependency (local) to dev-dependencies.
2+
Features:
3+
- one
4+
- two

‎tests/testsuite/cargo_add/git/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Adding git-package (git) to dependencies.
3+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/git_branch/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Adding git-package (git) to dependencies.
3+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/git_dev/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Adding git-package (git) to dev-dependencies.
3+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/git_inferred_name/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Updating git repository `[ROOTURL]/git-package`
33
Adding git-package (git) to dependencies.
4+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/git_multiple_names/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Adding my-package1 (git) to dependencies.
33
Adding my-package2 (git) to dependencies.
4+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/git_registry/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ fn git_registry() {
3232
])
3333
.current_dir(cwd)
3434
.assert()
35-
.success()
35+
.failure()
3636
.stdout_matches_path(curr_dir!().join("stdout.log"))
3737
.stderr_matches_path(curr_dir!().join("stderr.log"));
3838

‎tests/testsuite/cargo_add/git_registry/stderr.log

+4
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
Updating git repository `[ROOTURL]/versioned-package`
22
Adding versioned-package (git) to dependencies.
3+
error: failed to parse manifest at `[ROOT]/case/Cargo.toml`
4+
5+
Caused by:
6+
dependency (versioned-package) specification is ambiguous. Only one of `git` or `registry` is allowed.

‎tests/testsuite/cargo_add/git_rev/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Adding git-package (git) to dependencies.
3+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/git_tag/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
Updating git repository `[ROOTURL]/git-package`
22
Adding git-package (git) to dependencies.
3+
Updating git repository `[ROOTURL]/git-package`

‎tests/testsuite/cargo_add/list_features_path/in/dependency/Cargo.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ name = "your-face"
33
version = "0.1.3"
44

55
[dependencies]
6-
toml_edit = "0.1.5"
7-
atty = "0.2.13"
6+
my-package = "0.1.1"
87
optional-dependency = { path = "../optional", optional = true }
98

109
[features]

‎tests/testsuite/cargo_add/list_features_path/in/optional/Cargo.toml

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
[package]
2-
name = "optional-dep"
2+
name = "optional-dependency"
33
version = "0.1.3"
44

55
[dependencies]
6-
toml_edit = "0.1.5"
7-
atty = "0.2.13"
6+
my-package = "0.1.1"

‎tests/testsuite/cargo_add/list_features_path/out/dependency/Cargo.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ name = "your-face"
33
version = "0.1.3"
44

55
[dependencies]
6-
toml_edit = "0.1.5"
7-
atty = "0.2.13"
6+
my-package = "0.1.1"
87
optional-dependency = { path = "../optional", optional = true }
98

109
[features]

‎tests/testsuite/cargo_add/list_features_path/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@
44
+ nose
55
- eyes
66
- optional-dependency
7+
Updating `dummy-registry` index

‎tests/testsuite/cargo_add/list_features_path_no_default/in/dependency/Cargo.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ name = "your-face"
33
version = "0.1.3"
44

55
[dependencies]
6-
toml_edit = "0.1.5"
7-
atty = "0.2.13"
6+
my-package = "0.1.1"
87
optional-dependency = { path = "../optional", optional = true }
98

109
[features]

‎tests/testsuite/cargo_add/list_features_path_no_default/in/optional/Cargo.toml

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
[package]
2-
name = "optional-dep"
2+
name = "optional-dependency"
33
version = "0.1.3"
44

55
[dependencies]
6-
toml_edit = "0.1.5"
7-
atty = "0.2.13"
6+
my-package = "0.1.1"

‎tests/testsuite/cargo_add/list_features_path_no_default/out/dependency/Cargo.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ name = "your-face"
33
version = "0.1.3"
44

55
[dependencies]
6-
toml_edit = "0.1.5"
7-
atty = "0.2.13"
6+
my-package = "0.1.1"
87
optional-dependency = { path = "../optional", optional = true }
98

109
[features]

‎tests/testsuite/cargo_add/list_features_path_no_default/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@
44
- mouth
55
- nose
66
- optional-dependency
7+
Updating `dummy-registry` index

‎tests/testsuite/cargo_add/locked_changed/in

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../add-basic.in

‎tests/testsuite/cargo_add/locked_changed/mod.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::prelude::*;
3+
use cargo_test_support::Project;
4+
5+
use crate::cargo_add::init_registry;
6+
use cargo_test_support::curr_dir;
7+
8+
#[cargo_test]
9+
fn locked_changed() {
10+
init_registry();
11+
let project = Project::from_template(curr_dir!().join("in"));
12+
let project_root = project.root();
13+
let cwd = &project_root;
14+
15+
snapbox::cmd::Command::cargo_ui()
16+
.arg("add")
17+
.arg_line("my-package --locked")
18+
.current_dir(cwd)
19+
.assert()
20+
.failure()
21+
.stdout_matches_path(curr_dir!().join("stdout.log"))
22+
.stderr_matches_path(curr_dir!().join("stderr.log"));
23+
24+
assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
25+
}

‎tests/testsuite/cargo_add/locked_changed/out/Cargo.toml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[workspace]
2+
3+
[package]
4+
name = "cargo-list-test-fixture"
5+
version = "0.0.0"

‎tests/testsuite/cargo_add/locked_changed/stderr.log

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Updating `dummy-registry` index
2+
Adding my-package v99999.0.0 to dependencies.
3+
error: the manifest file [ROOT]/case/Cargo.toml needs to be updated but --locked was passed to prevent this

‎tests/testsuite/cargo_add/locked_changed/stdout.log

Whitespace-only changes.

‎tests/testsuite/cargo_add/locked_unchanged/in/Cargo.lock

+16
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/testsuite/cargo_add/locked_unchanged/in/Cargo.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[workspace]
2+
3+
[package]
4+
name = "cargo-list-test-fixture"
5+
version = "0.0.0"
6+
7+
[dependencies]
8+
my-package = "99999.0.0"

‎tests/testsuite/cargo_add/locked_unchanged/in/src/lib.rs

Whitespace-only changes.

‎tests/testsuite/cargo_add/locked_unchanged/mod.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::prelude::*;
3+
use cargo_test_support::Project;
4+
5+
use crate::cargo_add::init_registry;
6+
use cargo_test_support::curr_dir;
7+
8+
#[cargo_test]
9+
fn locked_unchanged() {
10+
init_registry();
11+
let project = Project::from_template(curr_dir!().join("in"));
12+
let project_root = project.root();
13+
let cwd = &project_root;
14+
15+
snapbox::cmd::Command::cargo_ui()
16+
.arg("add")
17+
.arg_line("my-package --locked")
18+
.current_dir(cwd)
19+
.assert()
20+
.success()
21+
.stdout_matches_path(curr_dir!().join("stdout.log"))
22+
.stderr_matches_path(curr_dir!().join("stderr.log"));
23+
24+
assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
25+
}

‎tests/testsuite/cargo_add/locked_unchanged/out/Cargo.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[workspace]
2+
3+
[package]
4+
name = "cargo-list-test-fixture"
5+
version = "0.0.0"
6+
7+
[dependencies]
8+
my-package = "99999.0.0"

‎tests/testsuite/cargo_add/locked_unchanged/stderr.log

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Updating `dummy-registry` index
2+
Adding my-package v99999.0.0 to dependencies.

‎tests/testsuite/cargo_add/locked_unchanged/stdout.log

Whitespace-only changes.

‎tests/testsuite/cargo_add/lockfile_updated/in/Cargo.lock

+17
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/testsuite/cargo_add/lockfile_updated/in/Cargo.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[workspace]
2+
3+
[package]
4+
name = "cargo-list-test-fixture"
5+
version = "0.0.0"
6+
7+
[dependencies]
8+
unrelateed-crate = "0.2.0"

‎tests/testsuite/cargo_add/lockfile_updated/in/src/lib.rs

Whitespace-only changes.

‎tests/testsuite/cargo_add/lockfile_updated/mod.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::prelude::*;
3+
use cargo_test_support::Project;
4+
5+
use crate::cargo_add::init_registry;
6+
use cargo_test_support::curr_dir;
7+
8+
#[cargo_test]
9+
fn lockfile_updated() {
10+
init_registry();
11+
let project = Project::from_template(curr_dir!().join("in"));
12+
let project_root = project.root();
13+
let cwd = &project_root;
14+
15+
snapbox::cmd::Command::cargo_ui()
16+
.arg("add")
17+
.arg_line("my-package")
18+
.current_dir(cwd)
19+
.assert()
20+
.success()
21+
.stdout_matches_path(curr_dir!().join("stdout.log"))
22+
.stderr_matches_path(curr_dir!().join("stderr.log"));
23+
24+
assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
25+
}

0 commit comments

Comments
 (0)