From 592eb56c987cc647f2f2aeb4a48eb82771930dfd Mon Sep 17 00:00:00 2001 From: cassaundra Date: Mon, 20 Jun 2022 00:13:57 -0700 Subject: [PATCH 1/6] Add `--target` flag to rm --- README.md | 7 ++- src/bin/add/add.rs | 2 +- src/bin/rm/rm.rs | 54 ++++++++++++++++--- src/manifest.rs | 35 ++++++------ tests/cmd/rm/invalid_rm_target.in | 1 + tests/cmd/rm/invalid_rm_target.out/Cargo.toml | 35 ++++++++++++ tests/cmd/rm/invalid_rm_target.toml | 15 ++++++ tests/cmd/rm/invalid_rm_target_dep.in | 1 + .../rm/invalid_rm_target_dep.out/Cargo.toml | 35 ++++++++++++ tests/cmd/rm/invalid_rm_target_dep.toml | 15 ++++++ tests/cmd/rm/rm_target.in/Cargo.toml | 35 ++++++++++++ tests/cmd/rm/rm_target.out/Cargo.toml | 32 +++++++++++ tests/cmd/rm/rm_target.toml | 11 ++++ tests/cmd/rm/rm_target_build.in | 1 + tests/cmd/rm/rm_target_build.out/Cargo.toml | 32 +++++++++++ tests/cmd/rm/rm_target_build.toml | 11 ++++ tests/cmd/rm/rm_target_dev.in | 1 + tests/cmd/rm/rm_target_dev.out/Cargo.toml | 32 +++++++++++ tests/cmd/rm/rm_target_dev.toml | 11 ++++ 19 files changed, 339 insertions(+), 27 deletions(-) create mode 120000 tests/cmd/rm/invalid_rm_target.in create mode 100644 tests/cmd/rm/invalid_rm_target.out/Cargo.toml create mode 100644 tests/cmd/rm/invalid_rm_target.toml create mode 120000 tests/cmd/rm/invalid_rm_target_dep.in create mode 100644 tests/cmd/rm/invalid_rm_target_dep.out/Cargo.toml create mode 100644 tests/cmd/rm/invalid_rm_target_dep.toml create mode 100644 tests/cmd/rm/rm_target.in/Cargo.toml create mode 100644 tests/cmd/rm/rm_target.out/Cargo.toml create mode 100644 tests/cmd/rm/rm_target.toml create mode 120000 tests/cmd/rm/rm_target_build.in create mode 100644 tests/cmd/rm/rm_target_build.out/Cargo.toml create mode 100644 tests/cmd/rm/rm_target_build.toml create mode 120000 tests/cmd/rm/rm_target_dev.in create mode 100644 tests/cmd/rm/rm_target_dev.out/Cargo.toml create mode 100644 tests/cmd/rm/rm_target_dev.toml diff --git a/README.md b/README.md index 9a334ddf9c..66236d9d7a 100644 --- a/README.md +++ b/README.md @@ -156,8 +156,6 @@ ARGS: ... Crates to be removed OPTIONS: - -B, --build Remove crate as build dependency - -D, --dev Remove crate as development dependency -h, --help Print help information --manifest-path Path to the manifest to remove a dependency from -p, --package Package id of the crate to remove this dependency from @@ -165,6 +163,11 @@ OPTIONS: -V, --version Print version information -Z Unstable (nightly-only) flags +SECTION: + -B, --build Remove crate as build dependency + -D, --dev Remove crate as development dependency + --target Remove as dependency from the given target platform + ``` ### `cargo upgrade` diff --git a/src/bin/add/add.rs b/src/bin/add/add.rs index 7347225570..a6e294f4ee 100644 --- a/src/bin/add/add.rs +++ b/src/bin/add/add.rs @@ -618,7 +618,7 @@ fn exec(mut args: AddArgs) -> CargoResult<()> { if was_sorted { if let Some(table) = manifest - .get_table_mut(&args.get_section()) + .get_table_mut(&args.get_section(), true) .ok() .and_then(TomlItem::as_table_like_mut) { diff --git a/src/bin/rm/rm.rs b/src/bin/rm/rm.rs index 1a0572009c..6134fa0804 100644 --- a/src/bin/rm/rm.rs +++ b/src/bin/rm/rm.rs @@ -15,13 +15,34 @@ pub struct RmArgs { crates: Vec, /// Remove crate as development dependency. - #[clap(long, short = 'D', conflicts_with = "build")] + #[clap( + long, + short = 'D', + conflicts_with = "build", + help_heading = "SECTION", + group = "SECTION" + )] dev: bool, /// Remove crate as build dependency. - #[clap(long, short = 'B', conflicts_with = "dev")] + #[clap( + long, + short = 'B', + conflicts_with = "dev", + help_heading = "SECTION", + group = "SECTION" + )] build: bool, + /// Remove as dependency from the given target platform. + #[clap( + long, + forbid_empty_values = true, + help_heading = "SECTION", + group = "section" + )] + target: Option, + /// Path to the manifest to remove a dependency from. #[clap( long, @@ -54,14 +75,26 @@ impl RmArgs { exec(self) } - /// Get depenency section - pub fn get_section(&self) -> &'static str { - if self.dev { + /// Get dependency section + pub fn get_section(&self) -> Vec { + let section_name = if self.dev { "dev-dependencies" } else if self.build { "build-dependencies" } else { "dependencies" + }; + + if let Some(ref target) = self.target { + assert!(!target.is_empty(), "Target specification may not be empty"); + + vec![ + "target".to_owned(), + target.clone(), + section_name.to_owned(), + ] + } else { + vec![section_name.to_owned()] } } } @@ -69,12 +102,17 @@ impl RmArgs { #[derive(Copy, Clone, Debug, PartialEq, Eq, clap::ArgEnum)] enum UnstableOptions {} -fn print_msg(name: &str, section: &str) -> CargoResult<()> { +fn print_msg(name: &str, section: &[String]) -> CargoResult<()> { let colorchoice = colorize_stderr(); let mut output = StandardStream::stderr(colorchoice); output.set_color(ColorSpec::new().set_fg(Some(Color::Green)).set_bold(true))?; write!(output, "{:>12}", "Removing")?; output.reset()?; + let section = if section.len() == 1 { + section[0].clone() + } else { + format!("{} for target `{}`", §ion[2], §ion[1]) + }; writeln!(output, " {} from {}", name, section)?; Ok(()) } @@ -92,10 +130,10 @@ fn exec(args: &RmArgs) -> CargoResult<()> { deps.iter() .map(|dep| { if !args.quiet { - print_msg(dep, args.get_section())?; + print_msg(dep, &args.get_section())?; } let result = manifest - .remove_from_table(args.get_section(), dep) + .remove_from_table(&args.get_section(), dep) .map_err(Into::into); // Now that we have removed the crate, if that was the last reference to that crate, diff --git a/src/manifest.rs b/src/manifest.rs index 9917613958..34d9bd884e 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -60,17 +60,23 @@ impl Manifest { pub fn get_table_mut<'a>( &'a mut self, table_path: &[String], + insert_if_not_exists: bool, ) -> CargoResult<&'a mut toml_edit::Item> { /// Descend into a manifest until the required table is found. fn descend<'a>( input: &'a mut toml_edit::Item, path: &[String], + insert_if_not_exists: bool, ) -> CargoResult<&'a mut toml_edit::Item> { if let Some(segment) = path.get(0) { - let value = input[&segment].or_insert(toml_edit::table()); + let value = if insert_if_not_exists { + input[&segment].or_insert(toml_edit::table()) + } else { + input.get_mut(&segment).ok_or_else(|| non_existent_table_err(segment))? + }; if value.is_table_like() { - descend(value, &path[1..]) + descend(value, &path[1..], insert_if_not_exists) } else { Err(non_existent_table_err(segment)) } @@ -79,7 +85,7 @@ impl Manifest { } } - descend(self.data.as_item_mut(), table_path) + descend(self.data.as_item_mut(), table_path, insert_if_not_exists) } /// Get all sections in the manifest that exist and might contain dependencies. @@ -376,7 +382,7 @@ impl LocalManifest { .to_owned(); let dep_key = dep.toml_key(); - let table = self.get_table_mut(table_path)?; + let table = self.get_table_mut(table_path, true)?; if let Some(dep_item) = table.as_table_like_mut().unwrap().get_mut(dep_key) { dep.update_toml(&crate_root, dep_item); } else { @@ -413,7 +419,7 @@ impl LocalManifest { .parent() .expect("manifest path is absolute") .to_owned(); - let table = self.get_table_mut(table_path)?; + let table = self.get_table_mut(table_path, true)?; // If (and only if) there is an old entry, merge the new one in. if table.as_table_like().unwrap().contains_key(dep_key) { @@ -447,22 +453,19 @@ impl LocalManifest { /// let mut manifest = LocalManifest { path, manifest: Manifest { data: toml_edit::Document::new() } }; /// let dep = Dependency::new("cargo-edit").set_version("0.1.0"); /// let _ = manifest.insert_into_table(&vec!["dependencies".to_owned()], &dep); - /// assert!(manifest.remove_from_table("dependencies", &dep.name).is_ok()); - /// assert!(manifest.remove_from_table("dependencies", &dep.name).is_err()); + /// assert!(manifest.remove_from_table(&["dependencies".to_owned()], &dep.name).is_ok()); + /// assert!(manifest.remove_from_table(&["dependencies".to_owned()], &dep.name).is_err()); /// assert!(!manifest.data.contains_key("dependencies")); /// ``` - pub fn remove_from_table(&mut self, table: &str, name: &str) -> CargoResult<()> { + pub fn remove_from_table(&mut self, table_path: &[String], name: &str) -> CargoResult<()> { let parent_table = self - .data - .get_mut(table) - .filter(|t| t.is_table_like()) - .ok_or_else(|| non_existent_table_err(table))?; + .get_table_mut(table_path, false)?; { let dep = parent_table .get_mut(name) .filter(|t| !t.is_none()) - .ok_or_else(|| non_existent_dependency_err(name, table))?; + .ok_or_else(|| non_existent_dependency_err(name, table_path.join(".")))?; // remove the dependency *dep = toml_edit::Item::None; } @@ -721,7 +724,7 @@ mod tests { let dep = Dependency::new("cargo-edit").set_version("0.1.0"); let _ = manifest.insert_into_table(&["dependencies".to_owned()], &dep); assert!(manifest - .remove_from_table("dependencies", &dep.name) + .remove_from_table(&["dependencies".to_owned()], &dep.name) .is_ok()); assert_eq!(manifest.data.to_string(), clone.data.to_string()); } @@ -780,7 +783,7 @@ mod tests { }; let dep = Dependency::new("cargo-edit").set_version("0.1.0"); assert!(manifest - .remove_from_table("dependencies", &dep.name) + .remove_from_table(&["dependencies".to_owned()], &dep.name) .is_err()); } @@ -799,7 +802,7 @@ mod tests { .insert_into_table(&["dependencies".to_owned()], &other_dep) .is_ok()); assert!(manifest - .remove_from_table("dependencies", &dep.name) + .remove_from_table(&["dependencies".to_owned()], &dep.name) .is_err()); } diff --git a/tests/cmd/rm/invalid_rm_target.in b/tests/cmd/rm/invalid_rm_target.in new file mode 120000 index 0000000000..4768db7584 --- /dev/null +++ b/tests/cmd/rm/invalid_rm_target.in @@ -0,0 +1 @@ +rm_target.in \ No newline at end of file diff --git a/tests/cmd/rm/invalid_rm_target.out/Cargo.toml b/tests/cmd/rm/invalid_rm_target.out/Cargo.toml new file mode 100644 index 0000000000..ea31ce722f --- /dev/null +++ b/tests/cmd/rm/invalid_rm_target.out/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "cargo-rm-target-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[target.x86_64-unknown-freebsd.build-dependencies] +semver = "0.1.0" + +[target.x86_64-unknown-linux-gnu.build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +pad = "0.1" +rustc-serialize = "0.3" +semver = "0.1" +toml = "0.1" +clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true} + +[target.x86_64-unknown-linux-gnu.dependencies] +dbus = "0.9.5" + +[dev-dependencies] +regex = "0.1.41" +serde = "1.0.90" + +[target.x86_64-unknown-linux-gnu.dev-dependencies] +ncurses = "5.101" + +[features] +std = ["serde/std", "semver/std"] +annoy = ["clippy"] diff --git a/tests/cmd/rm/invalid_rm_target.toml b/tests/cmd/rm/invalid_rm_target.toml new file mode 100644 index 0000000000..5bcc1b27ab --- /dev/null +++ b/tests/cmd/rm/invalid_rm_target.toml @@ -0,0 +1,15 @@ +bin.name = "cargo-rm" +args = ["rm", "--target", "powerpc-unknown-linux-gnu", "dbus"] +status.code = 1 +stdout = "" +stderr = """ + Removing dbus from dependencies for target `powerpc-unknown-linux-gnu` +Could not edit `Cargo.toml`. + +ERROR: The table `powerpc-unknown-linux-gnu` could not be found. +Error: The table `powerpc-unknown-linux-gnu` could not be found. +""" +fs.sandbox = true + +[env.add] +CARGO_IS_TEST="1" diff --git a/tests/cmd/rm/invalid_rm_target_dep.in b/tests/cmd/rm/invalid_rm_target_dep.in new file mode 120000 index 0000000000..4768db7584 --- /dev/null +++ b/tests/cmd/rm/invalid_rm_target_dep.in @@ -0,0 +1 @@ +rm_target.in \ No newline at end of file diff --git a/tests/cmd/rm/invalid_rm_target_dep.out/Cargo.toml b/tests/cmd/rm/invalid_rm_target_dep.out/Cargo.toml new file mode 100644 index 0000000000..ea31ce722f --- /dev/null +++ b/tests/cmd/rm/invalid_rm_target_dep.out/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "cargo-rm-target-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[target.x86_64-unknown-freebsd.build-dependencies] +semver = "0.1.0" + +[target.x86_64-unknown-linux-gnu.build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +pad = "0.1" +rustc-serialize = "0.3" +semver = "0.1" +toml = "0.1" +clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true} + +[target.x86_64-unknown-linux-gnu.dependencies] +dbus = "0.9.5" + +[dev-dependencies] +regex = "0.1.41" +serde = "1.0.90" + +[target.x86_64-unknown-linux-gnu.dev-dependencies] +ncurses = "5.101" + +[features] +std = ["serde/std", "semver/std"] +annoy = ["clippy"] diff --git a/tests/cmd/rm/invalid_rm_target_dep.toml b/tests/cmd/rm/invalid_rm_target_dep.toml new file mode 100644 index 0000000000..8d96c9fda6 --- /dev/null +++ b/tests/cmd/rm/invalid_rm_target_dep.toml @@ -0,0 +1,15 @@ +bin.name = "cargo-rm" +args = ["rm", "--target", "x86_64-unknown-linux-gnu", "toml"] +status.code = 1 +stdout = "" +stderr = """ + Removing toml from dependencies for target `x86_64-unknown-linux-gnu` +Could not edit `Cargo.toml`. + +ERROR: The dependency `toml` could not be found in `target.x86_64-unknown-linux-gnu.dependencies`. +Error: The dependency `toml` could not be found in `target.x86_64-unknown-linux-gnu.dependencies`. +""" +fs.sandbox = true + +[env.add] +CARGO_IS_TEST="1" diff --git a/tests/cmd/rm/rm_target.in/Cargo.toml b/tests/cmd/rm/rm_target.in/Cargo.toml new file mode 100644 index 0000000000..ea31ce722f --- /dev/null +++ b/tests/cmd/rm/rm_target.in/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "cargo-rm-target-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[target.x86_64-unknown-freebsd.build-dependencies] +semver = "0.1.0" + +[target.x86_64-unknown-linux-gnu.build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +pad = "0.1" +rustc-serialize = "0.3" +semver = "0.1" +toml = "0.1" +clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true} + +[target.x86_64-unknown-linux-gnu.dependencies] +dbus = "0.9.5" + +[dev-dependencies] +regex = "0.1.41" +serde = "1.0.90" + +[target.x86_64-unknown-linux-gnu.dev-dependencies] +ncurses = "5.101" + +[features] +std = ["serde/std", "semver/std"] +annoy = ["clippy"] diff --git a/tests/cmd/rm/rm_target.out/Cargo.toml b/tests/cmd/rm/rm_target.out/Cargo.toml new file mode 100644 index 0000000000..1f1d92e6c7 --- /dev/null +++ b/tests/cmd/rm/rm_target.out/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "cargo-rm-target-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[target.x86_64-unknown-freebsd.build-dependencies] +semver = "0.1.0" + +[target.x86_64-unknown-linux-gnu.build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +pad = "0.1" +rustc-serialize = "0.3" +semver = "0.1" +toml = "0.1" +clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true} + +[dev-dependencies] +regex = "0.1.41" +serde = "1.0.90" + +[target.x86_64-unknown-linux-gnu.dev-dependencies] +ncurses = "5.101" + +[features] +std = ["serde/std", "semver/std"] +annoy = ["clippy"] diff --git a/tests/cmd/rm/rm_target.toml b/tests/cmd/rm/rm_target.toml new file mode 100644 index 0000000000..332771f13f --- /dev/null +++ b/tests/cmd/rm/rm_target.toml @@ -0,0 +1,11 @@ +bin.name = "cargo-rm" +args = ["rm", "--target", "x86_64-unknown-linux-gnu", "dbus"] +status = "success" +stdout = "" +stderr = """ + Removing dbus from dependencies for target `x86_64-unknown-linux-gnu` +""" +fs.sandbox = true + +[env.add] +CARGO_IS_TEST="1" diff --git a/tests/cmd/rm/rm_target_build.in b/tests/cmd/rm/rm_target_build.in new file mode 120000 index 0000000000..4768db7584 --- /dev/null +++ b/tests/cmd/rm/rm_target_build.in @@ -0,0 +1 @@ +rm_target.in \ No newline at end of file diff --git a/tests/cmd/rm/rm_target_build.out/Cargo.toml b/tests/cmd/rm/rm_target_build.out/Cargo.toml new file mode 100644 index 0000000000..288e5c0dd1 --- /dev/null +++ b/tests/cmd/rm/rm_target_build.out/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "cargo-rm-target-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[target.x86_64-unknown-freebsd.build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +pad = "0.1" +rustc-serialize = "0.3" +semver = "0.1" +toml = "0.1" +clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true} + +[target.x86_64-unknown-linux-gnu.dependencies] +dbus = "0.9.5" + +[dev-dependencies] +regex = "0.1.41" +serde = "1.0.90" + +[target.x86_64-unknown-linux-gnu.dev-dependencies] +ncurses = "5.101" + +[features] +std = ["serde/std", "semver/std"] +annoy = ["clippy"] diff --git a/tests/cmd/rm/rm_target_build.toml b/tests/cmd/rm/rm_target_build.toml new file mode 100644 index 0000000000..619dbefc1b --- /dev/null +++ b/tests/cmd/rm/rm_target_build.toml @@ -0,0 +1,11 @@ +bin.name = "cargo-rm" +args = ["rm", "--build", "--target", "x86_64-unknown-linux-gnu", "semver"] +status = "success" +stdout = "" +stderr = """ + Removing semver from build-dependencies for target `x86_64-unknown-linux-gnu` +""" +fs.sandbox = true + +[env.add] +CARGO_IS_TEST="1" diff --git a/tests/cmd/rm/rm_target_dev.in b/tests/cmd/rm/rm_target_dev.in new file mode 120000 index 0000000000..4768db7584 --- /dev/null +++ b/tests/cmd/rm/rm_target_dev.in @@ -0,0 +1 @@ +rm_target.in \ No newline at end of file diff --git a/tests/cmd/rm/rm_target_dev.out/Cargo.toml b/tests/cmd/rm/rm_target_dev.out/Cargo.toml new file mode 100644 index 0000000000..ffd61912b4 --- /dev/null +++ b/tests/cmd/rm/rm_target_dev.out/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "cargo-rm-target-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[target.x86_64-unknown-freebsd.build-dependencies] +semver = "0.1.0" + +[target.x86_64-unknown-linux-gnu.build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +pad = "0.1" +rustc-serialize = "0.3" +semver = "0.1" +toml = "0.1" +clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true} + +[target.x86_64-unknown-linux-gnu.dependencies] +dbus = "0.9.5" + +[dev-dependencies] +regex = "0.1.41" +serde = "1.0.90" + +[features] +std = ["serde/std", "semver/std"] +annoy = ["clippy"] diff --git a/tests/cmd/rm/rm_target_dev.toml b/tests/cmd/rm/rm_target_dev.toml new file mode 100644 index 0000000000..d43967f321 --- /dev/null +++ b/tests/cmd/rm/rm_target_dev.toml @@ -0,0 +1,11 @@ +bin.name = "cargo-rm" +args = ["rm", "--dev", "--target", "x86_64-unknown-linux-gnu", "ncurses"] +status = "success" +stdout = "" +stderr = """ + Removing ncurses from dev-dependencies for target `x86_64-unknown-linux-gnu` +""" +fs.sandbox = true + +[env.add] +CARGO_IS_TEST="1" From 3754071406c2eff386f91d13142099d1dc671c15 Mon Sep 17 00:00:00 2001 From: cassaundra Date: Mon, 20 Jun 2022 11:40:33 -0700 Subject: [PATCH 2/6] Split `Manifest::get_table_mut` into two functions Now with inserting and erroring (non-inserting) variants --- src/bin/add/add.rs | 2 +- src/manifest.rs | 76 +++++++++++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/bin/add/add.rs b/src/bin/add/add.rs index a6e294f4ee..698f33ea63 100644 --- a/src/bin/add/add.rs +++ b/src/bin/add/add.rs @@ -618,7 +618,7 @@ fn exec(mut args: AddArgs) -> CargoResult<()> { if was_sorted { if let Some(table) = manifest - .get_table_mut(&args.get_section(), true) + .get_or_insert_table_mut(&args.get_section()) .ok() .and_then(TomlItem::as_table_like_mut) { diff --git a/src/manifest.rs b/src/manifest.rs index 34d9bd884e..4b8243d275 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -57,35 +57,23 @@ impl Manifest { } /// Get the specified table from the manifest. + /// + /// If there is no table at the specified path, then a non-existent table + /// error will be returned. pub fn get_table_mut<'a>( &'a mut self, table_path: &[String], - insert_if_not_exists: bool, ) -> CargoResult<&'a mut toml_edit::Item> { - /// Descend into a manifest until the required table is found. - fn descend<'a>( - input: &'a mut toml_edit::Item, - path: &[String], - insert_if_not_exists: bool, - ) -> CargoResult<&'a mut toml_edit::Item> { - if let Some(segment) = path.get(0) { - let value = if insert_if_not_exists { - input[&segment].or_insert(toml_edit::table()) - } else { - input.get_mut(&segment).ok_or_else(|| non_existent_table_err(segment))? - }; - - if value.is_table_like() { - descend(value, &path[1..], insert_if_not_exists) - } else { - Err(non_existent_table_err(segment)) - } - } else { - Ok(input) - } - } + self.get_table_mut_internal(table_path, false) + } - descend(self.data.as_item_mut(), table_path, insert_if_not_exists) + /// Get the specified table from the manifest, inserting it if it does not + /// exist. + pub fn get_or_insert_table_mut<'a>( + &'a mut self, + table_path: &[String], + ) -> CargoResult<&'a mut toml_edit::Item> { + self.get_table_mut_internal(table_path, true) } /// Get all sections in the manifest that exist and might contain dependencies. @@ -181,6 +169,39 @@ impl Manifest { Ok(features) } + + fn get_table_mut_internal<'a>( + &'a mut self, + table_path: &[String], + insert_if_not_exists: bool, + ) -> CargoResult<&'a mut toml_edit::Item> { + /// Descend into a manifest until the required table is found. + fn descend<'a>( + input: &'a mut toml_edit::Item, + path: &[String], + insert_if_not_exists: bool, + ) -> CargoResult<&'a mut toml_edit::Item> { + if let Some(segment) = path.get(0) { + let value = if insert_if_not_exists { + input[&segment].or_insert(toml_edit::table()) + } else { + input + .get_mut(&segment) + .ok_or_else(|| non_existent_table_err(segment))? + }; + + if value.is_table_like() { + descend(value, &path[1..], insert_if_not_exists) + } else { + Err(non_existent_table_err(segment)) + } + } else { + Ok(input) + } + } + + descend(self.data.as_item_mut(), table_path, insert_if_not_exists) + } } impl str::FromStr for Manifest { @@ -382,7 +403,7 @@ impl LocalManifest { .to_owned(); let dep_key = dep.toml_key(); - let table = self.get_table_mut(table_path, true)?; + let table = self.get_or_insert_table_mut(table_path)?; if let Some(dep_item) = table.as_table_like_mut().unwrap().get_mut(dep_key) { dep.update_toml(&crate_root, dep_item); } else { @@ -419,7 +440,7 @@ impl LocalManifest { .parent() .expect("manifest path is absolute") .to_owned(); - let table = self.get_table_mut(table_path, true)?; + let table = self.get_or_insert_table_mut(table_path)?; // If (and only if) there is an old entry, merge the new one in. if table.as_table_like().unwrap().contains_key(dep_key) { @@ -458,8 +479,7 @@ impl LocalManifest { /// assert!(!manifest.data.contains_key("dependencies")); /// ``` pub fn remove_from_table(&mut self, table_path: &[String], name: &str) -> CargoResult<()> { - let parent_table = self - .get_table_mut(table_path, false)?; + let parent_table = self.get_table_mut(table_path)?; { let dep = parent_table From c55dd721c1a6088e18d6b983cea5c68d267fab27 Mon Sep 17 00:00:00 2001 From: cassaundra Date: Mon, 20 Jun 2022 11:46:03 -0700 Subject: [PATCH 3/6] Apply linter and formatter suggestions --- src/bin/rm/rm.rs | 6 +----- src/manifest.rs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/bin/rm/rm.rs b/src/bin/rm/rm.rs index 6134fa0804..6ac3054588 100644 --- a/src/bin/rm/rm.rs +++ b/src/bin/rm/rm.rs @@ -88,11 +88,7 @@ impl RmArgs { if let Some(ref target) = self.target { assert!(!target.is_empty(), "Target specification may not be empty"); - vec![ - "target".to_owned(), - target.clone(), - section_name.to_owned(), - ] + vec!["target".to_owned(), target.clone(), section_name.to_owned()] } else { vec![section_name.to_owned()] } diff --git a/src/manifest.rs b/src/manifest.rs index 4b8243d275..2f49513582 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -1,5 +1,5 @@ use std::collections::BTreeMap; -use std::fs::{self}; +use std::fs; use std::io::Write; use std::ops::{Deref, DerefMut}; use std::path::{Path, PathBuf}; From 36dfb10dcf9f1e9ab812b1b1e73f6af52fda09ae Mon Sep 17 00:00:00 2001 From: cassaundra Date: Mon, 20 Jun 2022 11:49:32 -0700 Subject: [PATCH 4/6] Fix symlinking of rm target tests Tests now point to a template `rm-target.in` instead of `rm_target.in`, which belongs to a test. --- tests/cmd/rm/invalid_rm_target.in | 2 +- tests/cmd/rm/invalid_rm_target_dep.in | 2 +- tests/cmd/rm/{rm_target.in => rm-target.in}/Cargo.toml | 0 tests/cmd/rm/rm_target.in | 1 + tests/cmd/rm/rm_target_build.in | 2 +- tests/cmd/rm/rm_target_dev.in | 2 +- 6 files changed, 5 insertions(+), 4 deletions(-) rename tests/cmd/rm/{rm_target.in => rm-target.in}/Cargo.toml (100%) create mode 120000 tests/cmd/rm/rm_target.in diff --git a/tests/cmd/rm/invalid_rm_target.in b/tests/cmd/rm/invalid_rm_target.in index 4768db7584..03f4764e45 120000 --- a/tests/cmd/rm/invalid_rm_target.in +++ b/tests/cmd/rm/invalid_rm_target.in @@ -1 +1 @@ -rm_target.in \ No newline at end of file +rm-target.in \ No newline at end of file diff --git a/tests/cmd/rm/invalid_rm_target_dep.in b/tests/cmd/rm/invalid_rm_target_dep.in index 4768db7584..03f4764e45 120000 --- a/tests/cmd/rm/invalid_rm_target_dep.in +++ b/tests/cmd/rm/invalid_rm_target_dep.in @@ -1 +1 @@ -rm_target.in \ No newline at end of file +rm-target.in \ No newline at end of file diff --git a/tests/cmd/rm/rm_target.in/Cargo.toml b/tests/cmd/rm/rm-target.in/Cargo.toml similarity index 100% rename from tests/cmd/rm/rm_target.in/Cargo.toml rename to tests/cmd/rm/rm-target.in/Cargo.toml diff --git a/tests/cmd/rm/rm_target.in b/tests/cmd/rm/rm_target.in new file mode 120000 index 0000000000..03f4764e45 --- /dev/null +++ b/tests/cmd/rm/rm_target.in @@ -0,0 +1 @@ +rm-target.in \ No newline at end of file diff --git a/tests/cmd/rm/rm_target_build.in b/tests/cmd/rm/rm_target_build.in index 4768db7584..03f4764e45 120000 --- a/tests/cmd/rm/rm_target_build.in +++ b/tests/cmd/rm/rm_target_build.in @@ -1 +1 @@ -rm_target.in \ No newline at end of file +rm-target.in \ No newline at end of file diff --git a/tests/cmd/rm/rm_target_dev.in b/tests/cmd/rm/rm_target_dev.in index 4768db7584..03f4764e45 120000 --- a/tests/cmd/rm/rm_target_dev.in +++ b/tests/cmd/rm/rm_target_dev.in @@ -1 +1 @@ -rm_target.in \ No newline at end of file +rm-target.in \ No newline at end of file From 3a6281fff4fe5db6786f5ff667f8b846679db8d3 Mon Sep 17 00:00:00 2001 From: cassaundra Date: Mon, 20 Jun 2022 12:19:33 -0700 Subject: [PATCH 5/6] Remove rm arg group Opting for the original usage of `conflicts_with` for now. In the future, this group should only include `--build` and `--dev`. --- src/bin/rm/rm.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/bin/rm/rm.rs b/src/bin/rm/rm.rs index 6ac3054588..3b97776479 100644 --- a/src/bin/rm/rm.rs +++ b/src/bin/rm/rm.rs @@ -20,7 +20,6 @@ pub struct RmArgs { short = 'D', conflicts_with = "build", help_heading = "SECTION", - group = "SECTION" )] dev: bool, @@ -30,7 +29,6 @@ pub struct RmArgs { short = 'B', conflicts_with = "dev", help_heading = "SECTION", - group = "SECTION" )] build: bool, @@ -39,7 +37,6 @@ pub struct RmArgs { long, forbid_empty_values = true, help_heading = "SECTION", - group = "section" )] target: Option, From 4ceda177678e17f4823c635c8b49d5f32ee3215c Mon Sep 17 00:00:00 2001 From: cassaundra Date: Mon, 20 Jun 2022 12:25:54 -0700 Subject: [PATCH 6/6] Apply rustfmt --- src/bin/rm/rm.rs | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/bin/rm/rm.rs b/src/bin/rm/rm.rs index 3b97776479..d653a91533 100644 --- a/src/bin/rm/rm.rs +++ b/src/bin/rm/rm.rs @@ -15,29 +15,15 @@ pub struct RmArgs { crates: Vec, /// Remove crate as development dependency. - #[clap( - long, - short = 'D', - conflicts_with = "build", - help_heading = "SECTION", - )] + #[clap(long, short = 'D', conflicts_with = "build", help_heading = "SECTION")] dev: bool, /// Remove crate as build dependency. - #[clap( - long, - short = 'B', - conflicts_with = "dev", - help_heading = "SECTION", - )] + #[clap(long, short = 'B', conflicts_with = "dev", help_heading = "SECTION")] build: bool, /// Remove as dependency from the given target platform. - #[clap( - long, - forbid_empty_values = true, - help_heading = "SECTION", - )] + #[clap(long, forbid_empty_values = true, help_heading = "SECTION")] target: Option, /// Path to the manifest to remove a dependency from.