From 77bfca27221d49fc31cb4d7dba5df9ac872e3a5f Mon Sep 17 00:00:00 2001 From: "Daniel Silverstone (WSL2)" Date: Sat, 29 Aug 2020 21:22:42 +0100 Subject: [PATCH 01/21] WIP: Thoughts on #2458 Signed-off-by: Daniel Silverstone --- src/config.rs | 40 ++++++++++++++++++++++++++++++---------- src/errors.rs | 8 ++++++++ src/toolchain.rs | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/config.rs b/src/config.rs index 39132643fe..7159a1dd04 100644 --- a/src/config.rs +++ b/src/config.rs @@ -33,6 +33,7 @@ impl OverrideFile { #[derive(Debug, Default, Deserialize, PartialEq, Eq)] struct ToolchainSection { channel: Option, + path: Option, components: Option>, targets: Option>, } @@ -45,11 +46,21 @@ impl ToolchainSection { impl> From for OverrideFile { fn from(channel: T) -> Self { - Self { - toolchain: ToolchainSection { - channel: Some(channel.into()), - ..Default::default() - }, + let channel = channel.into(); + if channel.contains('/') || channel.contains('\\') { + Self { + toolchain: ToolchainSection { + path: Some(channel), + ..Default::default() + }, + } + } else { + Self { + toolchain: ToolchainSection { + channel: Some(channel), + ..Default::default() + }, + } } } } @@ -73,7 +84,7 @@ impl Display for OverrideReason { } } -#[derive(Default)] +#[derive(Default, Debug)] struct OverrideCfg<'a> { toolchain: Option>, components: Vec, @@ -83,9 +94,13 @@ struct OverrideCfg<'a> { impl<'a> OverrideCfg<'a> { fn from_file(cfg: &'a Cfg, file: OverrideFile) -> Result { Ok(Self { - toolchain: match file.toolchain.channel { - Some(name) => Some(Toolchain::from(cfg, &name)?), - None => None, + toolchain: match (file.toolchain.channel, file.toolchain.path) { + (Some(name), None) => Some(Toolchain::from(cfg, &name)?), + (None, Some(path)) => Some(Toolchain::from_path(cfg, &path)?), + (Some(channel), Some(path)) => { + return Err(ErrorKind::CannotSpecifyChannelAndPath(channel, path.into()).into()) + } + (None, None) => None, }, components: file.toolchain.components.unwrap_or_default(), targets: file.toolchain.targets.unwrap_or_default(), @@ -521,7 +536,6 @@ impl Cfg { path.display() ), }; - let override_cfg = OverrideCfg::from_file(self, file)?; if let Some(toolchain) = &override_cfg.toolchain { // Overridden toolchains can be literally any string, but only @@ -889,6 +903,7 @@ mod tests { OverrideFile { toolchain: ToolchainSection { channel: Some(contents.into()), + path: None, components: None, targets: None, } @@ -910,6 +925,7 @@ targets = [ "wasm32-unknown-unknown", "thumbv2-none-eabi" ] OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), + path: None, components: Some(vec!["rustfmt".into(), "rustc-dev".into()]), targets: Some(vec![ "wasm32-unknown-unknown".into(), @@ -932,6 +948,7 @@ channel = "nightly-2020-07-10" OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), + path: None, components: None, targets: None, } @@ -952,6 +969,7 @@ components = [] OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), + path: None, components: Some(vec![]), targets: None, } @@ -972,6 +990,7 @@ targets = [] OverrideFile { toolchain: ToolchainSection { channel: Some("nightly-2020-07-10".into()), + path: None, components: None, targets: Some(vec![]), } @@ -991,6 +1010,7 @@ components = [ "rustfmt" ] OverrideFile { toolchain: ToolchainSection { channel: None, + path: None, components: Some(vec!["rustfmt".into()]), targets: None, } diff --git a/src/errors.rs b/src/errors.rs index 9fd6050608..6878fda7dd 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -188,6 +188,14 @@ error_chain! { description("invalid toolchain name") display("invalid toolchain name: '{}'", t) } + InvalidToolchainPath(p: PathBuf) { + description("invalid toolchain path"), + display("invalid toolchain path: '{}'", p.display()) + } + CannotSpecifyChannelAndPath(channel: String, path: PathBuf) { + description("cannot specify channel and path simultaneously"), + display("cannot specify both channel ({}) and path ({}) simultaneously", channel, path.display()) + } InvalidProfile(t: String) { description("invalid profile name") display("invalid profile name: '{}'; valid names are: {}", t, valid_profile_names()) diff --git a/src/toolchain.rs b/src/toolchain.rs index d1bb62102a..f76bf14212 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -63,6 +63,9 @@ pub enum UpdateStatus { impl<'a> Toolchain<'a> { pub fn from(cfg: &'a Cfg, name: &str) -> Result { + if name.contains('/') || name.contains('\\') { + return Self::from_path(cfg, name); + } let resolved_name = cfg.resolve_toolchain(name)?; let path = cfg.toolchains_dir.join(&resolved_name); Ok(Toolchain { @@ -73,6 +76,26 @@ impl<'a> Toolchain<'a> { }) } + pub fn from_path(cfg: &'a Cfg, path: impl AsRef) -> Result { + let path = path.as_ref(); + let base = path + .components() + .last() + .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.into()))? + .as_os_str() + .to_string_lossy(); + // Perform minimal validation - that there's a `bin/` which might contain things for us to run + if !path.join("bin").is_dir() { + return Err(ErrorKind::InvalidToolchainPath(path.into()).into()); + } + Ok(Toolchain { + cfg, + name: base.into(), + path: path.to_path_buf(), + dist_handler: Box::new(move |n| (cfg.notify_handler)(n.into())), + }) + } + pub fn as_installed_common(&'a self) -> Result> { if !self.exists() { // Should be verify perhaps? @@ -255,6 +278,15 @@ impl<'a> Toolchain<'a> { } } +impl<'a> std::fmt::Debug for Toolchain<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Toolchain") + .field("name", &self.name) + .field("path", &self.path) + .finish() + } +} + /// Newtype hosting functions that apply to both custom and distributable toolchains that are installed. pub struct InstalledCommonToolchain<'a>(&'a Toolchain<'a>); From 3d5bb9de4d78b08a8a6b31aa7ea75c54cffc1764 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 22 Feb 2021 16:03:32 -0800 Subject: [PATCH 02/21] Remove plain-toolchain path override --- src/config.rs | 20 +++++--------------- src/toolchain.rs | 3 --- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/config.rs b/src/config.rs index 4c58d85d20..570991ff0a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -47,21 +47,11 @@ impl ToolchainSection { impl> From for OverrideFile { fn from(channel: T) -> Self { - let channel = channel.into(); - if channel.contains('/') || channel.contains('\\') { - Self { - toolchain: ToolchainSection { - path: Some(channel), - ..Default::default() - }, - } - } else { - Self { - toolchain: ToolchainSection { - channel: Some(channel), - ..Default::default() - }, - } + Self { + toolchain: ToolchainSection { + channel: Some(channel.into()), + ..Default::default() + }, } } } diff --git a/src/toolchain.rs b/src/toolchain.rs index 936c15a905..e0a17dbc6c 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -64,9 +64,6 @@ pub enum UpdateStatus { impl<'a> Toolchain<'a> { pub fn from(cfg: &'a Cfg, name: &str) -> Result { - if name.contains('/') || name.contains('\\') { - return Self::from_path(cfg, name); - } let resolved_name = cfg.resolve_toolchain(name)?; let path = cfg.toolchains_dir.join(&resolved_name); Ok(Toolchain { From 376f9ede86a667900628fec66873aafc6fcba521 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 22 Feb 2021 16:03:55 -0800 Subject: [PATCH 03/21] Check path validity where channels are validated --- src/config.rs | 7 +++++++ src/toolchain.rs | 4 ---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/config.rs b/src/config.rs index 570991ff0a..ffa517e721 100644 --- a/src/config.rs +++ b/src/config.rs @@ -609,6 +609,13 @@ impl Cfg { dist::validate_channel_name(&toolchain_name)?; } } + if let Some(toolchain_path) = &override_file.toolchain.path { + // Perform minimal validation; there should at least be a `bin/` that might + // contain things for us to run. + if !toolchain_path.join("bin").is_dir() { + return Err(ErrorKind::InvalidToolchainPath(toolchain_path.into()).into()); + } + } let reason = OverrideReason::ToolchainFile(toolchain_file); return Ok(Some((override_file, reason))); diff --git a/src/toolchain.rs b/src/toolchain.rs index e0a17dbc6c..3b90953224 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -82,10 +82,6 @@ impl<'a> Toolchain<'a> { .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.into()))? .as_os_str() .to_string_lossy(); - // Perform minimal validation - that there's a `bin/` which might contain things for us to run - if !path.join("bin").is_dir() { - return Err(ErrorKind::InvalidToolchainPath(path.into()).into()); - } Ok(Toolchain { cfg, name: base.into(), From b70f54a52ca348610ab652556239741e180c5233 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 22 Feb 2021 16:04:24 -0800 Subject: [PATCH 04/21] Basic parser test --- src/config.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index ffa517e721..f0c8010ce5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -33,7 +33,7 @@ impl OverrideFile { #[derive(Debug, Default, Deserialize, PartialEq, Eq)] struct ToolchainSection { channel: Option, - path: Option, + path: Option, components: Option>, targets: Option>, profile: Option, @@ -41,7 +41,10 @@ struct ToolchainSection { impl ToolchainSection { fn is_empty(&self) -> bool { - self.channel.is_none() && self.components.is_none() && self.targets.is_none() + self.channel.is_none() + && self.components.is_none() + && self.targets.is_none() + && self.path.is_none() } } @@ -534,6 +537,7 @@ impl Cfg { path.display() ), }; + let override_cfg = OverrideCfg::from_file(self, file)?; if let Some(toolchain) = &override_cfg.toolchain { // Overridden toolchains can be literally any string, but only @@ -1023,6 +1027,27 @@ channel = "nightly-2020-07-10" ); } + #[test] + fn parse_toml_toolchain_file_only_path() { + let contents = r#"[toolchain] +path = "foobar" +"#; + + let result = Cfg::parse_override_file(contents, ParseMode::Both); + assert_eq!( + result.unwrap(), + OverrideFile { + toolchain: ToolchainSection { + channel: None, + path: Some("foobar".into()), + components: None, + targets: None, + profile: None, + } + } + ); + } + #[test] fn parse_toml_toolchain_file_empty_components() { let contents = r#"[toolchain] From 3077d7258da08362188237780c7a6365bef23a3a Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 22 Feb 2021 17:09:56 -0800 Subject: [PATCH 05/21] Also handle relative paths --- src/config.rs | 23 +++++++++++++---------- src/toolchain.rs | 23 +++++++++++++++++++---- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/config.rs b/src/config.rs index f0c8010ce5..02b913623e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -87,11 +87,15 @@ struct OverrideCfg<'a> { } impl<'a> OverrideCfg<'a> { - fn from_file(cfg: &'a Cfg, file: OverrideFile) -> Result { + fn from_file( + cfg: &'a Cfg, + cfg_path: Option>, + file: OverrideFile, + ) -> Result { Ok(Self { toolchain: match (file.toolchain.channel, file.toolchain.path) { (Some(name), None) => Some(Toolchain::from(cfg, &name)?), - (None, Some(path)) => Some(Toolchain::from_path(cfg, &path)?), + (None, Some(path)) => Some(Toolchain::from_path(cfg, cfg_path, &path)?), (Some(channel), Some(path)) => { return Err(ErrorKind::CannotSpecifyChannelAndPath(channel, path.into()).into()) } @@ -538,7 +542,13 @@ impl Cfg { ), }; - let override_cfg = OverrideCfg::from_file(self, file)?; + let cfg_file = if let OverrideReason::ToolchainFile(ref path) = reason { + Some(path) + } else { + None + }; + + let override_cfg = OverrideCfg::from_file(self, cfg_file, file)?; if let Some(toolchain) = &override_cfg.toolchain { // Overridden toolchains can be literally any string, but only // distributable toolchains will be auto-installed by the wrapping @@ -613,13 +623,6 @@ impl Cfg { dist::validate_channel_name(&toolchain_name)?; } } - if let Some(toolchain_path) = &override_file.toolchain.path { - // Perform minimal validation; there should at least be a `bin/` that might - // contain things for us to run. - if !toolchain_path.join("bin").is_dir() { - return Err(ErrorKind::InvalidToolchainPath(toolchain_path.into()).into()); - } - } let reason = OverrideReason::ToolchainFile(toolchain_file); return Ok(Some((override_file, reason))); diff --git a/src/toolchain.rs b/src/toolchain.rs index 3b90953224..0b36b48df2 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -74,18 +74,33 @@ impl<'a> Toolchain<'a> { }) } - pub fn from_path(cfg: &'a Cfg, path: impl AsRef) -> Result { - let path = path.as_ref(); + pub fn from_path( + cfg: &'a Cfg, + cfg_file: Option>, + path: impl AsRef, + ) -> Result { + let path = if let Some(cfg_file) = cfg_file { + cfg_file.as_ref().parent().unwrap().join(path) + } else { + path.as_ref().to_path_buf() + }; + + // Perform minimal validation; there should at least be a `bin/` that might + // contain things for us to run. + if !path.join("bin").is_dir() { + return Err(ErrorKind::InvalidToolchainPath(path.into()).into()); + } + let base = path .components() .last() - .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.into()))? + .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.clone()))? .as_os_str() .to_string_lossy(); Ok(Toolchain { cfg, name: base.into(), - path: path.to_path_buf(), + path, dist_handler: Box::new(move |n| (cfg.notify_handler)(n.into())), }) } From 32ca04ea17d2044132c3ece2304a0a99954a4c69 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 22 Feb 2021 17:10:23 -0800 Subject: [PATCH 06/21] Add tests for toolchain path overrides --- tests/cli-rustup.rs | 87 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 1ebfd7e53a..cde17fa794 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1384,6 +1384,93 @@ fn file_override() { }); } +#[test] +#[cfg_attr( + not(unix), + ignore = "TODO: Figure out how to write a wrapper toolchain on Windows" +)] +fn file_override_path() { + setup(&|config| { + let cwd = config.current_dir(); + let toolchain_path = cwd.join("ephemeral"); + let toolchain_bin = toolchain_path.join("bin"); + fs::create_dir_all(&toolchain_bin).unwrap(); + + // Inject a wrapper binary for rustc. + let rustc = toolchain_bin.join("rustc"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + raw::write_file(&rustc, "#!/bin/sh\necho custom-toolchain").unwrap(); + fs::set_permissions(rustc, fs::Permissions::from_mode(0o755)).unwrap(); + } + + let toolchain_file = cwd.join("rust-toolchain.toml"); + raw::write_file( + &toolchain_file, + &format!("[toolchain]\npath=\"{}\"", toolchain_path.display()), + ) + .unwrap(); + + expect_stdout_ok(config, &["rustc", "--version"], "custom-toolchain"); + }); +} + +#[test] +#[cfg_attr( + not(unix), + ignore = "TODO: Figure out how to write a wrapper toolchain on Windows" +)] +fn file_override_path_relative() { + setup(&|config| { + let cwd = config.current_dir(); + let toolchain_path = cwd.join("ephemeral"); + let toolchain_bin = toolchain_path.join("bin"); + fs::create_dir_all(&toolchain_bin).unwrap(); + + // Inject a wrapper binary for rustc. + let rustc = toolchain_bin.join("rustc"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + raw::write_file(&rustc, "#!/bin/sh\necho custom-toolchain").unwrap(); + fs::set_permissions(rustc, fs::Permissions::from_mode(0o755)).unwrap(); + } + + let toolchain_file = cwd.join("rust-toolchain.toml"); + raw::write_file(&toolchain_file, "[toolchain]\npath=\"ephemeral\"").unwrap(); + + // Change into ephemeral so that we actually test that the path is relative to the override + config.change_dir(&toolchain_path, || { + expect_stdout_ok(config, &["rustc", "--version"], "custom-toolchain") + }); + }); +} + +#[test] +fn file_override_path_xor_channel() { + setup(&|config| { + // Make a plausible-looking toolchain + let cwd = config.current_dir(); + let toolchain_path = cwd.join("ephemeral"); + let toolchain_bin = toolchain_path.join("bin"); + fs::create_dir_all(&toolchain_bin).unwrap(); + + let toolchain_file = cwd.join("rust-toolchain.toml"); + raw::write_file( + &toolchain_file, + "[toolchain]\npath=\"ephemeral\"\nchannel=\"nightly\"", + ) + .unwrap(); + + expect_err( + config, + &["rustc", "--version"], + "cannot specify both channel (nightly) and path (ephemeral) simultaneously", + ); + }); +} + #[test] fn file_override_subdir() { setup(&|config| { From abf9bced042546d2b4e4f1c33a4d49d35b8c4e0f Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 22 Feb 2021 17:21:16 -0800 Subject: [PATCH 07/21] Document path toolchain override --- doc/src/overrides.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/src/overrides.md b/doc/src/overrides.md index 7d36e7864d..e0c1d77029 100644 --- a/doc/src/overrides.md +++ b/doc/src/overrides.md @@ -83,13 +83,16 @@ a directory, the latter is used for backwards compatibility. The files use the ``` toml [toolchain] channel = "nightly-2020-07-10" +# or +path = "/path/to/local/toolchain" + components = [ "rustfmt", "rustc-dev" ] targets = [ "wasm32-unknown-unknown", "thumbv2-none-eabi" ] profile = "minimal" ``` The `[toolchain]` section is mandatory, and at least one property must be -specified. +specified. `channel` and `path` are mutually exclusive. For backwards compatibility, `rust-toolchain` files also support a legacy format that only contains a toolchain name without any TOML encoding, e.g. @@ -104,7 +107,9 @@ The toolchains named in these files have a more restricted form than `rustup` toolchains generally, and may only contain the names of the three release channels, 'stable', 'beta', 'nightly', Rust version numbers, like '1.0.0', and optionally an archive date, like 'nightly-2017-01-01'. They may not name -custom toolchains, nor host-specific toolchains. +custom toolchains, nor host-specific toolchains, except by giving the +`path` to said toolchain directly. A relative `path` is resolved +relative to the location of the `rust-toolchain.toml` file. ## Default toolchain From 6c429e9c63e0019c74ec184af8f8eb0ac85c006a Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 11:40:15 -0800 Subject: [PATCH 08/21] Clearer docs for path --- doc/src/overrides.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/doc/src/overrides.md b/doc/src/overrides.md index e0c1d77029..a45b203861 100644 --- a/doc/src/overrides.md +++ b/doc/src/overrides.md @@ -83,9 +83,6 @@ a directory, the latter is used for backwards compatibility. The files use the ``` toml [toolchain] channel = "nightly-2020-07-10" -# or -path = "/path/to/local/toolchain" - components = [ "rustfmt", "rustc-dev" ] targets = [ "wasm32-unknown-unknown", "thumbv2-none-eabi" ] profile = "minimal" @@ -107,9 +104,19 @@ The toolchains named in these files have a more restricted form than `rustup` toolchains generally, and may only contain the names of the three release channels, 'stable', 'beta', 'nightly', Rust version numbers, like '1.0.0', and optionally an archive date, like 'nightly-2017-01-01'. They may not name -custom toolchains, nor host-specific toolchains, except by giving the -`path` to said toolchain directly. A relative `path` is resolved -relative to the location of the `rust-toolchain.toml` file. +custom toolchains, nor host-specific toolchains. To use a custom local +toolchain, you can instead use a `path` toolchain: + +``` toml +[toolchain] +path = "/path/to/local/toolchain" +``` + +Since a `path` directive directly names a local toolchain, other options +like `components`, `targets`, and `profile` have no effect. `channel` +and `path` are mutually exclusive, since a `path` already points to a +specific toolchain. A relative `path` is resolved relative to the +location of the `rust-toolchain.toml` file. ## Default toolchain From 83f8b169950b602aaf2312cdfe121615ad90237e Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 12:15:35 -0800 Subject: [PATCH 09/21] Make path tests OS-agnostic --- tests/cli-rustup.rs | 109 ++++++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 39 deletions(-) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index cde17fa794..646e64150f 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1385,64 +1385,95 @@ fn file_override() { } #[test] -#[cfg_attr( - not(unix), - ignore = "TODO: Figure out how to write a wrapper toolchain on Windows" -)] fn file_override_path() { setup(&|config| { - let cwd = config.current_dir(); - let toolchain_path = cwd.join("ephemeral"); - let toolchain_bin = toolchain_path.join("bin"); - fs::create_dir_all(&toolchain_bin).unwrap(); - - // Inject a wrapper binary for rustc. - let rustc = toolchain_bin.join("rustc"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - raw::write_file(&rustc, "#!/bin/sh\necho custom-toolchain").unwrap(); - fs::set_permissions(rustc, fs::Permissions::from_mode(0o755)).unwrap(); - } + expect_ok(config, &["rustup", "default", "stable"]); + expect_ok( + config, + &[ + "rustup", + "toolchain", + "install", + "nightly", + "--no-self-update", + ], + ); - let toolchain_file = cwd.join("rust-toolchain.toml"); + let toolchain_path = config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_triple())); + let toolchain_file = config.current_dir().join("rust-toolchain.toml"); raw::write_file( &toolchain_file, &format!("[toolchain]\npath=\"{}\"", toolchain_path.display()), ) .unwrap(); - expect_stdout_ok(config, &["rustc", "--version"], "custom-toolchain"); + expect_stdout_ok(config, &["rustc", "--version"], "hash-nightly-2"); }); } #[test] -#[cfg_attr( - not(unix), - ignore = "TODO: Figure out how to write a wrapper toolchain on Windows" -)] fn file_override_path_relative() { setup(&|config| { - let cwd = config.current_dir(); - let toolchain_path = cwd.join("ephemeral"); - let toolchain_bin = toolchain_path.join("bin"); - fs::create_dir_all(&toolchain_bin).unwrap(); + expect_ok(config, &["rustup", "default", "stable"]); + expect_ok( + config, + &[ + "rustup", + "toolchain", + "install", + "nightly", + "--no-self-update", + ], + ); - // Inject a wrapper binary for rustc. - let rustc = toolchain_bin.join("rustc"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - raw::write_file(&rustc, "#!/bin/sh\necho custom-toolchain").unwrap(); - fs::set_permissions(rustc, fs::Permissions::from_mode(0o755)).unwrap(); + let toolchain_path = config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_triple())) + .canonicalize() + .unwrap(); + let toolchain_file = config + .current_dir() + .canonicalize() + .unwrap() + .join("rust-toolchain.toml"); + + // Find shared prefix so we can determine a relative path + let mut p1 = dbg!(&toolchain_path).components().peekable(); + let mut p2 = dbg!(&toolchain_file).components().peekable(); + while let (Some(p1p), Some(p2p)) = (p1.peek(), p2.peek()) { + if p1p == p2p { + let _ = p1.next(); + let _ = p2.next(); + } else { + // The two paths diverge here + break; + } } + let mut relative_path = PathBuf::new(); + // NOTE: We skip 1 since we don't need to .. across the .toml file at the end of the path + for _ in p2.skip(1) { + relative_path.push(".."); + } + for p in p1 { + relative_path.push(p); + } + assert!(relative_path.is_relative()); - let toolchain_file = cwd.join("rust-toolchain.toml"); - raw::write_file(&toolchain_file, "[toolchain]\npath=\"ephemeral\"").unwrap(); + raw::write_file( + &toolchain_file, + &format!("[toolchain]\npath=\"{}\"", relative_path.display()), + ) + .unwrap(); - // Change into ephemeral so that we actually test that the path is relative to the override - config.change_dir(&toolchain_path, || { - expect_stdout_ok(config, &["rustc", "--version"], "custom-toolchain") + // Change into an ephemeral dir so that we test that the path is relative to the override + let ephemeral = config.current_dir().join("ephemeral"); + fs::create_dir_all(&ephemeral).unwrap(); + config.change_dir(&ephemeral, || { + expect_stdout_ok(config, &["rustc", "--version"], "hash-nightly-2"); }); }); } From f7aeab6259e421f3f4895eae8e8b36fae6079550 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 12:23:43 -0800 Subject: [PATCH 10/21] Disallow path + toolchain options --- src/config.rs | 10 +++++++++- src/errors.rs | 4 ++++ tests/cli-rustup.rs | 48 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 02b913623e..ac14ab70b5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -95,7 +95,15 @@ impl<'a> OverrideCfg<'a> { Ok(Self { toolchain: match (file.toolchain.channel, file.toolchain.path) { (Some(name), None) => Some(Toolchain::from(cfg, &name)?), - (None, Some(path)) => Some(Toolchain::from_path(cfg, cfg_path, &path)?), + (None, Some(path)) => { + if file.toolchain.targets.is_some() + || file.toolchain.components.is_some() + || file.toolchain.profile.is_some() + { + return Err(ErrorKind::CannotSpecifyPathAndOptions(path.into()).into()); + } + Some(Toolchain::from_path(cfg, cfg_path, &path)?) + } (Some(channel), Some(path)) => { return Err(ErrorKind::CannotSpecifyChannelAndPath(channel, path.into()).into()) } diff --git a/src/errors.rs b/src/errors.rs index 303880f54d..be04da9491 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -192,6 +192,10 @@ error_chain! { description("invalid toolchain path"), display("invalid toolchain path: '{}'", p.display()) } + CannotSpecifyPathAndOptions(path: PathBuf) { + description("toolchain options are ignored for path toolchains"), + display("toolchain options are ignored for path toolchain ({})", path.display()) + } CannotSpecifyChannelAndPath(channel: String, path: PathBuf) { description("cannot specify channel and path simultaneously"), display("cannot specify both channel ({}) and path ({}) simultaneously", channel, path.display()) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 646e64150f..0dbe12132a 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1478,6 +1478,54 @@ fn file_override_path_relative() { }); } +#[test] +fn file_override_path_no_options() { + setup(&|config| { + // Make a plausible-looking toolchain + let cwd = config.current_dir(); + let toolchain_path = cwd.join("ephemeral"); + let toolchain_bin = toolchain_path.join("bin"); + fs::create_dir_all(&toolchain_bin).unwrap(); + + let toolchain_file = cwd.join("rust-toolchain.toml"); + raw::write_file( + &toolchain_file, + "[toolchain]\npath=\"ephemeral\"\ntargets=[\"dummy\"]", + ) + .unwrap(); + + expect_err( + config, + &["rustc", "--version"], + "toolchain options are ignored for path toolchain (ephemeral)", + ); + + raw::write_file( + &toolchain_file, + "[toolchain]\npath=\"ephemeral\"\ncomponents=[\"dummy\"]", + ) + .unwrap(); + + expect_err( + config, + &["rustc", "--version"], + "toolchain options are ignored for path toolchain (ephemeral)", + ); + + raw::write_file( + &toolchain_file, + "[toolchain]\npath=\"ephemeral\"\nprofile=\"minimal\"", + ) + .unwrap(); + + expect_err( + config, + &["rustc", "--version"], + "toolchain options are ignored for path toolchain (ephemeral)", + ); + }); +} + #[test] fn file_override_path_xor_channel() { setup(&|config| { From 59d9285c372485d04854b2d165082f53878a9522 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 12:30:09 -0800 Subject: [PATCH 11/21] Explicitly test path toolchain name --- tests/cli-rustup.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 0dbe12132a..a55c747949 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1411,6 +1411,13 @@ fn file_override_path() { .unwrap(); expect_stdout_ok(config, &["rustc", "--version"], "hash-nightly-2"); + + // Check that the toolchain has the right name + expect_stdout_ok( + config, + &["rustup", "show", "active-toolchain"], + &format!("nightly-{}", this_host_triple()), + ); }); } From 862ec5685ecfe34e3e8e2ad1fc7ae168e167641d Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 13:56:44 -0800 Subject: [PATCH 12/21] Allow and test path override through env/cmd This is needed so that proxy commands (like `cargo` -> `rustc`) will continue to use the path toolchain. --- src/config.rs | 20 ++++++--- src/toolchain.rs | 12 +++--- tests/cli-rustup.rs | 98 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 116 insertions(+), 14 deletions(-) diff --git a/src/config.rs b/src/config.rs index ac14ab70b5..1af1895979 100644 --- a/src/config.rs +++ b/src/config.rs @@ -50,11 +50,21 @@ impl ToolchainSection { impl> From for OverrideFile { fn from(channel: T) -> Self { - Self { - toolchain: ToolchainSection { - channel: Some(channel.into()), - ..Default::default() - }, + let override_ = channel.into(); + if Path::new(&override_).is_absolute() { + Self { + toolchain: ToolchainSection { + channel: Some(override_), + ..Default::default() + }, + } + } else { + Self { + toolchain: ToolchainSection { + path: Some(PathBuf::from(override_)), + ..Default::default() + }, + } } } } diff --git a/src/toolchain.rs b/src/toolchain.rs index 0b36b48df2..b0d1deb003 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -91,15 +91,13 @@ impl<'a> Toolchain<'a> { return Err(ErrorKind::InvalidToolchainPath(path.into()).into()); } - let base = path - .components() - .last() - .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.clone()))? - .as_os_str() - .to_string_lossy(); Ok(Toolchain { cfg, - name: base.into(), + name: path + .as_os_str() + .to_str() + .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.clone().into()))? + .to_owned(), path, dist_handler: Box::new(move |n| (cfg.notify_handler)(n.into())), }) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index a55c747949..0c8bd480dc 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1384,6 +1384,70 @@ fn file_override() { }); } +#[test] +fn env_override_path() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "stable"]); + expect_ok( + config, + &[ + "rustup", + "toolchain", + "install", + "nightly", + "--no-self-update", + ], + ); + + let toolchain_path = config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_triple())); + + let mut cmd = clitools::cmd(config, "rustc", &["--version"]); + clitools::env(config, &mut cmd); + cmd.env("RUSTUP_TOOLCHAIN", toolchain_path.to_str().unwrap()); + + let out = cmd.output().unwrap(); + assert!(String::from_utf8(out.stdout) + .unwrap() + .contains("hash-nightly-2")); + }); +} + +#[test] +fn plus_override_path() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "stable"]); + expect_ok( + config, + &[ + "rustup", + "toolchain", + "install", + "nightly", + "--no-self-update", + ], + ); + + let toolchain_path = config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_triple())); + expect_stdout_ok( + config, + &[ + "rustup", + "run", + toolchain_path.to_str().unwrap(), + "rustc", + "--version", + ], + "hash-nightly-2", + ); + }); +} + #[test] fn file_override_path() { setup(&|config| { @@ -1406,7 +1470,7 @@ fn file_override_path() { let toolchain_file = config.current_dir().join("rust-toolchain.toml"); raw::write_file( &toolchain_file, - &format!("[toolchain]\npath=\"{}\"", toolchain_path.display()), + &format!("[toolchain]\npath=\"{}\"", toolchain_path.to_str().unwrap()), ) .unwrap(); @@ -1421,6 +1485,36 @@ fn file_override_path() { }); } +#[test] +fn proxy_override_path() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "stable"]); + expect_ok( + config, + &[ + "rustup", + "toolchain", + "install", + "nightly", + "--no-self-update", + ], + ); + + let toolchain_path = config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_triple())); + let toolchain_file = config.current_dir().join("rust-toolchain.toml"); + raw::write_file( + &toolchain_file, + &format!("[toolchain]\npath=\"{}\"", toolchain_path.to_str().unwrap()), + ) + .unwrap(); + + expect_stdout_ok(config, &["cargo", "--call-rustc"], "hash-nightly-2"); + }); +} + #[test] fn file_override_path_relative() { setup(&|config| { @@ -1472,7 +1566,7 @@ fn file_override_path_relative() { raw::write_file( &toolchain_file, - &format!("[toolchain]\npath=\"{}\"", relative_path.display()), + &format!("[toolchain]\npath=\"{}\"", relative_path.to_str().unwrap()), ) .unwrap(); From 9957ce60aa2af9591bcda1d9ccf2f352def84383 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 14:58:03 -0800 Subject: [PATCH 13/21] Mixed up boolean condition --- src/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index 1af1895979..8d2a1f2545 100644 --- a/src/config.rs +++ b/src/config.rs @@ -54,14 +54,14 @@ impl> From for OverrideFile { if Path::new(&override_).is_absolute() { Self { toolchain: ToolchainSection { - channel: Some(override_), + path: Some(PathBuf::from(override_)), ..Default::default() }, } } else { Self { toolchain: ToolchainSection { - path: Some(PathBuf::from(override_)), + channel: Some(override_), ..Default::default() }, } From 7dcb260a9e79cba986fc05f0bf31db202a77fbd0 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 15:48:24 -0800 Subject: [PATCH 14/21] Debug Windows through CI --- src/errors.rs | 2 +- tests/cli-rustup.rs | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index be04da9491..2e43e86b42 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -190,7 +190,7 @@ error_chain! { } InvalidToolchainPath(p: PathBuf) { description("invalid toolchain path"), - display("invalid toolchain path: '{}'", p.display()) + display("invalid toolchain path: '{}'", p.to_string_lossy()) } CannotSpecifyPathAndOptions(path: PathBuf) { description("toolchain options are ignored for path toolchains"), diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 0c8bd480dc..a7a67a3092 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1470,7 +1470,10 @@ fn file_override_path() { let toolchain_file = config.current_dir().join("rust-toolchain.toml"); raw::write_file( &toolchain_file, - &format!("[toolchain]\npath=\"{}\"", toolchain_path.to_str().unwrap()), + &dbg!(format!( + "[toolchain]\npath=\"{}\"", + toolchain_path.to_str().unwrap() + )), ) .unwrap(); @@ -1545,7 +1548,7 @@ fn file_override_path_relative() { // Find shared prefix so we can determine a relative path let mut p1 = dbg!(&toolchain_path).components().peekable(); let mut p2 = dbg!(&toolchain_file).components().peekable(); - while let (Some(p1p), Some(p2p)) = (p1.peek(), p2.peek()) { + while let (Some(p1p), Some(p2p)) = (dbg!(p1.peek()), dbg!(p2.peek())) { if p1p == p2p { let _ = p1.next(); let _ = p2.next(); @@ -1556,13 +1559,14 @@ fn file_override_path_relative() { } let mut relative_path = PathBuf::new(); // NOTE: We skip 1 since we don't need to .. across the .toml file at the end of the path - for _ in p2.skip(1) { + for p in p2.skip(1) { + dbg!(p); relative_path.push(".."); } for p in p1 { - relative_path.push(p); + relative_path.push(dbg!(p)); } - assert!(relative_path.is_relative()); + assert!(dbg!(&relative_path).is_relative()); raw::write_file( &toolchain_file, From b872e2e1434f5c3e7ba3eaf8cad997f14d3fb389 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 16:12:58 -0800 Subject: [PATCH 15/21] Fix Windows backslash interpolation --- tests/cli-rustup.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index a7a67a3092..415e24364e 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1470,10 +1470,7 @@ fn file_override_path() { let toolchain_file = config.current_dir().join("rust-toolchain.toml"); raw::write_file( &toolchain_file, - &dbg!(format!( - "[toolchain]\npath=\"{}\"", - toolchain_path.to_str().unwrap() - )), + &format!("[toolchain]\npath='{}'", toolchain_path.to_str().unwrap()), ) .unwrap(); @@ -1510,7 +1507,7 @@ fn proxy_override_path() { let toolchain_file = config.current_dir().join("rust-toolchain.toml"); raw::write_file( &toolchain_file, - &format!("[toolchain]\npath=\"{}\"", toolchain_path.to_str().unwrap()), + &format!("[toolchain]\npath='{}'", toolchain_path.to_str().unwrap()), ) .unwrap(); @@ -1546,9 +1543,9 @@ fn file_override_path_relative() { .join("rust-toolchain.toml"); // Find shared prefix so we can determine a relative path - let mut p1 = dbg!(&toolchain_path).components().peekable(); - let mut p2 = dbg!(&toolchain_file).components().peekable(); - while let (Some(p1p), Some(p2p)) = (dbg!(p1.peek()), dbg!(p2.peek())) { + let mut p1 = toolchain_path.components().peekable(); + let mut p2 = toolchain_file.components().peekable(); + while let (Some(p1p), Some(p2p)) = (p1.peek(), p2.peek()) { if p1p == p2p { let _ = p1.next(); let _ = p2.next(); @@ -1559,18 +1556,17 @@ fn file_override_path_relative() { } let mut relative_path = PathBuf::new(); // NOTE: We skip 1 since we don't need to .. across the .toml file at the end of the path - for p in p2.skip(1) { - dbg!(p); + for _ in p2.skip(1) { relative_path.push(".."); } for p in p1 { - relative_path.push(dbg!(p)); + relative_path.push(p); } - assert!(dbg!(&relative_path).is_relative()); + assert!(relative_path.is_relative()); raw::write_file( &toolchain_file, - &format!("[toolchain]\npath=\"{}\"", relative_path.to_str().unwrap()), + &format!("[toolchain]\npath='{}'", relative_path.to_str().unwrap()), ) .unwrap(); From c76604296eb8b180658cd41418a2869bb5b5d95c Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 16:44:53 -0800 Subject: [PATCH 16/21] More windows debugging --- src/toolchain.rs | 6 ++---- tests/cli-rustup.rs | 7 +++++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/toolchain.rs b/src/toolchain.rs index b0d1deb003..1bfc6fa6fd 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -87,15 +87,13 @@ impl<'a> Toolchain<'a> { // Perform minimal validation; there should at least be a `bin/` that might // contain things for us to run. - if !path.join("bin").is_dir() { + if !dbg!(path.join("bin")).is_dir() { return Err(ErrorKind::InvalidToolchainPath(path.into()).into()); } Ok(Toolchain { cfg, - name: path - .as_os_str() - .to_str() + name: dbg!(path.to_str()) .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.clone().into()))? .to_owned(), path, diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 415e24364e..c44a62f3a7 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1562,11 +1562,14 @@ fn file_override_path_relative() { for p in p1 { relative_path.push(p); } - assert!(relative_path.is_relative()); + assert!(dbg!(&relative_path).is_relative()); raw::write_file( &toolchain_file, - &format!("[toolchain]\npath='{}'", relative_path.to_str().unwrap()), + dbg!(&format!( + "[toolchain]\npath='{}'", + relative_path.to_str().unwrap() + )), ) .unwrap(); From a16531a1353363302079e2224b33173af68c41eb Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 23 Feb 2021 17:07:29 -0800 Subject: [PATCH 17/21] Maybe Windows chokes on canonicalization? --- tests/cli-rustup.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index c44a62f3a7..252f0ce542 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1533,14 +1533,8 @@ fn file_override_path_relative() { let toolchain_path = config .rustupdir .join("toolchains") - .join(format!("nightly-{}", this_host_triple())) - .canonicalize() - .unwrap(); - let toolchain_file = config - .current_dir() - .canonicalize() - .unwrap() - .join("rust-toolchain.toml"); + .join(format!("nightly-{}", this_host_triple())); + let toolchain_file = config.current_dir().join("rust-toolchain.toml"); // Find shared prefix so we can determine a relative path let mut p1 = toolchain_path.components().peekable(); From 0f6640b243d569b5c8dfa2d4a0a1c8ea0440e615 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 24 Feb 2021 10:00:05 -0800 Subject: [PATCH 18/21] Ignore relative path test on Windows due to UNC --- src/toolchain.rs | 5 +++-- tests/cli-rustup.rs | 8 +++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/toolchain.rs b/src/toolchain.rs index 1bfc6fa6fd..7f1412e9ce 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -87,13 +87,14 @@ impl<'a> Toolchain<'a> { // Perform minimal validation; there should at least be a `bin/` that might // contain things for us to run. - if !dbg!(path.join("bin")).is_dir() { + if !path.join("bin").is_dir() { return Err(ErrorKind::InvalidToolchainPath(path.into()).into()); } Ok(Toolchain { cfg, - name: dbg!(path.to_str()) + name: path + .to_str() .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.clone().into()))? .to_owned(), path, diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 252f0ce542..eea6e57fd1 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1516,6 +1516,7 @@ fn proxy_override_path() { } #[test] +#[ignore = "FIXME: Windows uses UNC paths which do not work with relative paths"] fn file_override_path_relative() { setup(&|config| { expect_ok(config, &["rustup", "default", "stable"]); @@ -1556,14 +1557,11 @@ fn file_override_path_relative() { for p in p1 { relative_path.push(p); } - assert!(dbg!(&relative_path).is_relative()); + assert!(relative_path.is_relative()); raw::write_file( &toolchain_file, - dbg!(&format!( - "[toolchain]\npath='{}'", - relative_path.to_str().unwrap() - )), + &format!("[toolchain]\npath='{}'", relative_path.to_str().unwrap()), ) .unwrap(); From bd3713559a01d4a6c289479baa0536971ef5ee91 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 24 Feb 2021 10:01:56 -0800 Subject: [PATCH 19/21] Only ignore on windows --- tests/cli-rustup.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index eea6e57fd1..307a7655b7 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1516,7 +1516,10 @@ fn proxy_override_path() { } #[test] -#[ignore = "FIXME: Windows uses UNC paths which do not work with relative paths"] +#[cfg_attr( + windows, + ignore = "FIXME: Windows uses UNC paths which do not work with relative paths" +)] fn file_override_path_relative() { setup(&|config| { expect_ok(config, &["rustup", "default", "stable"]); From d704b654697e8c312a0eae11e23087141a820d5e Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 11 Mar 2021 14:01:59 -0800 Subject: [PATCH 20/21] Canonicalize paths only when needed --- src/config.rs | 7 +++---- src/toolchain.rs | 2 +- tests/cli-rustup.rs | 4 ---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/config.rs b/src/config.rs index 8d2a1f2545..a873482249 100644 --- a/src/config.rs +++ b/src/config.rs @@ -552,11 +552,11 @@ impl Cfg { } OverrideReason::OverrideDB(ref path) => format!( "the directory override for '{}' specifies an uninstalled toolchain", - path.display() + utils::canonicalize_path(path, self.notify_handler.as_ref()).display(), ), OverrideReason::ToolchainFile(ref path) => format!( "the toolchain file at '{}' specifies an uninstalled toolchain", - path.display() + utils::canonicalize_path(path, self.notify_handler.as_ref()).display(), ), }; @@ -593,8 +593,7 @@ impl Cfg { settings: &Settings, ) -> Result> { let notify = self.notify_handler.as_ref(); - let dir = utils::canonicalize_path(dir, notify); - let mut dir = Some(&*dir); + let mut dir = Some(dir); while let Some(d) = dir { // First check the override database diff --git a/src/toolchain.rs b/src/toolchain.rs index 7f1412e9ce..b2931b565c 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -93,7 +93,7 @@ impl<'a> Toolchain<'a> { Ok(Toolchain { cfg, - name: path + name: utils::canonicalize_path(&path, cfg.notify_handler.as_ref()) .to_str() .ok_or_else(|| ErrorKind::InvalidToolchainPath(path.clone().into()))? .to_owned(), diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 307a7655b7..9b4db3cc89 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1516,10 +1516,6 @@ fn proxy_override_path() { } #[test] -#[cfg_attr( - windows, - ignore = "FIXME: Windows uses UNC paths which do not work with relative paths" -)] fn file_override_path_relative() { setup(&|config| { expect_ok(config, &["rustup", "default", "stable"]); From e72d2fa3ac39408e3a0036af03f00ecc375453e6 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 11 Mar 2021 14:42:27 -0800 Subject: [PATCH 21/21] Canonicalize another path for display --- src/notifications.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/notifications.rs b/src/notifications.rs index beaa8a6a92..1250a81498 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -143,8 +143,14 @@ impl<'a> Display for Notification<'a> { } => write!( f, "both `{0}` and `{1}` exist. Using `{0}`", - rust_toolchain.display(), - rust_toolchain_toml.display() + rust_toolchain + .canonicalize() + .unwrap_or_else(|_| PathBuf::from(rust_toolchain)) + .display(), + rust_toolchain_toml + .canonicalize() + .unwrap_or_else(|_| PathBuf::from(rust_toolchain_toml)) + .display(), ), } }