Skip to content

Commit

Permalink
feat: use enum instead
Browse files Browse the repository at this point in the history
  • Loading branch information
linyihai committed May 11, 2024
1 parent b5fd826 commit 7592481
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 119 deletions.
65 changes: 17 additions & 48 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GitFeatures> = ("Enable support for shallow git fetch operations"),
#[serde(deserialize_with = "deserialize_gitoxide_features")]
gitoxide: Option<GitoxideFeatures> = ("Use gitoxide for the given git interactions, or all of them if no argument is given"),
gitoxide: Option<Gitoxide> = ("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"),
Expand Down Expand Up @@ -952,6 +951,14 @@ fn parse_git(it: impl Iterator<Item = impl AsRef<str>>) -> CargoResult<Option<Gi
Ok(Some(out))
}

#[derive(Debug, Copy, Clone, Default, Deserialize, Ord, PartialOrd, Eq, PartialEq)]
#[serde(untagged)]
pub enum Gitoxide {
#[default]
All,
Some(GitoxideFeatures),
}

#[derive(Debug, Copy, Clone, Default, Deserialize, Ord, PartialOrd, Eq, PartialEq)]
pub struct GitoxideFeatures {
/// All fetches are done with `gitoxide`, which includes git dependencies as well as the crates index.
Expand Down Expand Up @@ -993,47 +1000,6 @@ impl GitoxideFeatures {
}
}

fn deserialize_gitoxide_features<'de, D>(
deserializer: D,
) -> Result<Option<GitoxideFeatures>, D::Error>
where
D: serde::de::Deserializer<'de>,
{
struct GitoxideFeaturesVisitor;

impl<'de> serde::de::Visitor<'de> for GitoxideFeaturesVisitor {
type Value = Option<GitoxideFeatures>;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str(&GitoxideFeatures::expecting())
}

fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(parse_gitoxide(s.split(",")).map_err(serde::de::Error::custom)?)
}

fn visit_none<E>(self) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(None)
}

fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
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<Item = impl AsRef<str>>,
) -> CargoResult<Option<GitoxideFeatures>> {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)?,
Expand Down
6 changes: 5 additions & 1 deletion src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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();
Expand Down
142 changes: 72 additions & 70 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -1936,104 +1936,77 @@ 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::<Option<cargo::core::CliUnstable>>("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::<Option<cargo::core::CliUnstable>>("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]
gitoxide = 'all'
",
);
let gctx = GlobalContextBuilder::new().build();
verify(gctx, Some(GitoxideFeatures::all()));

write_config_toml(
assert_error(
gctx.get::<Option<cargo::core::CliUnstable>>("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<GitoxideFeatures>) {
let unstable_flags = gctx
.get::<Option<cargo::core::CliUnstable>>("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::<Option<cargo::core::CliUnstable>>("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::<Option<cargo::core::CliUnstable>>("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]
Expand All @@ -2044,13 +2017,42 @@ fn gitoxide_features_as_table() {
gctx.get::<Option<cargo::core::CliUnstable>>("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::<Option<cargo::core::CliUnstable>>("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<GitoxideFeatures>) {
fn verify(gctx: GlobalContext, expect: Option<Gitoxide>) {
println!("expect: {:?}", expect);
let unstable_flags = gctx
.get::<Option<cargo::core::CliUnstable>>("unstable")
.unwrap()
Expand Down

0 comments on commit 7592481

Please sign in to comment.