From 70f84bf3b04ddc3806c31299e92af172ce1f0c63 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 19 Dec 2018 17:40:01 -0800 Subject: [PATCH] Rewrite `login` and registry cleanups. --- src/bin/cargo/commands/login.rs | 52 +++------------- src/bin/cargo/commands/package.rs | 1 - src/cargo/ops/cargo_package.rs | 1 - src/cargo/ops/registry.rs | 77 ++++++++++++++++------- src/cargo/sources/registry/mod.rs | 1 + src/crates-io/lib.rs | 11 +++- tests/testsuite/alt_registry.rs | 100 ++++++++++++++++++++++++++++-- tests/testsuite/login.rs | 41 +++--------- tests/testsuite/main.rs | 2 +- tests/testsuite/registry.rs | 16 +++-- 10 files changed, 188 insertions(+), 114 deletions(-) diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index eef05526cfe..5d76e7e12fd 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -1,11 +1,6 @@ use crate::command_prelude::*; -use std::io::{self, BufRead}; - -use cargo::core::{Source, SourceId}; use cargo::ops; -use cargo::sources::RegistrySource; -use cargo::util::CargoResultExt; pub fn cli() -> App { subcommand("login") @@ -14,46 +9,19 @@ pub fn cli() -> App { If token is not specified, it will be read from stdin.", ) .arg(Arg::with_name("token")) - .arg(opt("host", "Host to set the token for").value_name("HOST")) + .arg( + opt("host", "Host to set the token for") + .value_name("HOST") + .hidden(true), + ) .arg(opt("registry", "Registry to use").value_name("REGISTRY")) } pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { - let registry = args.registry(config)?; - - let token = match args.value_of("token") { - Some(token) => token.to_string(), - None => { - let host = match registry { - Some(ref _registry) => { - return Err(failure::format_err!( - "token must be provided when \ - --registry is provided." - ) - .into()); - } - None => { - let src = SourceId::crates_io(config)?; - let mut src = RegistrySource::remote(src, config); - src.update()?; - let config = src.config()?.unwrap(); - args.value_of("host") - .map(|s| s.to_string()) - .unwrap_or_else(|| config.api.unwrap()) - } - }; - println!("please visit {}/me and paste the API Token below", host); - let mut line = String::new(); - let input = io::stdin(); - input - .lock() - .read_line(&mut line) - .chain_err(|| "failed to read stdin") - .map_err(failure::Error::from)?; - line.trim().to_string() - } - }; - - ops::registry_login(config, token, registry)?; + ops::registry_login( + config, + args.value_of("token").map(String::from), + args.value_of("registry").map(String::from), + )?; Ok(()) } diff --git a/src/bin/cargo/commands/package.rs b/src/bin/cargo/commands/package.rs index 212c5ee93da..a1ef4793cea 100644 --- a/src/bin/cargo/commands/package.rs +++ b/src/bin/cargo/commands/package.rs @@ -42,7 +42,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { allow_dirty: args.is_present("allow-dirty"), target: args.target(), jobs: args.jobs()?, - registry: None, }, )?; Ok(()) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index e616c5136d8..c0200197a89 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -26,7 +26,6 @@ pub struct PackageOpts<'cfg> { pub verify: bool, pub jobs: Option, pub target: Option, - pub registry: Option, } static VCS_INFO_FILE: &'static str = ".cargo_vcs_info.json"; diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 8a1c4d6badd..dbb95fc5168 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; use std::fs::{self, File}; +use std::io::{self, BufRead}; use std::iter::repeat; use std::str; use std::time::Duration; @@ -65,6 +66,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { opts.token.clone(), opts.index.clone(), opts.registry.clone(), + true, )?; verify_dependencies(pkg, reg_id)?; @@ -80,7 +82,6 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { allow_dirty: opts.allow_dirty, target: opts.target.clone(), jobs: opts.jobs, - registry: opts.registry.clone(), }, )? .unwrap(); @@ -320,6 +321,7 @@ pub fn registry( token: Option, index: Option, registry: Option, + force_update: bool, ) -> CargoResult<(Registry, SourceId)> { // Parse all configuration options let RegistryConfig { @@ -330,9 +332,17 @@ pub fn registry( let sid = get_source_id(config, index_config.or(index), registry)?; let api_host = { let mut src = RegistrySource::remote(sid, config); - src.update() - .chain_err(|| format!("failed to update {}", sid))?; - (src.config()?).unwrap().api.unwrap() + // Only update the index if the config is not available or `force` is set. + let cfg = src.config(); + let cfg = if force_update || cfg.is_err() { + src.update() + .chain_err(|| format!("failed to update {}", sid))?; + cfg.or_else(|_| src.config())? + } else { + cfg.unwrap() + }; + cfg.and_then(|cfg| cfg.api) + .ok_or_else(|| failure::format_err!("{} does not support API commands", sid))? }; let handle = http_handle(config)?; Ok((Registry::new_handle(api_host, token, handle), sid)) @@ -503,18 +513,51 @@ fn http_proxy_exists(config: &Config) -> CargoResult { } } -pub fn registry_login(config: &Config, token: String, registry: Option) -> CargoResult<()> { +pub fn registry_login( + config: &Config, + token: Option, + reg: Option, +) -> CargoResult<()> { + let (registry, _) = registry(config, token.clone(), None, reg.clone(), false)?; + + let token = match token { + Some(token) => token, + None => { + println!( + "please visit {}/me and paste the API Token below", + registry.host() + ); + let mut line = String::new(); + let input = io::stdin(); + input + .lock() + .read_line(&mut line) + .chain_err(|| "failed to read stdin") + .map_err(failure::Error::from)?; + line.trim().to_string() + } + }; + let RegistryConfig { token: old_token, .. - } = registry_configuration(config, registry.clone())?; + } = registry_configuration(config, reg.clone())?; if let Some(old_token) = old_token { if old_token == token { + config.shell().status("Login", "already logged in")?; return Ok(()); } } - config::save_credentials(config, token, registry) + config::save_credentials(config, token, reg.clone())?; + config.shell().status( + "Login", + format!( + "token for `{}` saved", + reg.as_ref().map_or("crates.io", String::as_str) + ), + )?; + Ok(()) } pub struct OwnersOptions { @@ -542,6 +585,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { opts.token.clone(), opts.index.clone(), opts.registry.clone(), + true, )?; if let Some(ref v) = opts.to_add { @@ -602,7 +646,7 @@ pub fn yank( None => failure::bail!("a version must be specified to yank"), }; - let (mut registry, _) = registry(config, token, index, reg)?; + let (mut registry, _) = registry(config, token, index, reg, true)?; if undo { config @@ -658,22 +702,7 @@ pub fn search( prefix } - let sid = get_source_id(config, index, reg)?; - - let mut regsrc = RegistrySource::remote(sid, config); - let cfg = match regsrc.config() { - Ok(c) => c, - Err(_) => { - regsrc - .update() - .chain_err(|| format!("failed to update {}", &sid))?; - regsrc.config()? - } - }; - - let api_host = cfg.unwrap().api.unwrap(); - let handle = http_handle(config)?; - let mut registry = Registry::new_handle(api_host, None, handle); + let (mut registry, _) = registry(config, None, index, reg, false)?; let (crates, total_crates) = registry .search(query, limit) .chain_err(|| "failed to retrieve search results from the registry")?; diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 2c618c904f0..2771a1821a1 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -211,6 +211,7 @@ pub struct RegistryConfig { /// API endpoint for the registry. This is what's actually hit to perform /// operations like yanks, owner modifications, publish new crates, etc. + /// If this is None, the registry does not support API commands. pub api: Option, } diff --git a/src/crates-io/lib.rs b/src/crates-io/lib.rs index 15a23f1b536..342442519a5 100644 --- a/src/crates-io/lib.rs +++ b/src/crates-io/lib.rs @@ -15,8 +15,12 @@ use url::percent_encoding::{percent_encode, QUERY_ENCODE_SET}; pub type Result = std::result::Result; pub struct Registry { + /// The base URL for issuing API requests. host: String, + /// Optional authorization token. + /// If None, commands requiring authorization will fail. token: Option, + /// Curl handle for issuing requests. handle: Easy, } @@ -51,7 +55,8 @@ pub struct NewCrate { pub license_file: Option, pub repository: Option, pub badges: BTreeMap>, - #[serde(default)] pub links: Option, + #[serde(default)] + pub links: Option, } #[derive(Serialize)] @@ -131,6 +136,10 @@ impl Registry { } } + pub fn host(&self) -> &str { + &self.host + } + pub fn add_owners(&mut self, krate: &str, owners: &[&str]) -> Result { let body = serde_json::to_string(&OwnersReq { users: owners })?; let body = self.put(&format!("/crates/{}/owners", krate), body.as_bytes())?; diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index ce0728497bd..6906395ad6e 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -1,6 +1,6 @@ use crate::support::registry::{self, alt_api_path, Package}; -use crate::support::{basic_manifest, paths, project}; -use std::fs::File; +use crate::support::{basic_manifest, git, paths, project}; +use std::fs::{self, File}; use std::io::Write; #[test] @@ -203,13 +203,13 @@ fn depend_on_alt_registry_depends_on_crates_io() { p.cargo("build") .masquerade_as_nightly_cargo() - .with_stderr(&format!( + .with_stderr_unordered(&format!( "\ [UPDATING] `{alt_reg}` index [UPDATING] `{reg}` index [DOWNLOADING] crates ... -[DOWNLOADED] [..] v0.0.1 (registry `[ROOT][..]`) -[DOWNLOADED] [..] v0.0.1 (registry `[ROOT][..]`) +[DOWNLOADED] baz v0.0.1 (registry `[ROOT][..]`) +[DOWNLOADED] bar v0.0.1 (registry `[ROOT][..]`) [COMPILING] baz v0.0.1 (registry `[ROOT][..]`) [COMPILING] bar v0.0.1 (registry `[ROOT][..]`) [COMPILING] foo v0.0.1 ([CWD]) @@ -605,3 +605,93 @@ Caused by: .run(); } } + +#[test] +fn no_api() { + Package::new("bar", "0.0.1").alternative(true).publish(); + // Configure without `api`. + let repo = git2::Repository::open(registry::alt_registry_path()).unwrap(); + let cfg_path = registry::alt_registry_path().join("config.json"); + fs::write( + cfg_path, + format!(r#"{{"dl": "{}"}}"#, registry::alt_dl_url()), + ) + .unwrap(); + git::add(&repo); + git::commit(&repo); + + // First check that a dependency works. + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["alternative-registries"] + [package] + name = "foo" + version = "0.0.1" + + [dependencies.bar] + version = "0.0.1" + registry = "alternative" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_stderr(&format!( + "\ +[UPDATING] `{reg}` index +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.1 (registry `[ROOT][..]`) +[COMPILING] bar v0.0.1 (registry `[ROOT][..]`) +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + reg = registry::alt_registry_path().to_str().unwrap() + )) + .run(); + + // Check all of the API commands. + let err = format!( + "[ERROR] registry `{}` does not support API commands", + registry::alt_registry_path().display() + ); + + p.cargo("login --registry alternative TOKEN -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains(&err) + .run(); + + p.cargo("publish --registry alternative -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains(&err) + .run(); + + p.cargo("search --registry alternative -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains(&err) + .run(); + + p.cargo("owner --registry alternative -Zunstable-options --list") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains(&err) + .run(); + + p.cargo("yank --registry alternative -Zunstable-options --vers=0.0.1 bar") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains(&err) + .run(); + + p.cargo("yank --registry alternative -Zunstable-options --vers=0.0.1 bar") + .masquerade_as_nightly_cargo() + .with_stderr_contains(&err) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 443f4ea02fc..f75bb9ad766 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -3,26 +3,13 @@ use std::io::prelude::*; use crate::support::cargo_process; use crate::support::install::cargo_home; -use crate::support::registry::registry; +use crate::support::registry::{self, registry}; use cargo::core::Shell; use cargo::util::config::Config; use toml; const TOKEN: &str = "test-token"; const ORIGINAL_TOKEN: &str = "api-token"; -const CONFIG_FILE: &str = r#" - [registry] - token = "api-token" - - [registries.test-reg] - index = "http://dummy_index/" -"#; - -fn setup_old_credentials() { - let config = cargo_home().join("config"); - t!(fs::create_dir_all(config.parent().unwrap())); - t!(t!(File::create(&config)).write_all(CONFIG_FILE.as_bytes())); -} fn setup_new_credentials() { let config = cargo_home().join("credentials"); @@ -72,29 +59,20 @@ fn check_token(expected_token: &str, registry: Option<&str>) -> bool { #[test] fn login_with_old_credentials() { - setup_old_credentials(); + registry::init(); cargo_process("login --host") .arg(registry().to_string()) .arg(TOKEN) .run(); - let config = cargo_home().join("config"); - assert!(config.is_file()); - - let mut contents = String::new(); - File::open(&config) - .unwrap() - .read_to_string(&mut contents) - .unwrap(); - assert_eq!(CONFIG_FILE, contents); - // Ensure that we get the new token for the registry assert!(check_token(TOKEN, None)); } #[test] fn login_with_new_credentials() { + registry::init(); setup_new_credentials(); cargo_process("login --host") @@ -102,9 +80,6 @@ fn login_with_new_credentials() { .arg(TOKEN) .run(); - let config = cargo_home().join("config"); - assert!(!config.is_file()); - // Ensure that we get the new token for the registry assert!(check_token(TOKEN, None)); } @@ -117,21 +92,19 @@ fn login_with_old_and_new_credentials() { #[test] fn login_without_credentials() { + registry::init(); cargo_process("login --host") .arg(registry().to_string()) .arg(TOKEN) .run(); - let config = cargo_home().join("config"); - assert!(!config.is_file()); - // Ensure that we get the new token for the registry assert!(check_token(TOKEN, None)); } #[test] fn new_credentials_is_used_instead_old() { - setup_old_credentials(); + registry::init(); setup_new_credentials(); cargo_process("login --host") @@ -147,10 +120,10 @@ fn new_credentials_is_used_instead_old() { #[test] fn registry_credentials() { - setup_old_credentials(); + registry::init(); setup_new_credentials(); - let reg = "test-reg"; + let reg = "alternative"; cargo_process("login --registry") .arg(reg) diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 6100f5d6b2b..d170646999a 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -1,5 +1,5 @@ #![warn(rust_2018_idioms)] // while we're getting used to 2018 -#![cfg_attr(feature="deny-warnings", deny(warnings))] +#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![allow(clippy::blacklisted_name)] #![allow(clippy::explicit_iter_loop)] diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 2570f6fecbc..6383007254a 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -792,19 +792,25 @@ fn dev_dependency_not_used() { #[test] fn login_with_no_cargo_dir() { - let home = paths::home().join("new-home"); - t!(fs::create_dir(&home)); + // Create a config in the root directory because `login` requires the + // index to be updated, and we don't want to hit crates.io. + registry::init(); + fs::rename(paths::home().join(".cargo"), paths::root().join(".cargo")).unwrap(); + paths::home().rm_rf(); cargo_process("login foo -v").run(); + let credentials = fs::read_to_string(paths::home().join(".cargo/credentials")).unwrap(); + assert_eq!(credentials, "[registry]\ntoken = \"foo\"\n"); } #[test] fn login_with_differently_sized_token() { - // Verify that the configuration file gets properly trunchated. - let home = paths::home().join("new-home"); - t!(fs::create_dir(&home)); + // Verify that the configuration file gets properly truncated. + registry::init(); cargo_process("login lmaolmaolmao -v").run(); cargo_process("login lmao -v").run(); cargo_process("login lmaolmaolmao -v").run(); + let credentials = fs::read_to_string(paths::home().join(".cargo/credentials")).unwrap(); + assert_eq!(credentials, "[registry]\ntoken = \"lmaolmaolmao\"\n"); } #[test]