Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use RegistryOrIndex enum to replace two booleans #12677

Merged
merged 4 commits into from
Sep 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub fn cli() -> Command {
.about("Create a new cargo package in an existing directory")
.arg(Arg::new("path").action(ArgAction::Set).default_value("."))
.arg_new_opts()
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_registry("Registry to use")
.arg_quiet()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help init</>` for more detailed information.\n"
Expand Down
9 changes: 5 additions & 4 deletions src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
} else if krates.is_empty() {
from_cwd = true;
SourceId::for_path(config.cwd())?
} else if let Some(index) = args.get_one::<String>("index") {
SourceId::for_registry(&index.into_url()?)?
} else if let Some(registry) = args.registry(config)? {
SourceId::alt_registry(config, &registry)?
} else if let Some(reg_or_index) = args.registry_or_index(config)? {
match reg_or_index {
ops::RegistryOrIndex::Registry(r) => SourceId::alt_registry(config, &r)?,
ops::RegistryOrIndex::Index(url) => SourceId::for_registry(&url)?,
}
} else {
SourceId::crates_io(config)?
};
Expand Down
16 changes: 11 additions & 5 deletions src/bin/cargo/commands/login.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::command_prelude::*;

use cargo::ops;
use cargo::ops::RegistryOrIndex;

use crate::command_prelude::*;

pub fn cli() -> Command {
subcommand("login")
.about("Log in to a registry.")
.arg(Arg::new("token").action(ArgAction::Set))
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_registry("Registry to use")
.arg(
Arg::new("args")
.help("Arguments for the credential provider (unstable)")
Expand All @@ -20,7 +21,12 @@ pub fn cli() -> Command {
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
let reg = args.registry_or_index(config)?;
assert!(
!matches!(reg, Some(RegistryOrIndex::Index(..))),
"must not be index URL"
);

let extra_args = args
.get_many::<String>("args")
.unwrap_or_default()
Expand All @@ -29,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
ops::registry_login(
config,
args.get_one::<String>("token").map(|s| s.as_str().into()),
registry.as_deref(),
reg.as_ref(),
&extra_args,
)?;
Ok(())
Expand Down
15 changes: 11 additions & 4 deletions src/bin/cargo/commands/logout.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
use crate::command_prelude::*;
use cargo::ops;
use cargo::ops::RegistryOrIndex;

use crate::command_prelude::*;

pub fn cli() -> Command {
subcommand("logout")
.about("Remove an API token from the registry locally")
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_registry("Registry to use")
.arg_quiet()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help logout</>` for more detailed information.\n"
))
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
ops::registry_logout(config, registry.as_deref())?;
let reg = args.registry_or_index(config)?;
assert!(
!matches!(reg, Some(RegistryOrIndex::Index(..))),
"must not be index URL"
);

ops::registry_logout(config, reg)?;
Ok(())
}
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub fn cli() -> Command {
.about("Create a new cargo package at <path>")
.arg(Arg::new("path").action(ArgAction::Set).required(true))
.arg_new_opts()
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_registry("Registry to use")
.arg_quiet()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help new</>` for more detailed information.\n"
Expand Down
8 changes: 3 additions & 5 deletions src/bin/cargo/commands/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,27 @@ pub fn cli() -> Command {
.short('r'),
)
.arg(flag("list", "List owners of a crate").short('l'))
.arg(opt("index", "Registry index to modify owners for").value_name("INDEX"))
.arg_index("Registry index URL to modify owners for")
.arg_registry("Registry to modify owners for")
.arg(opt("token", "API token to use when authenticating").value_name("TOKEN"))
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_quiet()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help owner</>` for more detailed information.\n"
))
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
let opts = OwnersOptions {
krate: args.get_one::<String>("crate").cloned(),
token: args.get_one::<String>("token").cloned().map(Secret::from),
index: args.get_one::<String>("index").cloned(),
reg_or_index: args.registry_or_index(config)?,
to_add: args
.get_many::<String>("add")
.map(|xs| xs.cloned().collect()),
to_remove: args
.get_many::<String>("remove")
.map(|xs| xs.cloned().collect()),
list: args.flag("list"),
registry,
};
ops::modify_owners(config, &opts)?;
Ok(())
Expand Down
10 changes: 4 additions & 6 deletions src/bin/cargo/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ pub fn cli() -> Command {
subcommand("publish")
.about("Upload a package to the registry")
.arg_dry_run("Perform all checks without uploading")
.arg_index()
.arg(opt("registry", "Registry to publish to").value_name("REGISTRY"))
.arg_index("Registry index URL to upload the package to")
.arg_registry("Registry to upload the package to")
.arg(opt("token", "Token to use when uploading").value_name("TOKEN"))
.arg(flag(
"no-verify",
Expand All @@ -30,7 +30,7 @@ pub fn cli() -> Command {
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
let reg_or_index = args.registry_or_index(config)?;
let ws = args.workspace(config)?;
if ws.root_maybe().is_embedded() {
return Err(anyhow::format_err!(
Expand All @@ -39,7 +39,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
)
.into());
}
let index = args.index()?;

ops::publish(
&ws,
Expand All @@ -48,15 +47,14 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
token: args
.get_one::<String>("token")
.map(|s| s.to_string().into()),
index,
reg_or_index,
verify: !args.flag("no-verify"),
allow_dirty: args.flag("allow-dirty"),
to_publish: args.packages_from_flags()?,
targets: args.targets()?,
jobs: args.jobs()?,
keep_going: args.keep_going(),
dry_run: args.dry_run(),
registry,
cli_features: args.cli_features()?,
},
)?;
Expand Down
9 changes: 4 additions & 5 deletions src/bin/cargo/commands/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,16 @@ pub fn cli() -> Command {
)
.value_name("LIMIT"),
)
.arg_index()
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_index("Registry index URL to search packages in")
.arg_registry("Registry to search packages in")
.arg_quiet()
.after_help(color_print::cstr!(
"Run `<cyan,bold>cargo help search</>` for more detailed information.\n"
))
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
let index = args.index()?;
let reg_or_index = args.registry_or_index(config)?;
let limit = args.value_of_u32("limit")?;
let limit = min(100, limit.unwrap_or(10));
let query: Vec<&str> = args
Expand All @@ -34,6 +33,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
.map(String::as_str)
.collect();
let query: String = query.join("+");
ops::search(&query, config, index, limit, registry)?;
ops::search(&query, config, reg_or_index, limit)?;
Ok(())
}
9 changes: 3 additions & 6 deletions src/bin/cargo/commands/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ pub fn cli() -> Command {
"undo",
"Undo a yank, putting a version back into the index",
))
.arg(opt("index", "Registry index to yank from").value_name("INDEX"))
.arg(opt("registry", "Registry to use").value_name("REGISTRY"))
.arg_index("Registry index URL to yank from")
.arg_registry("Registry to yank from")
.arg(opt("token", "API token to use when authenticating").value_name("TOKEN"))
.arg_quiet()
.after_help(color_print::cstr!(
Expand All @@ -26,8 +26,6 @@ pub fn cli() -> Command {
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;

let (krate, version) = resolve_crate(
args.get_one::<String>("crate").map(String::as_str),
args.get_one::<String>("version").map(String::as_str),
Expand All @@ -41,9 +39,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
krate.map(|s| s.to_string()),
version.map(|s| s.to_string()),
args.get_one::<String>("token").cloned().map(Secret::from),
args.get_one::<String>("index").cloned(),
args.registry_or_index(config)?,
args.flag("undo"),
registry,
)?;
Ok(())
}
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub use self::registry::yank;
pub use self::registry::OwnersOptions;
pub use self::registry::PublishOpts;
pub use self::registry::RegistryCredentialConfig;
pub use self::registry::RegistryOrIndex;
pub use self::resolve::{
add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws, resolve_ws_with_opts,
WorkspaceResolve,
Expand Down
15 changes: 11 additions & 4 deletions src/cargo/ops/registry/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,23 @@ use cargo_credential::Secret;

use super::get_source_id;
use super::registry;
use super::RegistryOrIndex;

pub fn registry_login(
config: &Config,
token_from_cmdline: Option<Secret<&str>>,
reg: Option<&str>,
reg_or_index: Option<&RegistryOrIndex>,
args: &[&str],
) -> CargoResult<()> {
let source_ids = get_source_id(config, None, reg)?;

let login_url = match registry(config, token_from_cmdline.clone(), None, reg, false, None) {
let source_ids = get_source_id(config, reg_or_index)?;

let login_url = match registry(
config,
token_from_cmdline.clone(),
reg_or_index,
false,
None,
) {
Ok((registry, _)) => Some(format!("{}/me", registry.host())),
Err(e) if e.is::<AuthorizationError>() => e
.downcast::<AuthorizationError>()
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/ops/registry/logout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use crate::CargoResult;
use crate::Config;

use super::get_source_id;
use super::RegistryOrIndex;

pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> {
let source_ids = get_source_id(config, None, reg)?;
pub fn registry_logout(config: &Config, reg_or_index: Option<RegistryOrIndex>) -> CargoResult<()> {
let source_ids = get_source_id(config, reg_or_index.as_ref())?;
auth::logout(config, &source_ids.original)?;
Ok(())
}
36 changes: 24 additions & 12 deletions src/cargo/ops/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::task::Poll;
use anyhow::{bail, format_err, Context as _};
use cargo_credential::{Operation, Secret};
use crates_io::{self, Registry};
use url::Url;

use crate::core::SourceId;
use crate::sources::source::Source;
Expand All @@ -24,7 +25,6 @@ use crate::util::auth;
use crate::util::config::{Config, PathAndArgs};
use crate::util::errors::CargoResult;
use crate::util::network::http::http_handle;
use crate::util::IntoUrl;

pub use self::login::registry_login;
pub use self::logout::registry_logout;
Expand All @@ -35,6 +35,19 @@ pub use self::publish::PublishOpts;
pub use self::search::search;
pub use self::yank::yank;

/// Represents either `--registry` or `--index` argument, which is mutually exclusive.
#[derive(Debug, Clone)]
pub enum RegistryOrIndex {
Registry(String),
Index(Url),
}

impl RegistryOrIndex {
fn is_index(&self) -> bool {
matches!(self, RegistryOrIndex::Index(..))
}
}

/// Registry settings loaded from config files.
///
/// This is loaded based on the `--registry` flag and the config settings.
Expand Down Expand Up @@ -103,14 +116,14 @@ impl RegistryCredentialConfig {
fn registry(
config: &Config,
token_from_cmdline: Option<Secret<&str>>,
index: Option<&str>,
registry: Option<&str>,
reg_or_index: Option<&RegistryOrIndex>,
force_update: bool,
token_required: Option<Operation<'_>>,
) -> CargoResult<(Registry, RegistrySourceIds)> {
let source_ids = get_source_id(config, index, registry)?;
let source_ids = get_source_id(config, reg_or_index)?;

if token_required.is_some() && index.is_some() && token_from_cmdline.is_none() {
let is_index = reg_or_index.map(|v| v.is_index()).unwrap_or_default();
if is_index && token_required.is_some() && token_from_cmdline.is_none() {
bail!("command-line argument --index requires --token to be specified");
}
if let Some(token) = token_from_cmdline {
Expand Down Expand Up @@ -171,13 +184,12 @@ fn registry(
/// crates.io (such as index.crates.io), while the second is always the original source.
fn get_source_id(
config: &Config,
index: Option<&str>,
reg: Option<&str>,
reg_or_index: Option<&RegistryOrIndex>,
) -> CargoResult<RegistrySourceIds> {
let sid = match (reg, index) {
(None, None) => SourceId::crates_io(config)?,
(_, Some(i)) => SourceId::for_registry(&i.into_url()?)?,
(Some(r), None) => SourceId::alt_registry(config, r)?,
let sid = match reg_or_index {
None => SourceId::crates_io(config)?,
Some(RegistryOrIndex::Index(url)) => SourceId::for_registry(url)?,
Some(RegistryOrIndex::Registry(r)) => SourceId::alt_registry(config, r)?,
};
// Load source replacements that are built-in to Cargo.
let builtin_replacement_sid = SourceConfigMap::empty(config)?
Expand All @@ -186,7 +198,7 @@ fn get_source_id(
let replacement_sid = SourceConfigMap::new(config)?
.load(sid, &HashSet::new())?
.replaced_source_id();
if reg.is_none() && index.is_none() && replacement_sid != builtin_replacement_sid {
if reg_or_index.is_none() && replacement_sid != builtin_replacement_sid {
// Neither --registry nor --index was passed and the user has configured source-replacement.
if let Some(replacement_name) = replacement_sid.alt_registry_key() {
bail!("crates-io is replaced with remote registry {replacement_name};\ninclude `--registry {replacement_name}` or `--registry crates-io`");
Expand Down
Loading