From e74c71a801f3e34fdfbd3c2b008c2830ad91f642 Mon Sep 17 00:00:00 2001 From: Niklas Lindorfer Date: Wed, 11 Sep 2024 18:30:52 +0100 Subject: [PATCH] feat: Implement `--force` (#127) * Fix lints * Implement force for registry * Typos * Implement force for version mismatch --- src/cli.rs | 6 +- src/context.rs | 5 ++ src/lib.rs | 31 +++---- tests/all/cli.rs | 4 +- tests/all/main.rs | 220 +++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 246 insertions(+), 20 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 0df98af..85c0e01 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -29,7 +29,7 @@ pub struct Override { #[arg(long)] /// Name of the registry to use. - /// Usually `cargo-override` can correctly determine which regestiry to use without needing this flag + /// Usually `cargo-override` can correctly determine which registry to use without needing this flag pub registry: Option, /// Path to the `Cargo.toml` file that needs patching. @@ -46,6 +46,10 @@ pub struct Override { /// Equivalent to specifying both --locked and --offline #[arg(long)] pub frozen: bool, + + /// Force the override, ignoring compatibility checks. + #[arg(long)] + pub force: bool, } #[derive(Args, Debug)] diff --git a/src/context.rs b/src/context.rs index 214feff..07d8972 100644 --- a/src/context.rs +++ b/src/context.rs @@ -13,6 +13,8 @@ pub struct Context { pub manifest_path: Option, pub mode: Mode, + + pub force: bool, } #[derive(Copy, Clone)] @@ -40,6 +42,7 @@ impl TryFrom for Context { manifest_path, source: cli::Source { path, git }, git: cli::Git { branch, tag, rev }, + force, }), }: Cli, ) -> Result { @@ -79,6 +82,8 @@ impl TryFrom for Context { manifest_path, mode, + + force, }) } } diff --git a/src/lib.rs b/src/lib.rs index 20db9f8..df1c113 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,7 @@ pub use context::Context; use std::path::{Path, PathBuf}; -use anyhow::{bail, Context as _}; +use anyhow::{bail, ensure, Context as _}; use fs_err as fs; pub static DEFAULT_REGISTRY: &str = "crates-io"; @@ -24,6 +24,7 @@ pub fn run(working_dir: &Path, args: Cli) -> anyhow::Result<()> { manifest_path, registry_hint, mode, + force, } = args.try_into()?; let path = match &mode { @@ -47,7 +48,7 @@ pub fn run(working_dir: &Path, args: Cli) -> anyhow::Result<()> { let manifest_path = project_manifest(manifest_dir, cargo)?; - let project_deps = metadata::direct_dependencies(&manifest_dir, cargo) + let project_deps = metadata::direct_dependencies(manifest_dir, cargo) .context("failed to get dependencies for current project")?; let mut direct_deps = project_deps @@ -56,19 +57,18 @@ pub fn run(working_dir: &Path, args: Cli) -> anyhow::Result<()> { .peekable(); let dependency = if direct_deps.peek().is_some() { - if let Some(dep) = direct_deps.find(|dependency| match dependency.requirement { - Some(ref req) => req.matches(&patch_manifest.version), - None => false, - }) { - dep.clone() - } else { - bail!("patch could not be applied because version is incompatible") - } + direct_deps + .find(|dep| { + dep.requirement + .as_ref() + .is_some_and(|req| req.matches(&patch_manifest.version) || force) + }) + .context("patch could not be applied because version is incompatible")? } else { let resolved_deps = metadata::resolved_dependencies(manifest_dir, cargo) .context("failed to get dependencies for current project")?; - resolved_deps + &resolved_deps .into_iter() .find(|dep| dep.name == patch_manifest.name) .with_context(|| { @@ -82,7 +82,7 @@ pub fn run(working_dir: &Path, args: Cli) -> anyhow::Result<()> { let dependency_registry = if dependency.registry == Some(DEFAULT_REGISTRY_URL.to_owned()) { None } else { - dependency.registry + dependency.registry.as_deref() }; let registry = if let Some(registry_url) = &dependency_registry { @@ -97,8 +97,8 @@ pub fn run(working_dir: &Path, args: Cli) -> anyhow::Result<()> { registry_guess } (Some(registry_flag), Some(registry_guess)) => { - // TODO: force is unimplemented - bail!( + ensure!( + force, "user provided registry `{}` with the `--registry` flag \ but dependency `{}` \ uses registry `{}`. @@ -106,7 +106,8 @@ pub fn run(working_dir: &Path, args: Cli) -> anyhow::Result<()> { registry_flag, dependency.name, registry_guess - ) + ); + registry_flag } (None, None) => bail!( "unable to determine registry name for `{}` diff --git a/tests/all/cli.rs b/tests/all/cli.rs index 8fceda0..233fbc0 100644 --- a/tests/all/cli.rs +++ b/tests/all/cli.rs @@ -60,7 +60,7 @@ fn override_subcommand_help_message() { --rev Specific commit to use when overriding from git --registry - Name of the registry to use. Usually `cargo-override` can correctly determine which regestiry to use without needing this flag + Name of the registry to use. Usually `cargo-override` can correctly determine which registry to use without needing this flag --manifest-path Path to the `Cargo.toml` file that needs patching. By default, `cargo-override` searches for the `Cargo.toml` file in the current directory or any parent directory --locked @@ -69,6 +69,8 @@ fn override_subcommand_help_message() { Prevents cargo from accessing the network --frozen Equivalent to specifying both --locked and --offline + --force + Force the override, ignoring compatibility checks -h, --help Print help -V, --version diff --git a/tests/all/main.rs b/tests/all/main.rs index 2319ee5..532ec93 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -639,7 +639,7 @@ fn patch_absolute_path() { #[test_case("0.1.0", "0.0.2")] #[test_case(">=1.2.3, <1.8.0", "1.2.3-alpha.1")] #[googletest::test] -fn patch_version_incompatible(dependency_version: &str, patch_version: &str) { +fn patch_version_incompatible_fails(dependency_version: &str, patch_version: &str) { let working_dir = TempDir::new().unwrap(); let working_dir = working_dir.path(); @@ -695,6 +695,77 @@ fn patch_version_incompatible(dependency_version: &str, patch_version: &str) { expect_eq!(manifest_before, manifest_after); } +#[googletest::test] +fn patch_version_incompatible_force_succeeds() { + let working_dir = TempDir::new().unwrap(); + let working_dir = working_dir.path(); + + let patch_crate_name = "redact"; + + let patch_folder = patch_crate_name.to_string(); + let patch_folder_path = working_dir.join(patch_folder.clone()); + + fs::create_dir(&patch_folder_path).expect("failed to create patch folder"); + + let package_name = "package-name"; + let manifest_header = Header::basic(package_name); + let manifest = Manifest::new(manifest_header) + .add_target(Target::bin(package_name, "src/main.rs")) + .add_dependency(Dependency::new(patch_crate_name, "0.1.0")) + .render(); + + let working_dir_manifest_path = create_cargo_manifest(working_dir, &manifest); + let _patch_manifest_path = create_cargo_manifest( + &patch_folder_path, + &Manifest::new( + Header::basic(patch_crate_name) + .name(patch_crate_name.to_owned()) + .version("0.0.2".to_owned()), + ) + .add_target(Target::lib("patch_pacakge", "src/lib.rs")) + .render(), + ); + + let mut command = override_path(&patch_folder, working_dir, |command| command.arg("--force")); + + let assert = command.assert(); + + let output = assert.get_output(); + + let stdout = String::from_utf8(output.stdout.clone()).unwrap(); + let stderr = String::from_utf8(output.stderr.clone()).unwrap(); + + assert.success(); + + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Patched dependency "redact" on registry "crates-io" + "###); + + let manifest = fs::read_to_string(working_dir_manifest_path).unwrap(); + + insta::assert_toml_snapshot!(manifest, @r###" + ''' + [package] + name = "package-name" + version = "0.1.0" + edition = "2021" + + # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + + [dependencies] + redact = "0.1.0" + + [[bin]] + name = "package-name" + path = "src/main.rs" + + [patch.crates-io] + redact = { path = "redact" } + ''' + "###); +} + #[test_case(None, None)] #[test_case(Some("anyhow"), None)] #[test_case(None, Some("0.1.0"))] @@ -765,7 +836,7 @@ fn patch_exists_put_project_does_not_depend_on_it() { let manifest_before = fs::read_to_string(&working_dir_manifest_path).unwrap(); - let mut command = override_path(&patch_folder, working_dir, |command| command); + let mut command = override_path(patch_folder, working_dir, |command| command); let assert = command.assert(); @@ -916,7 +987,7 @@ fn basic_cargo_config(path: &Path) { #[cfg(feature = "failing_tests")] fn basic_cargo_env_config(path: &Path) { write_cargo_config( - &path, + path, r#" [env] CARGO_REGISTRIES_PRIVATE_REGISTRY_INDEX = "https://dl.cloudsmith.io/basic/private/registry/cargo/index.git" @@ -1000,6 +1071,149 @@ fn patch_exists_alt_registry(setup: impl Fn(&Path)) { } } +#[cfg_attr(feature = "failing_tests", test_case(basic_cargo_env_config))] +#[test_case(basic_cargo_config)] +#[googletest::test] +fn patch_registry_mismatch_fails(setup: impl Fn(&Path)) { + let mut working_dir = tempfile::Builder::new(); + let working_dir = working_dir.keep(true).tempdir().unwrap(); + let working_dir = working_dir.path(); + + setup(working_dir); + + let patch_crate_name = "anyhow"; + let patch_folder = patch_crate_name.to_string(); + let patch_folder_path = working_dir.join(patch_folder.clone()); + + fs::create_dir(&patch_folder_path).expect("failed to create patch folder"); + + let package_name = "package-name"; + let manifest_header = Header::basic(package_name); + let manifest = Manifest::new(manifest_header) + .add_target(Target::bin(package_name, "src/main.rs")) + .add_dependency(Dependency::new(patch_crate_name, "1.0.86").registry("private-registry")) + .render(); + + let working_dir_manifest_path = create_cargo_manifest(working_dir, &manifest); + let _patch_manifest_path = create_cargo_manifest( + &patch_folder_path, + &Manifest::new(Header::basic(patch_crate_name).version("1.1.5".to_owned())) + .add_target(Target::lib(patch_crate_name, "src/lib.rs")) + .render(), + ); + + let manifest_before = fs::read_to_string(&working_dir_manifest_path).unwrap(); + + let mut command = override_path(&patch_folder, working_dir, |command| { + command.arg("--registry").arg("another-registry") + }); + + let assert = command.assert(); + + let output = assert.get_output(); + + let stdout = String::from_utf8(output.stdout.clone()).unwrap(); + let stderr = String::from_utf8(output.stderr.clone()).unwrap(); + + assert.failure(); + + insta::allow_duplicates! { + insta::with_settings!({filters => vec![ + (patch_folder.as_str(), "[PATCH]"), + ]}, { + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + error: user provided registry `another-registry` with the `--registry` flag but dependency `[PATCH]` uses registry `private-registry`. + To use the registry, you passed, use `--force` + "###); + }) + }; + + let manifest_after = fs::read_to_string(working_dir_manifest_path).unwrap(); + + expect_eq!(manifest_before, manifest_after); +} + +#[cfg_attr(feature = "failing_tests", test_case(basic_cargo_env_config))] +#[test_case(basic_cargo_config)] +#[googletest::test] +fn patch_registry_mismatch_force_succeeds(setup: impl Fn(&Path)) { + let mut working_dir = tempfile::Builder::new(); + let working_dir = working_dir.keep(true).tempdir().unwrap(); + let working_dir = working_dir.path(); + + setup(working_dir); + + let patch_crate_name = "anyhow"; + let patch_folder = patch_crate_name.to_string(); + let patch_folder_path = working_dir.join(patch_folder.clone()); + + fs::create_dir(&patch_folder_path).expect("failed to create patch folder"); + + let package_name = "package-name"; + let manifest_header = Header::basic(package_name); + let manifest = Manifest::new(manifest_header) + .add_target(Target::bin(package_name, "src/main.rs")) + .add_dependency(Dependency::new(patch_crate_name, "1.0.86").registry("private-registry")) + .render(); + + let working_dir_manifest_path = create_cargo_manifest(working_dir, &manifest); + let _patch_manifest_path = create_cargo_manifest( + &patch_folder_path, + &Manifest::new(Header::basic(patch_crate_name).version("1.1.5".to_owned())) + .add_target(Target::lib(patch_crate_name, "src/lib.rs")) + .render(), + ); + + let mut command = override_path(&patch_folder, working_dir, |command| { + command + .arg("--registry") + .arg("another-registry") + .arg("--force") + }); + + let assert = command.assert(); + + let output = assert.get_output(); + + let stdout = String::from_utf8(output.stdout.clone()).unwrap(); + let stderr = String::from_utf8(output.stderr.clone()).unwrap(); + + assert.success(); + + insta::allow_duplicates! { + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Patched dependency "anyhow" on registry "another-registry" + "###); + } + + let manifest = fs::read_to_string(working_dir_manifest_path).unwrap(); + + insta::allow_duplicates! { + insta::assert_toml_snapshot!(manifest, @r###" + ''' + [package] + name = "package-name" + version = "0.1.0" + edition = "2021" + + # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + + [dependencies] + anyhow = { version = "1.0.86", registry = "private-registry" } + + [[bin]] + name = "package-name" + path = "src/main.rs" + + [patch.another-registry] + anyhow = { path = "anyhow" } + ''' + "###); + } +} + #[googletest::test] fn patch_exists_alt_registry_from_env() { let working_dir = TempDir::new().unwrap();