From 7dea40a99a79c843b153b6a4d8efad1f71c71ef4 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 10 Jul 2023 13:37:41 +1000 Subject: [PATCH] Support `--registry` and more options from `.cargo/config.toml` (#1195) Fixed #885 Now we can take advantage of new argument `--registry` and env overrides: - `CARGO_REGISTRIES_DEFAULT` if `--registry` is not specified - `CARGO_REGISTRIES_{registry_name}_INDEX` for the registry index url We can also read from `.cargo/config.toml` for: - default registry and registries configurations - additional CA bundle `http.cainfo` Signed-off-by: Jiahao XU --- crates/bin/src/args.rs | 32 +++- crates/bin/src/entry.rs | 62 ++++++-- crates/bin/src/install_path.rs | 29 ++-- crates/binstalk-manifests/src/cargo_config.rs | 143 ++++++++++++++---- crates/binstalk/src/errors.rs | 29 +++- e2e-tests/registries.sh | 62 ++++++++ justfile | 3 +- 7 files changed, 297 insertions(+), 63 deletions(-) create mode 100644 e2e-tests/registries.sh diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index 1f9f40b8..74584025 100644 --- a/crates/bin/src/args.rs +++ b/crates/bin/src/args.rs @@ -223,10 +223,28 @@ pub struct Args { #[clap(help_heading = "Options", long, alias = "roots")] pub root: Option, - /// The URL of the registry index to use + /// The URL of the registry index to use. + /// + /// Cannot be used with `--registry`. #[clap(help_heading = "Options", long)] pub index: Option, + /// Name of the registry to use. Registry names are defined in Cargo config + /// files . + /// + /// If not specified in cmdline or via environment variable, the default + /// registry is used, which is defined by the + /// `registry.default` config key in `.cargo/config.toml` which defaults + /// to crates-io. + /// + /// If it is set, then it will try to read environment variable + /// `CARGO_REGISTRIES_{registry_name}_INDEX` for index url and fallback to + /// reading from `registries..index`. + /// + /// Cannot be used with `--index`. + #[clap(help_heading = "Options", long, env = "CARGO_REGISTRY_DEFAULT")] + pub registry: Option, + /// This option will be passed through to all `cargo-install` invocations. /// /// It will require `Cargo.lock` to be up to date. @@ -421,6 +439,18 @@ You cannot use --manifest-path and --git. Do one or the other."# .exit(); } + if opts.index.is_some() && opts.registry.is_some() { + command + .error( + ErrorKind::ArgumentConflict, + format_args!( + r#"Multiple override options for registry. +You cannot use --index and --registry. Do one or the other."# + ), + ) + .exit(); + } + if opts.crate_names.len() > 1 { let option = if opts.version_req.is_some() { "version" diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs index 3ffa7026..4fa03672 100644 --- a/crates/bin/src/entry.rs +++ b/crates/bin/src/entry.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Cow, env, fs, future::Future, path::{Path, PathBuf}, @@ -16,12 +17,14 @@ use binstalk::{ remote::{Certificate, Client}, tasks::AutoAbortJoinHandle, }, + home::cargo_home, ops::{ self, resolve::{CrateName, Resolution, ResolutionFetch, VersionReqExt}, CargoTomlFetchOverride, Options, Resolver, }, }; +use binstalk_manifests::cargo_config::Config; use binstalk_manifests::cargo_toml_binstall::PkgOverride; use file_format::FileFormat; use log::LevelFilter; @@ -56,10 +59,19 @@ pub fn install_crates( }) .collect(); + // Load .cargo/config.toml + let cargo_home = cargo_home().map_err(BinstallError::from)?; + let mut config = Config::load_from_path(cargo_home.join("config.toml"))?; + // Compute paths let cargo_root = args.root; - let (install_path, mut manifests, temp_dir) = - compute_paths_and_load_manifests(cargo_root.clone(), args.install_path, args.no_track)?; + let (install_path, mut manifests, temp_dir) = compute_paths_and_load_manifests( + cargo_root.clone(), + args.install_path, + args.no_track, + cargo_home, + &mut config, + )?; // Remove installed crates let mut crate_names = @@ -83,12 +95,17 @@ pub fn install_crates( // Initialize reqwest client let rate_limit = args.rate_limit; + let mut http = config.http.take(); + let client = Client::new( concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")), args.min_tls_version.map(|v| v.into()), Duration::from_millis(rate_limit.duration.get()), rate_limit.request_count, - read_root_certs(args.root_certificates), + read_root_certs( + args.root_certificates, + http.as_mut().and_then(|http| http.cainfo.take()), + ), ) .map_err(BinstallError::from)?; @@ -127,7 +144,27 @@ pub fn install_crates( client, gh_api_client, jobserver_client, - registry: args.index.unwrap_or_default(), + registry: if let Some(index) = args.index { + index + } else if let Some(registry_name) = args + .registry + .or_else(|| config.registry.map(|registry| registry.default)) + { + env::var(format!("CARGO_REGISTRIES_{registry_name}_INDEX")) + .map(Cow::Owned) + .or_else(|_| { + config + .registries + .as_ref() + .and_then(|registries| registries.get(®istry_name)) + .map(|registry| Cow::Borrowed(registry.index.as_str())) + .ok_or_else(|| BinstallError::UnknownRegistryName(registry_name)) + })? + .parse() + .map_err(BinstallError::from)? + } else { + Default::default() + }, }); // Destruct args before any async function to reduce size of the future @@ -225,9 +262,13 @@ fn do_read_root_cert(path: &Path) -> Result, BinstallError> open_cert(&buffer).map_err(From::from).map(Some) } -fn read_root_certs(root_certificate_paths: Vec) -> impl Iterator { +fn read_root_certs( + root_certificate_paths: Vec, + config_cainfo: Option, +) -> impl Iterator { root_certificate_paths .into_iter() + .chain(config_cainfo) .filter_map(|path| match do_read_root_cert(&path) { Ok(optional_cert) => optional_cert, Err(err) => { @@ -245,12 +286,15 @@ fn compute_paths_and_load_manifests( roots: Option, install_path: Option, no_track: bool, + cargo_home: PathBuf, + config: &mut Config, ) -> Result<(PathBuf, Option, tempfile::TempDir)> { // Compute cargo_roots - let cargo_roots = install_path::get_cargo_roots_path(roots).ok_or_else(|| { - error!("No viable cargo roots path found of specified, try `--roots`"); - miette!("No cargo roots path found or specified") - })?; + let cargo_roots = + install_path::get_cargo_roots_path(roots, cargo_home, config).ok_or_else(|| { + error!("No viable cargo roots path found of specified, try `--roots`"); + miette!("No cargo roots path found or specified") + })?; // Compute install directory let (install_path, custom_install_path) = diff --git a/crates/bin/src/install_path.rs b/crates/bin/src/install_path.rs index f37313fc..b5dea041 100644 --- a/crates/bin/src/install_path.rs +++ b/crates/bin/src/install_path.rs @@ -3,11 +3,14 @@ use std::{ path::{Path, PathBuf}, }; -use binstalk::home::cargo_home; use binstalk_manifests::cargo_config::Config; use tracing::debug; -pub fn get_cargo_roots_path(cargo_roots: Option) -> Option { +pub fn get_cargo_roots_path( + cargo_roots: Option, + cargo_home: PathBuf, + config: &mut Config, +) -> Option { if let Some(p) = cargo_roots { Some(p) } else if let Some(p) = var_os("CARGO_INSTALL_ROOT") { @@ -15,24 +18,12 @@ pub fn get_cargo_roots_path(cargo_roots: Option) -> Option { let p = PathBuf::from(p); debug!("using CARGO_INSTALL_ROOT ({})", p.display()); Some(p) - } else if let Ok(cargo_home) = cargo_home() { - let config_path = cargo_home.join("config.toml"); - if let Some(root) = Config::load_from_path(&config_path) - .ok() - .and_then(|config| config.install.root) - { - debug!( - "using `install.root` {} from config {}", - root.display(), - config_path.display() - ); - Some(root) - } else { - debug!("using ({}) as cargo home", cargo_home.display()); - Some(cargo_home) - } + } else if let Some(root) = config.install.take().and_then(|install| install.root) { + debug!("using `install.root` {} from cargo config", root.display()); + Some(root) } else { - None + debug!("using ({}) as cargo home", cargo_home.display()); + Some(cargo_home) } } diff --git a/crates/binstalk-manifests/src/cargo_config.rs b/crates/binstalk-manifests/src/cargo_config.rs index f84b1887..195650eb 100644 --- a/crates/binstalk-manifests/src/cargo_config.rs +++ b/crates/binstalk-manifests/src/cargo_config.rs @@ -5,6 +5,8 @@ //! Binstall reads from them to be compatible with `cargo-install`'s behavior. use std::{ + borrow::Cow, + collections::BTreeMap, fs::File, io, path::{Path, PathBuf}, @@ -17,39 +19,57 @@ use miette::Diagnostic; use serde::Deserialize; use thiserror::Error; -#[derive(Debug, Default, Deserialize)] +#[derive(Debug, Deserialize)] pub struct Install { /// `cargo install` destination directory pub root: Option, } -#[derive(Debug, Default, Deserialize)] +#[derive(Debug, Deserialize)] pub struct Http { /// HTTP proxy in libcurl format: "host:port" + /// + /// env: CARGO_HTTP_PROXY or HTTPS_PROXY or https_proxy or http_proxy pub proxy: Option, /// timeout for each HTTP request, in seconds + /// + /// env: CARGO_HTTP_TIMEOUT or HTTP_TIMEOUT pub timeout: Option, /// path to Certificate Authority (CA) bundle pub cainfo: Option, - // TODO: - // Support field ssl-version, ssl-version.max, ssl-version.min, - // which needs `toml_edit::Item`. +} + +#[derive(Eq, PartialEq, Debug, Deserialize)] +#[serde(untagged)] +pub enum Env { + Value(CompactString), + WithOptions { + value: CompactString, + force: Option, + relative: Option, + }, +} + +#[derive(Debug, Deserialize)] +pub struct Registry { + pub index: CompactString, +} + +#[derive(Debug, Deserialize)] +pub struct DefaultRegistry { + pub default: CompactString, } #[derive(Debug, Default, Deserialize)] pub struct Config { - pub install: Install, - pub http: Http, - // TODO: - // Add support for section patch, source and registry for alternative - // crates.io registry. - - // TODO: - // Add field env for specifying env vars - // which needs `toml_edit::Item`. + pub install: Option, + pub http: Option, + pub env: Option>, + pub registries: Option>, + pub registry: Option, } -fn join_if_relative(path: &mut Option, dir: &Path) { +fn join_if_relative(path: Option<&mut PathBuf>, dir: &Path) { match path { Some(path) if path.is_relative() => *path = dir.join(&path), _ => (), @@ -81,8 +101,32 @@ impl Config { Ok(Default::default()) } else { let mut config: Config = toml_edit::de::from_slice(&vec)?; - join_if_relative(&mut config.install.root, dir); - join_if_relative(&mut config.http.cainfo, dir); + join_if_relative( + config + .install + .as_mut() + .and_then(|install| install.root.as_mut()), + dir, + ); + join_if_relative( + config.http.as_mut().and_then(|http| http.cainfo.as_mut()), + dir, + ); + if let Some(envs) = config.env.as_mut() { + for env in envs.values_mut() { + if let Env::WithOptions { + value, + relative: Some(true), + .. + } = env + { + let path = Cow::Borrowed(Path::new(&value)); + if path.is_relative() { + *value = dir.join(&path).to_string_lossy().into(); + } + } + } + } Ok(config) } } @@ -91,10 +135,19 @@ impl Config { } pub fn load_from_path(path: impl AsRef) -> Result { - let path = path.as_ref(); - let file = FileLock::new_shared(File::open(path)?)?; - // Any regular file must have a parent dir - Self::load_from_reader(file, path.parent().unwrap()) + fn inner(path: &Path) -> Result { + match File::open(path) { + Ok(file) => { + let file = FileLock::new_shared(file)?; + // Any regular file must have a parent dir + Config::load_from_reader(file, path.parent().unwrap()) + } + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(Default::default()), + Err(err) => Err(err.into()), + } + } + + inner(path.as_ref()) } } @@ -114,11 +167,19 @@ impl From for ConfigLoadError { } } +impl From for ConfigLoadError { + fn from(e: toml_edit::TomlError) -> Self { + ConfigLoadError::TomlParse(Box::new(e.into())) + } +} + #[cfg(test)] mod tests { use super::*; - use std::io::Cursor; + use std::{io::Cursor, path::MAIN_SEPARATOR}; + + use compact_str::format_compact; const CONFIG: &str = r#" [env] @@ -127,7 +188,7 @@ ENV_VAR_NAME = "value" # Set even if already present in environment ENV_VAR_NAME_2 = { value = "value", force = true } # Value is relative to .cargo directory containing `config.toml`, make absolute -ENV_VAR_NAME_3 = { value = "relative/path", relative = true } +ENV_VAR_NAME_3 = { value = "relative-path", relative = true } [http] debug = false # HTTP debugging @@ -141,21 +202,39 @@ root = "/some/path" # `cargo install` destination directory #[test] fn test_loading() { - let config = Config::load_from_reader(Cursor::new(&CONFIG), Path::new("/root")).unwrap(); + let config = Config::load_from_reader(Cursor::new(&CONFIG), Path::new("root")).unwrap(); assert_eq!( - config.install.root.as_deref().unwrap(), + config.install.unwrap().root.as_deref().unwrap(), Path::new("/some/path") ); - assert_eq!( - config.http.proxy, - Some(CompactString::new_inline("host:port")) - ); - assert_eq!(config.http.timeout, Some(30)); + let http = config.http.unwrap(); + assert_eq!(http.proxy.unwrap(), CompactString::new_inline("host:port")); + assert_eq!(http.timeout.unwrap(), 30); + assert_eq!(http.cainfo.unwrap(), Path::new("root").join("cert.pem")); + + let env = config.env.unwrap(); + assert_eq!(env.len(), 3); assert_eq!( - config.http.cainfo.as_deref().unwrap(), - Path::new("/root/cert.pem") + env.get("ENV_VAR_NAME").unwrap(), + &Env::Value(CompactString::new("value")) + ); + assert_eq!( + env.get("ENV_VAR_NAME_2").unwrap(), + &Env::WithOptions { + value: CompactString::new("value"), + force: Some(true), + relative: None, + } + ); + assert_eq!( + env.get("ENV_VAR_NAME_3").unwrap(), + &Env::WithOptions { + value: format_compact!("root{MAIN_SEPARATOR}relative-path"), + force: None, + relative: Some(true), + } ); } } diff --git a/crates/binstalk/src/errors.rs b/crates/binstalk/src/errors.rs index fa050304..86db0c61 100644 --- a/crates/binstalk/src/errors.rs +++ b/crates/binstalk/src/errors.rs @@ -15,7 +15,10 @@ use thiserror::Error; use tokio::task; use tracing::{error, warn}; -use crate::{drivers::RegistryError, helpers::cargo_toml_workspace::LoadManifestFromWSError}; +use crate::{ + drivers::{InvalidRegistryError, RegistryError}, + helpers::cargo_toml_workspace::LoadManifestFromWSError, +}; #[derive(Debug, Error)] #[error("version string '{v}' is not semver: {err}")] @@ -133,6 +136,14 @@ pub enum BinstallError { #[diagnostic(severity(error), code(binstall::io))] Io(io::Error), + /// Unknown registry name + /// + /// - Code: `binstall::cargo_registry` + /// - Exit: 75 + #[error("Unknown registry name {0}, env `CARGO_REGISTRIES_{0}_INDEX` nor is it in .cargo/config.toml")] + #[diagnostic(severity(error), code(binstall::cargo_registry))] + UnknownRegistryName(CompactString), + /// An error interacting with the crates.io API. /// /// This could either be a "not found" or a server/transport error. @@ -167,6 +178,14 @@ pub enum BinstallError { )] CargoManifest(Box), + /// Failure to parse registry index url + /// + /// - Code: `binstall::cargo_registry` + /// - Exit: 79 + #[error(transparent)] + #[diagnostic(severity(error), code(binstall::cargo_registry))] + RegistryParseError(#[from] Box), + /// A version is not valid semver. /// /// Note that we use the [`semver`] crate, which parses Cargo version syntax; this may be @@ -348,9 +367,11 @@ impl BinstallError { Download(_) => 68, SubProcess { .. } => 70, Io(_) => 74, + UnknownRegistryName(_) => 75, RegistryError { .. } => 76, CargoManifestPath => 77, CargoManifest { .. } => 78, + RegistryParseError(..) => 79, VersionParse { .. } => 80, VersionMismatch { .. } => 82, SuperfluousVersionOption => 84, @@ -473,3 +494,9 @@ impl From for BinstallError { BinstallError::RegistryError(Box::new(e)) } } + +impl From for BinstallError { + fn from(e: InvalidRegistryError) -> Self { + BinstallError::RegistryParseError(Box::new(e)) + } +} diff --git a/e2e-tests/registries.sh b/e2e-tests/registries.sh new file mode 100644 index 00000000..7eb3f041 --- /dev/null +++ b/e2e-tests/registries.sh @@ -0,0 +1,62 @@ +#!/bin/bash + +set -euxo pipefail + +test_cargo_binstall_install() { + # Test that the installed binaries can be run + cargo binstall --help >/dev/null + + cargo_binstall_version="$(cargo binstall -V)" + echo "$cargo_binstall_version" + + [ "$cargo_binstall_version" = "cargo-binstall 0.12.0" ] +} + +unset CARGO_INSTALL_ROOT + +CARGO_HOME="$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')" +export CARGO_HOME +export PATH="$CARGO_HOME/bin:$PATH" + +# Testing conflicts of `--index` and `--registry` +set +e + +"./$1" binstall --index 'sparse+https://index.crates.io/' --registry t1 cargo-binstall +exit_code="$?" + +set -e + +if [ "$exit_code" != 2 ]; then + echo "Expected exit code 2, but actual exit code $exit_code" + exit 1 +fi + +cat >"$CARGO_HOME/config.toml" << EOF +[registries] +t1 = { index = "https://github.com/rust-lang/crates.io-index" } +t2 = { index = "sparse+https://index.crates.io/" } + +[registry] +default = "t1" +EOF + +# Install binaries using default registry in config +"./$1" binstall --force -y cargo-binstall@0.12.0 + +test_cargo_binstall_install + +# Install binaries using registry t2 in config +"./$1" binstall --force --registry t2 -y cargo-binstall@0.12.0 + +test_cargo_binstall_install + +# Install binaries using registry t3 in env +CARGO_REGISTRIES_t3_INDEX='sparse+https://index.crates.io/' "./$1" binstall --force --registry t3 -y cargo-binstall@0.12.0 + +test_cargo_binstall_install + + +# Install binaries using index directly +"./$1" binstall --force --index 'sparse+https://index.crates.io/' -y cargo-binstall@0.12.0 + +test_cargo_binstall_install diff --git a/justfile b/justfile index 17b9a51a..7b5109f7 100644 --- a/justfile +++ b/justfile @@ -213,6 +213,7 @@ e2e-test-self-upgrade-no-symlink: (e2e-test "self-upgrade-no-symlink") e2e-test-uninstall: (e2e-test "uninstall") e2e-test-no-track: (e2e-test "no-track") e2e-test-git: (e2e-test "git") +e2e-test-registries: (e2e-test "registries") # WinTLS (Windows in CI) does not have TLS 1.3 support [windows] @@ -221,7 +222,7 @@ e2e-test-tls: (e2e-test "tls" "1.2") [macos] e2e-test-tls: (e2e-test "tls" "1.2") (e2e-test "tls" "1.3") -e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-git e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate e2e-test-no-track +e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-git e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate e2e-test-no-track e2e-test-registries unit-tests: print-env {{cargo-bin}} test {{cargo-build-args}}