From 7592481e7d07220f2ed32c3d57d5b98fc3f8850f Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Sat, 11 May 2024 15:12:02 +0800 Subject: [PATCH] feat: use enum instead --- src/cargo/core/features.rs | 65 ++++----------- src/cargo/sources/git/utils.rs | 6 +- tests/testsuite/config.rs | 142 +++++++++++++++++---------------- 3 files changed, 94 insertions(+), 119 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 3152483df318..0263a10bbe42 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -761,8 +761,7 @@ unstable_cli_options!( gc: bool = ("Track cache usage and \"garbage collect\" unused files"), #[serde(deserialize_with = "deserialize_git_features")] git: Option = ("Enable support for shallow git fetch operations"), - #[serde(deserialize_with = "deserialize_gitoxide_features")] - gitoxide: Option = ("Use gitoxide for the given git interactions, or all of them if no argument is given"), + gitoxide: Option = ("Use gitoxide for the given git interactions, or all of them if no argument is given"), host_config: bool = ("Enable the `[host]` section in the .cargo/config.toml file"), minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"), msrv_policy: bool = ("Enable rust-version aware policy within cargo"), @@ -952,6 +951,14 @@ fn parse_git(it: impl Iterator>) -> CargoResult( - deserializer: D, -) -> Result, D::Error> -where - D: serde::de::Deserializer<'de>, -{ - struct GitoxideFeaturesVisitor; - - impl<'de> serde::de::Visitor<'de> for GitoxideFeaturesVisitor { - type Value = Option; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str(&GitoxideFeatures::expecting()) - } - - fn visit_str(self, s: &str) -> Result - where - E: serde::de::Error, - { - Ok(parse_gitoxide(s.split(",")).map_err(serde::de::Error::custom)?) - } - - fn visit_none(self) -> Result - where - E: serde::de::Error, - { - Ok(None) - } - - fn visit_map(self, map: V) -> Result - where - V: serde::de::MapAccess<'de>, - { - let mvd = serde::de::value::MapAccessDeserializer::new(map); - Ok(Some(GitoxideFeatures::deserialize(mvd)?)) - } - } - - deserializer.deserialize_any(GitoxideFeaturesVisitor) -} - fn parse_gitoxide( it: impl Iterator>, ) -> CargoResult> { @@ -1088,7 +1054,7 @@ impl CliUnstable { } if self.gitoxide.is_none() && cargo_use_gitoxide_instead_of_git2() { - self.gitoxide = GitoxideFeatures::safe().into(); + self.gitoxide = Some(Gitoxide::Some(GitoxideFeatures::safe().into())); } Ok(warnings) } @@ -1234,10 +1200,13 @@ impl CliUnstable { )? } "gitoxide" => { - self.gitoxide = v.map_or_else( - || Ok(Some(GitoxideFeatures::all())), - |v| parse_gitoxide(v.split(',')), - )? + self.gitoxide = { + let rt = v.map_or_else( + || Ok(Some(GitoxideFeatures::all())), + |v| parse_gitoxide(v.split(',')), + )?; + Some(Gitoxide::Some(rt.unwrap())) + } } "host-config" => self.host_config = parse_empty(k, v)?, "next-lockfile-bump" => self.next_lockfile_bump = parse_empty(k, v)?, diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 8d84c2763221..74a36fc2f7c3 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -1,6 +1,7 @@ //! Utilities for handling git repositories, mainly around //! authentication/cloning. +use crate::core::features::{Gitoxide, GitoxideFeatures}; use crate::core::{GitReference, Verbosity}; use crate::sources::git::fetch::RemoteKind; use crate::sources::git::oxide; @@ -992,7 +993,10 @@ pub fn fetch( return fetch_with_cli(repo, remote_url, &refspecs, tags, gctx); } - if gctx.cli_unstable().gitoxide.map_or(false, |git| git.fetch) { + if gctx.cli_unstable().gitoxide.map_or(false, |git| match git { + Gitoxide::All => GitoxideFeatures::all().fetch, + Gitoxide::Some(git) => git.fetch, + }) { let git2_repo = repo; let config_overrides = cargo_config_to_gitoxide_overrides(gctx)?; let repo_reinitialized = AtomicBool::default(); diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 3574a68a6ec0..547cb60e9f45 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1,6 +1,6 @@ //! Tests for config settings. -use cargo::core::features::{GitFeatures, GitoxideFeatures}; +use cargo::core::features::{GitFeatures, Gitoxide, GitoxideFeatures}; use cargo::core::{PackageIdSpec, Shell}; use cargo::util::context::{ self, Definition, GlobalContext, JobsConfig, SslVersionConfig, StringList, @@ -1936,42 +1936,49 @@ Caused by: } #[cargo_test] -fn gitoxide_features_as_str() { +fn gitoxide_features_as_table() { let gctx = GlobalContextBuilder::new() - .env("CARGO_UNSTABLE_GITOXIDE", "fetch") + .env("CARGO_UNSTABLE_GITOXIDE_ABC", "true") .build(); - verify( - gctx, - Some(GitoxideFeatures { - fetch: true, - ..GitoxideFeatures::default() - }), - ); + verify(gctx, Some(Gitoxide::All)); let gctx = GlobalContextBuilder::new() - .env("CARGO_UNSTABLE_GITOXIDE", "all,fetch") + .env("CARGO_UNSTABLE_GITOXIDE", "true") .build(); - verify(gctx, Some(GitoxideFeatures::all())); + assert_error( + gctx.get::>("unstable") + .unwrap_err(), + "\ +data did not match any variant of untagged enum Gitoxide", + ); let gctx = GlobalContextBuilder::new() - .env("CARGO_UNSTABLE_GITOXIDE", "fetch,checkout") + .env("CARGO_UNSTABLE_GITOXIDE_CHECKOUT", "true") .build(); - verify(gctx, Some(GitoxideFeatures::all())); + verify(gctx, Some(Gitoxide::All)); let gctx = GlobalContextBuilder::new() - .env("CARGO_UNSTABLE_GITOXIDE", "fetch,abc") + .env("CARGO_UNSTABLE_GITOXIDE_FETCH", "true") + .env("CARGO_UNSTABLE_GITOXIDE_CHECKOUT", "true") + .env("CARGO_UNSTABLE_GITOXIDE_INTERNAL_USE_GIT2", "true") .build(); + verify(gctx, Some(Gitoxide::All)); + // Test gitoxide configured as a bool value. + write_config_toml( + "\ + [unstable] + gitoxide = true + ", + ); + let gctx = GlobalContextBuilder::new().build(); assert_error( gctx.get::>("unstable") .unwrap_err(), "\ -error in environment variable `CARGO_UNSTABLE_GITOXIDE`: could not load config key `unstable.gitoxide` - -Caused by: - unstable 'gitoxide' only takes 'all' and 'fetch' and 'checkout' and 'internal-use-git2' as valid inputs, your can use 'all' to turn out all gitoxide features, for shallow fetches see `shallow-index,shallow-deps`", +data did not match any variant of untagged enum Gitoxide", ); - + // Test gitoxide configured as a string 'all'. write_config_toml( "\ [unstable] @@ -1979,61 +1986,27 @@ Caused by: ", ); let gctx = GlobalContextBuilder::new().build(); - verify(gctx, Some(GitoxideFeatures::all())); - - write_config_toml( + assert_error( + gctx.get::>("unstable") + .unwrap_err(), "\ - [unstable] - gitoxide = \"fetch\" - ", +data did not match any variant of untagged enum Gitoxide", ); - let gctx = GlobalContextBuilder::new().build(); - verify( - gctx, - Some(GitoxideFeatures { - fetch: true, - ..GitoxideFeatures::default() - }), - ); - - fn verify(gctx: GlobalContext, expect: Option) { - let unstable_flags = gctx - .get::>("unstable") - .unwrap() - .unwrap(); - assert_eq!(unstable_flags.gitoxide, expect); - } -} - -#[cargo_test] -fn gitoxide_features_as_table() { - // A single feature in ENV can't be specified - let gctx = GlobalContextBuilder::new() - .env("CARGO_UNSTABLE_GITOXIDE_SHALLOW_INDEX", "true") - .build(); - let unstable_flags = gctx - .get::>("unstable") - .unwrap(); - assert!(unstable_flags.unwrap().gitoxide.is_none()); - + // Test gitoxide some fields listed. write_config_toml( "\ [unstable.gitoxide] fetch = true - checkout = false - internal_use_git2 = false + checkout = true ", ); - let gctx = GlobalContextBuilder::new().build(); - verify( - gctx, - Some(GitoxideFeatures { - fetch: true, - checkout: false, - internal_use_git2: false, - }), + assert_error( + gctx.get::>("unstable") + .unwrap_err(), + "\ +data did not match any variant of untagged enum Gitoxide", ); - // Still warning missing a field. + // Test no fields listed. write_config_toml( "\ [unstable.gitoxide] @@ -2044,13 +2017,42 @@ fn gitoxide_features_as_table() { gctx.get::>("unstable") .unwrap_err(), "\ -error in [..]/.cargo/config.toml: could not load config key `unstable.gitoxide` +data did not match any variant of untagged enum Gitoxide", + ); -Caused by: - missing field `fetch`", + // Test gitoxide didn't configured. + write_config_toml( + "\ + [unstable] + ", + ); + assert_error( + gctx.get::>("unstable") + .unwrap_err(), + "\ +data did not match any variant of untagged enum Gitoxide", + ); + // Test gitoxide all fields listed. + write_config_toml( + "\ + [unstable.gitoxide] + fetch = true + checkout = true + internal_use_git2 = true + ", + ); + let gctx = GlobalContextBuilder::new().build(); + verify( + gctx, + Some(Gitoxide::Some(GitoxideFeatures { + fetch: true, + checkout: true, + internal_use_git2: true, + })), ); - fn verify(gctx: GlobalContext, expect: Option) { + fn verify(gctx: GlobalContext, expect: Option) { + println!("expect: {:?}", expect); let unstable_flags = gctx .get::>("unstable") .unwrap()