Skip to content

fix(cli/self-update): postpone initialization of Cfg in setup_mode #4996

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.

Sign up for GitHub

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

Open
rami3l wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(cli/self-update): postpone initialization of Cfg in setup_mode #4996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d8d289
test(cli-inst-interactive): test file creation on cancelled installation
rami3l Aug 5, 2026
299e9a1
fix(settings): prevent creating new file with `SettingsFile::read_set…
rami3l Aug 4, 2026
5e8388c
refactor(cli/self-update): take `Process` in `check_existence_of_sett…
rami3l Aug 4, 2026
57f2dfc
fix(cli/self-update): postpone initialization of `Cfg` in `setup_mode`
rami3l Aug 4, 2026
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
99 changes: 55 additions & 44 deletions src/cli/self_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use crate::{
errors::CliError,
markdown::md,
},
config::Cfg,
config::{Cfg, default_host_tuple},
dist::{
DistOptions, PartialToolchainDesc, Profile, TargetTuple, ToolchainDesc,
download::DownloadCfg,
Expand All @@ -67,6 +67,7 @@ use crate::{
errors::RustupError,
install::{InstallMethod, UpdateStatus},
process::Process,
settings::SettingsFile,
toolchain::{
DistributableToolchain, MaybeOfficialToolchainName, ResolvableToolchainName, Toolchain,
ToolchainName,
Expand Down Expand Up @@ -106,6 +107,7 @@ pub(crate) struct InstallOpts<'a> {
pub no_update_toolchain: bool,
pub components: &'a [&'a str],
pub targets: &'a [&'a str],
pub process: &'a Process,
}

impl InstallOpts<'_> {
Expand All @@ -115,76 +117,82 @@ impl InstallOpts<'_> {
/// - Copying the running binary to the binary directory.
/// - Hard-linking the various Rust tools to that copied binary.
/// - Adding the binary directory to the `$PATH` unless `no_modify_path` is set.
pub(crate) async fn install(mut self, no_prompt: bool, cfg: &mut Cfg<'_>) -> Result<ExitCode> {
pub(crate) async fn install(
mut self,
current_dir: PathBuf,
no_prompt: bool,
quiet: bool,
) -> Result<ExitCode> {
#[cfg_attr(not(unix), allow(unused_mut))]
let mut exit_code = ExitCode::SUCCESS;

self.validate(cfg.process).map_err(|e| {
let process = self.process;

self.validate(process).map_err(|e| {
anyhow!(
"Pre-checks for host and toolchain failed: {e}\n\
If you are unsure of suitable values, the 'stable' toolchain is the default.\n\
Valid host tuples look something like: {}",
TargetTuple::from_host_or_build(cfg.process)
TargetTuple::from_host_or_build(process)
)
})?;

if cfg
.process
if process
.var_os("RUSTUP_INIT_SKIP_EXISTENCE_CHECKS")
.is_none_or(|s| s != "yes")
{
check_existence_of_rustc_or_cargo_in_path(no_prompt, cfg.process)?;
check_existence_of_settings_file(cfg)?;
check_existence_of_rustc_or_cargo_in_path(no_prompt, process)?;
check_existence_of_settings_file(process)?;
}

#[cfg(unix)]
{
exit_code &= unix::do_anti_sudo_check(no_prompt, cfg.process)?;
exit_code &= unix::do_anti_sudo_check(no_prompt, process)?;
}

let mut term = cfg.process.stdout();
let mut term = process.stdout();

#[cfg(windows)]
windows::maybe_install_msvc(&mut term, no_prompt, &self, &*cfg).await?;
windows::maybe_install_msvc(&mut term, no_prompt, quiet, &self, process).await?;

if !no_prompt {
let msg = pre_install_msg(self.no_modify_path, cfg.process)?;
let msg = pre_install_msg(self.no_modify_path, process)?;

md(&mut term, msg);
let mut customized_install = false;
loop {
md(&mut term, current_install_opts(&self, cfg.process));
match common::confirm_advanced(customized_install, cfg.process)? {
md(&mut term, current_install_opts(&self));
match common::confirm_advanced(customized_install, process)? {
Confirm::No => {
info!("aborting installation");
return Ok(ExitCode::SUCCESS);
}
Confirm::Yes => break,
Confirm::Advanced => {
customized_install = true;
self.customize(cfg.process)?;
self.customize(process)?;
}
}
}
}

let no_modify_path = self.no_modify_path;
if let Err(e) = self.install_rust(cfg).await {
report_error(&e, cfg.process);
if let Err(e) = self.install_rust(current_dir, quiet).await {
report_error(&e, process);

// On windows, where installation happens in a console
// that may have opened just for this purpose, give
// the user an opportunity to see the error before the
// window closes.
#[cfg(windows)]
if !no_prompt {
windows::ensure_prompt(cfg.process)?;
windows::ensure_prompt(process)?;
}

return Ok(ExitCode::FAILURE);
}

let cargo_home = canonical_cargo_home(cfg.process)?;
let cargo_home = canonical_cargo_home(process)?;
#[cfg(windows)]
let cargo_home = cargo_home.replace('\\', r"\\");
#[cfg(windows)]
Expand All @@ -197,7 +205,7 @@ impl InstallOpts<'_> {
format!(post_install_msg_win!(), cargo_home = cargo_home)
};
#[cfg(not(windows))]
let source_env_lines = shell::build_source_env_lines(cfg.process);
let source_env_lines = shell::build_source_env_lines(process);
#[cfg(not(windows))]
let msg = if no_modify_path {
format!(
Expand All @@ -215,33 +223,33 @@ impl InstallOpts<'_> {
md(&mut term, msg);

#[cfg(unix)]
warn_if_default_linker_missing(cfg.process);
warn_if_default_linker_missing(process);

#[cfg(windows)]
if !no_prompt {
// On windows, where installation happens in a console
// that may have opened just for this purpose, require
// the user to press a key to continue.
windows::ensure_prompt(cfg.process)?;
windows::ensure_prompt(process)?;
}

Ok(exit_code)
}

/// Installs the rustup binary and proxies, and installs a toolchain if specified.
async fn install_rust(self, cfg: &mut Cfg<'_>) -> Result<()> {
install_bins(cfg.process)?;
async fn install_rust(self, current_dir: PathBuf, quiet: bool) -> Result<()> {
install_bins(self.process)?;

#[cfg(unix)]
unix::do_write_env_files(cfg.process)?;
unix::do_write_env_files(self.process)?;

if !self.no_modify_path {
do_add_to_path(cfg.process)?;
do_add_to_path(self.process)?;
}

// If RUSTUP_HOME is not set, make sure it exists
if cfg.process.var_os("RUSTUP_HOME").is_none() {
let home = cfg
if self.process.var_os("RUSTUP_HOME").is_none() {
let home = self
.process
.home_dir()
.map(|p| p.join(".rustup"))
Expand All @@ -250,19 +258,21 @@ impl InstallOpts<'_> {
fs::create_dir_all(home).context("unable to create ~/.rustup")?;
}

let mut cfg = Cfg::from_env(current_dir, quiet, false, self.process)?;

let (components, targets) = (self.components, self.targets);
let toolchain = self.select_toolchain(cfg)?;
let toolchain = self.select_toolchain(&mut cfg)?;
if let Some(desc) = toolchain {
let options =
DistOptions::new(components, targets, &desc, cfg.get_profile()?, true, cfg)?;
let status = if Toolchain::exists(cfg, &desc.clone().into())? {
DistOptions::new(components, targets, &desc, cfg.get_profile()?, true, &cfg)?;
let status = if Toolchain::exists(&cfg, &desc.clone().into())? {
warn!("Updating existing toolchain, profile choice will be ignored");
// If we have a partial install we might not be able to read content here. We could:
// - fail and folk have to delete the partially present toolchain to recover
// - silently ignore it (and provide inconsistent metadata for reporting the install/update change)
// - delete the partial install and start over
// For now, we error.
let toolchain = DistributableToolchain::new(cfg, desc.clone())?;
let toolchain = DistributableToolchain::new(&cfg, desc.clone())?;
InstallMethod::Dist(options.for_update(&toolchain, false))
.install(None)
.await?
Expand All @@ -274,7 +284,7 @@ impl InstallOpts<'_> {

cfg.set_default(Some(&desc.clone().into()))?;
writeln!(cfg.process.stdout().lock())?;
common::show_channel_update(cfg, PackageUpdate::Toolchain(desc), Ok(status))?;
common::show_channel_update(&cfg, PackageUpdate::Toolchain(desc), Ok(status))?;
}
Ok(())
}
Expand All @@ -293,6 +303,7 @@ impl InstallOpts<'_> {
no_update_toolchain,
components,
targets,
..
} = self;

cfg.set_profile(profile)?;
Expand Down Expand Up @@ -625,23 +636,22 @@ fn check_existence_of_rustc_or_cargo_in_path(no_prompt: bool, process: &Process)
Ok(())
}

fn check_existence_of_settings_file(cfg: &Cfg<'_>) -> Result<()> {
let rustup_dir = cfg.process.rustup_home()?;
let settings_file_path = rustup_dir.join("settings.toml");
if !utils::path_exists(&settings_file_path) {
fn check_existence_of_settings_file(process: &Process) -> Result<()> {
let rustup_dir = process.rustup_home()?;
let settings_file = SettingsFile::new(rustup_dir.join("settings.toml"));
if !utils::path_exists(&settings_file.path) {
return Ok(());
}
let settings_toolchain = cfg
.settings_file
.with(|s| Ok(s.default_toolchain.clone()))?;
let settings_toolchain = settings_file.with(|s| Ok(s.default_toolchain.clone()))?;
// If there is already a non-empty `settings.toml` file (e.g., not a fresh install),
// then we warn the user that there was an already configured default toolchain.
let Some(default_toolchain) = settings_toolchain else {
return Ok(());
};
warn!("it looks like you have an existing rustup settings file at:");
warn!("{}", settings_file_path.display());
let inferred = PartialToolchainDesc::from_str("stable")?.resolve(&cfg.default_host_tuple()?)?;
warn!("{}", settings_file.path.display());
let default_host_tuple = settings_file.with(|s| Ok(default_host_tuple(s, process)))?;

@rami3l rami3l Aug 4, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is essentially the same as Cfg::default_host_tuple(), should we extract it somehow?

View changes since the review

let inferred = PartialToolchainDesc::from_str("stable")?.resolve(&default_host_tuple)?;
if default_toolchain != inferred.to_string() {
warn!("rustup will install the default toolchain as specified in the settings file,");
warn!("instead of the one inferred from the default host tuple.");
Expand Down Expand Up @@ -690,7 +700,7 @@ fn pre_install_msg(no_modify_path: bool, process: &Process) -> Result<String> {
}
}

fn current_install_opts(opts: &InstallOpts<'_>, process: &Process) -> String {
fn current_install_opts(opts: &InstallOpts<'_>) -> String {
format!(
r"Current installation options:

Expand All @@ -702,7 +712,7 @@ fn current_install_opts(opts: &InstallOpts<'_>, process: &Process) -> String {
opts.default_host_tuple
.as_ref()
.map(TargetTuple::new)
.unwrap_or_else(|| TargetTuple::from_host_or_build(process)),
.unwrap_or_else(|| TargetTuple::from_host_or_build(opts.process)),
match &opts.default_toolchain {
Some(name) => name.to_string(),
None => "stable (default)".to_owned(),
Expand Down Expand Up @@ -1375,6 +1385,7 @@ mod tests {
components: &[],
targets: &[],
no_update_toolchain: false,
process: &tp.process,
};

assert_eq!(
Expand Down
33 changes: 17 additions & 16 deletions src/cli/self_update/windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use super::super::errors::CliError;
use super::common;
Comment thread
rami3l marked this conversation as resolved.
use super::{InstallOpts, install_bins, report_error};
use crate::cli::markdown::md;
use crate::config::Cfg;
use crate::dist::TargetTuple;
use crate::download::DownloadOptions;
use crate::process::{ColorableTerminal, Process};
Expand Down Expand Up @@ -91,35 +90,36 @@ pub(crate) fn choose_vs_install(process: &Process) -> Result<Option<VsInstallPla
pub(super) async fn maybe_install_msvc(
term: &mut ColorableTerminal,
no_prompt: bool,
quiet: bool,
opts: &InstallOpts<'_>,
cfg: &Cfg<'_>,
process: &Process,
) -> Result<()> {
let Some(plan) = do_msvc_check(opts, cfg.process) else {
let Some(plan) = do_msvc_check(opts, process) else {
return Ok(());
};

if no_prompt {
warn!("installing msvc toolchain without its prerequisites");
} else if !cfg.quiet && plan == VsInstallPlan::Automatic {
} else if !quiet && plan == VsInstallPlan::Automatic {
md(term, MSVC_AUTO_INSTALL_MESSAGE);
match choose_vs_install(cfg.process)? {
match choose_vs_install(process)? {
Some(VsInstallPlan::Automatic) => {
match try_install_msvc(opts, cfg).await {
match try_install_msvc(opts, process).await {
Err(e) => {
// Make sure the console doesn't exit before the user can
// see the error and give the option to continue anyway.
report_error(&e, cfg.process);
if !common::question_bool("\nContinue?", false, cfg.process)? {
report_error(&e, process);
if !common::question_bool("\nContinue?", false, process)? {
info!("aborting installation");
}
}
Ok(ContinueInstall::No) => ensure_prompt(cfg.process)?,
Ok(ContinueInstall::No) => ensure_prompt(process)?,
_ => {}
}
}
Some(VsInstallPlan::Manual) => {
md(term, MSVC_MANUAL_INSTALL_MESSAGE);
if !common::question_bool("\nContinue?", false, cfg.process)? {
if !common::question_bool("\nContinue?", false, process)? {
info!("aborting installation");
}
}
Expand All @@ -128,7 +128,7 @@ pub(super) async fn maybe_install_msvc(
} else {
md(term, MSVC_MESSAGE);
md(term, MSVC_MANUAL_INSTALL_MESSAGE);
if !common::question_bool("\nContinue?", false, cfg.process)? {
if !common::question_bool("\nContinue?", false, process)? {
info!("aborting installation");
}
}
Expand Down Expand Up @@ -259,7 +259,7 @@ pub(crate) enum ContinueInstall {
/// but the rustup install should not be continued at this time.
pub(crate) async fn try_install_msvc(
opts: &InstallOpts<'_>,
cfg: &Cfg<'_>,
process: &Process,
) -> Result<ContinueInstall> {
// download the installer
let visual_studio_url = utils::parse_url("https://aka.ms/vs/17/release/vs_community.exe")?;
Expand All @@ -270,8 +270,9 @@ pub(crate) async fn try_install_msvc(
.context("error creating temp directory")?;

let visual_studio = tempdir.path().join("vs_setup.exe");

info!("downloading Visual Studio installer");
DownloadOptions::try_from(cfg.process)?
DownloadOptions::try_from(process)?
.start(&visual_studio_url, &visual_studio)
.download()
.await?;
Expand All @@ -289,7 +290,7 @@ pub(crate) async fn try_install_msvc(

// It's possible an earlier or later version of the Windows SDK has been
// installed separately from Visual Studio so installing it can be skipped.
if !has_windows_sdk_libs(cfg.process) {
if !has_windows_sdk_libs(process) {
cmd.args([
"--add",
"Microsoft.VisualStudio.Component.Windows11SDK.26100",
Expand Down Expand Up @@ -322,8 +323,8 @@ pub(crate) async fn try_install_msvc(
// It's possible that the installer returned a non-zero exit code
// even though the required components were successfully installed.
// In that case we warn about the error but continue on.
let have_msvc = do_msvc_check(opts, cfg.process).is_none();
let has_libs = has_windows_sdk_libs(cfg.process);
let have_msvc = do_msvc_check(opts, process).is_none();
let has_libs = has_windows_sdk_libs(process);
if have_msvc && has_libs {
warn!("Visual Studio is installed but a problem occurred during installation");
warn!("{}", err);
Expand Down
6 changes: 2 additions & 4 deletions src/cli/setup_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::{
common::{self, update_console_filter},
self_update::{self, InstallOpts},
},
config::Cfg,
dist::Profile,
process::Process,
toolchain::MaybeOfficialToolchainName,
Expand Down Expand Up @@ -128,8 +127,7 @@ pub async fn main(
no_update_toolchain: no_update_default_toolchain,
components: &component.iter().map(|s| &**s).collect::<Vec<_>>(),
targets: &target.iter().map(|s| &**s).collect::<Vec<_>>(),
process,
};

let mut cfg = Cfg::from_env(current_dir, quiet, false, process)?;
opts.install(no_prompt, &mut cfg).await
opts.install(current_dir, no_prompt, quiet).await
}
Loading
Loading