From 37e8dbfea6de743087864c878b7b607ad56059c1 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 20 Apr 2026 18:19:11 -0300 Subject: [PATCH 01/16] (WIP): uninstall binary --- src/commands/uninstall.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs index f59f5bf8..a7e68b9b 100644 --- a/src/commands/uninstall.rs +++ b/src/commands/uninstall.rs @@ -31,11 +31,11 @@ pub enum UninstallError { InternalCargoError(String), } -pub fn uninstall( - config: &Config, - channel: &Channel, - local_manifest: &mut Manifest, -) -> anyhow::Result<()> { + +pub fn uninstall_executable( + name: impl AsRef + Display, + root_dir: impl AsRef, +) -> Result<(), UninstallError> { let installed_toolchains_dir = config.midenup_home.join("toolchains"); let toolchain_dir = installed_toolchains_dir.join(format!("{}", &channel.name)); From f16957f0b58a8d49442f9958d8855eb4c208c13f Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 21 Apr 2026 17:46:51 -0300 Subject: [PATCH 02/16] Revert "(WIP): uninstall binary" This reverts commit 37e8dbfea6de743087864c878b7b607ad56059c1. --- src/commands/uninstall.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs index a7e68b9b..f59f5bf8 100644 --- a/src/commands/uninstall.rs +++ b/src/commands/uninstall.rs @@ -31,11 +31,11 @@ pub enum UninstallError { InternalCargoError(String), } - -pub fn uninstall_executable( - name: impl AsRef + Display, - root_dir: impl AsRef, -) -> Result<(), UninstallError> { +pub fn uninstall( + config: &Config, + channel: &Channel, + local_manifest: &mut Manifest, +) -> anyhow::Result<()> { let installed_toolchains_dir = config.midenup_home.join("toolchains"); let toolchain_dir = installed_toolchains_dir.join(format!("{}", &channel.name)); From 8d9e8360a0b2a3dd413a1c3ec687362c679dea4a Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 23 Apr 2026 17:18:29 -0300 Subject: [PATCH 03/16] refactor: only store artifacts in the LocalManifest if they were actually used --- src/channel.rs | 2 +- src/commands/install.rs | 75 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/channel.rs b/src/channel.rs index 29cd8038..2b64019f 100644 --- a/src/channel.rs +++ b/src/channel.rs @@ -590,7 +590,7 @@ pub struct Component { pub initialization: Vec, /// Pre-built artifact. #[serde(flatten)] - artifacts: Option, + pub artifacts: Option, } impl Component { diff --git a/src/commands/install.rs b/src/commands/install.rs index 531cdb19..0125feb6 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -1,4 +1,4 @@ -use std::{io::Write, path::Path, time::SystemTime}; +use std::{collections::HashSet, io::Write, path::{Path, PathBuf}, time::SystemTime}; use anyhow::{Context, bail}; @@ -138,6 +138,12 @@ pub fn install( channel.clone() }; + // We determine how the component got installed. + // A component could have been installed either by cargo install (i.e. "from + // source") or via a pre-compiled miden-provided binary artifact. + // We can only *truly* determine how it got installed after the fact. + let cargo_installed_binaries = get_installed_cargo_binaries(toolchain_dir)?; + for component in channel_to_save.components.iter_mut() { match &component.version { // If a component was installed with --branch, then write down the current commit. @@ -176,6 +182,33 @@ pub fn install( last_modification: Some(latest_time), } }, + Authority::Cargo { package, ..} => { + // If a component is marked with Cargo as an authority and + // also has artifacts listed as available, determine which + // got used for the installation. + // + // Currently, by convention, if a component has an artifacts + // field listed on the *LOCAL* manifest, then that means + // that artifacts were used. + if !component.get_artifact_uri(&config.target).is_some() { + continue; + } + + let package = package.as_deref().unwrap_or(component.name.as_ref()).to_string(); + + let installed_via_cargo = cargo_installed_binaries.contains(package.as_str()); + + // TODO (fabrio): Unify this in the local manifest, I don't + // believe there really is a need to store both fields. + if installed_via_cargo { + // This means that the component had an artifacts entry, + // yet it was not utilized. While rare, this can happen + // due to a number of factors, such as: no artifact for + // this system's triple or Github being offline (with + // the latter becoming more likely). + component.artifacts = None; + } + }, _ => (), } } @@ -664,3 +697,43 @@ fn main() { .to_string() .unwrap_or_else(|err| panic!("install script rendering failed: {err}")) } + +// TODO: Rename +type InstalledBinary = String; +/// Returns the names of all packages installed via cargo at the given root. +/// +/// Runs `cargo install --list --root ` and parses each package header line. +pub fn get_installed_cargo_binaries(root_dir: PathBuf) -> anyhow::Result> { + let output = std::process::Command::new("cargo") + .arg("install") + .arg("--root") + .arg(&root_dir) + .arg("--list") + .output() + .with_context(|| "Failed to obtain binaries intalled via cargo")?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + bail!("Failed to obtain binaries installed via cargo {stderr}"); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let programs = stdout + .lines() + // The format of cargo install --list is as follows: + // + // + // + // e.g.: + // ripgrep v15.1.0: + // rg + // sccache v0.10.0: + // sccache + .filter(|line| !line.is_empty() && !line.starts_with(char::is_whitespace)) + // The first item is the name of the crate that we have installed. + .filter_map(|line| line.split_whitespace().next()) + .map(String::from) + .collect(); + + Ok(programs) +} From d03f516c03676be78ae4cc12581f061408d11902 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 24 Apr 2026 18:31:44 -0300 Subject: [PATCH 04/16] WIP: tmp commment out deny(warnings) --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 52e6fbdf..3d513ba1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![deny(warnings)] +// #![deny(warnings)] mod artifact; pub mod channel; From 8ba2a24a93ee2f789b9db1e7a276fec2db9d2de0 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 24 Apr 2026 17:25:14 -0300 Subject: [PATCH 05/16] WIP: simplify uninstall --- src/commands/uninstall.rs | 270 ++++++++++++++++++++++++-------------- src/commands/update.rs | 4 +- 2 files changed, 174 insertions(+), 100 deletions(-) diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs index f59f5bf8..73ebea03 100644 --- a/src/commands/uninstall.rs +++ b/src/commands/uninstall.rs @@ -9,14 +9,13 @@ use anyhow::{Context, bail}; use thiserror::Error; use crate::{ - channel::{Channel, Component, InstalledFile}, - config::Config, - manifest::Manifest, - version::Authority, + channel::{Channel, Component, InstalledFile}, config::Config, manifest::Manifest, version::Authority }; #[derive(Error, Debug)] pub enum UninstallError { + #[error("Channel {0} is not installed, nothing to uninstall.")] + ChannelNotInstalled(semver::Version), #[error("Could not find installation-successful or .installation-in-progress at {0}")] MissingInstalledComponentsFile(PathBuf), #[error("Could not find channel.json file at: {0}. {1}")] @@ -25,6 +24,14 @@ pub enum UninstallError { IllFormedChannelJson(PathBuf, String, String), #[error("Couldn't delete file at: {0}. {1}")] FailedToDeleteFile(PathBuf, String), + #[error("Couldn't delete directory at: {0}. {1}")] + FailedToDeleteDirectory(PathBuf, std::io::Error), + #[error(transparent)] + Cargo(#[from] CargoError), +} + +#[derive(Error, Debug)] +pub enum CargoError { #[error("Failed to uninstall package: {0}, with status: {1}. {2}")] FailedToUninstallPackage(String, i32, String), #[error("Internal cargo error: {0}")] @@ -33,20 +40,17 @@ pub enum UninstallError { pub fn uninstall( config: &Config, - channel: &Channel, + upstream_channel: &Channel, local_manifest: &mut Manifest, ) -> anyhow::Result<()> { - let installed_toolchains_dir = config.midenup_home.join("toolchains"); - - let toolchain_dir = installed_toolchains_dir.join(format!("{}", &channel.name)); - if !toolchain_dir.exists() { - bail!("Channel {} is not installed, nothing to uninstall.", channel.name); - }; + // TODO (fabrio): should we remove the channel from the manifest ASAP? + let local_channel = local_manifest.get_channel_by_name(&upstream_channel.name) + .context(format!("Channel {} is not installed, nothing to uninstall.", upstream_channel.name))?; // NOTE: If either of the installed components files are missing, we continue with the uninstall // process regardless. All the installed components and additional files are going to get // deleted by remove_dir_all. - match uninstall_channel(&toolchain_dir) { + match uninstall_channel(config, &local_channel) { Ok(()) => (), Err(UninstallError::MissingInstalledComponentsFile(path)) => { println!( @@ -59,9 +63,11 @@ Uninstallation will procede by deleting toolchain manually, instead of going thr Err(err) => bail!("Failed to uninstall {err}"), } + let installed_toolchains_dir = config.midenup_home.join("toolchains"); // Now that the installation indicator is deleted, we can remove the symlink. If anything goes // wrong during this process, re-issuing the installation should brink the symlink back. { + let toolchain_dir = installed_toolchains_dir.join(format!("{}", &local_channel.name)); let stable_symlink = installed_toolchains_dir.join("stable"); // Only remove the stable symlink if it actually points to the toolchain being uninstalled. @@ -76,7 +82,7 @@ Uninstallation will procede by deleting toolchain manually, instead of going thr } } - local_manifest.remove_channel(channel.name.clone()); + local_manifest.remove_channel(local_channel.name.clone()); let local_manifest_path = config.midenup_home.join("manifest").with_extension("json"); let mut local_manifest_file = @@ -94,97 +100,36 @@ Uninstallation will procede by deleting toolchain manually, instead of going thr ) .context("Couldn't create local manifest file")?; - // We now remove the toolchain directory with all the remaining files. - std::fs::remove_dir_all(&toolchain_dir).context(format!( - "midenup failed to delete the toolchain directory. - However, manual removal should be safe. The toolchain's PATH is the following: -{} -", - toolchain_dir.display() - ))?; Ok(()) } -fn uninstall_channel(toolchain_dir: &PathBuf) -> Result<(), UninstallError> { - let installed_components_path = { - let installed_successfully = toolchain_dir.join("installation-successful"); - let installation_in_progress = toolchain_dir.join(".installation-in-progress"); - - if installed_successfully.exists() { - installed_successfully - } else if installation_in_progress.exists() { - // If this file exists, it means that installation got cut off before finishing. - // In this case, we simply delete the components that managed to get installed. - installation_in_progress - } else { - // If neither of those files are present, then we will rely on remove_dir_all to handle - // deletion - return Err(UninstallError::MissingInstalledComponentsFile( - toolchain_dir.to_path_buf(), - )); - } - }; - - // This is the channel.json at the time of installation. We use this to reconstruct the - // Component struct and thus figure out how the component was installed, i.e git, cargo, path. - let channel_content_path = toolchain_dir.join(".installed_channel.json"); - let channel_content = std::fs::read_to_string(&channel_content_path).map_err(|err| { - UninstallError::ChannelJsonMissing(channel_content_path.clone(), err.to_string()) - })?; - - let channel = serde_json::from_str::(&channel_content).map_err(|err| { - UninstallError::IllFormedChannelJson(channel_content_path, channel_content, err.to_string()) - })?; - - // We check the existance above - let components: Vec<&Component> = std::fs::read_to_string(&installed_components_path) - .unwrap() - .lines() - .map(String::from) - .map(|channel_name| channel.get_component(channel_name)) - .collect::>>() - .expect("Couldn't find installed component in channel"); - - // Right after reading the components list, we delete the file. This way, if anything goes wrong - // during uninstallation, a user can simply re-install to get back to a "stable" state. - // - // NOTE: We are ignoring errors when deleting this file, since it will (hopefully) get deleted - // at the end of this function. - let _ = std::fs::remove_file(installed_components_path); - - let (installed_libraries, installed_executables): (Vec<&Component>, Vec<&Component>) = - components - .iter() - .partition(|c| matches!(c.get_installed_file(), InstalledFile::Library { .. })); - - for lib in installed_libraries { - let lib_path = toolchain_dir.join("lib").join(lib.name.as_ref()).with_extension("masp"); - std::fs::remove_file(&lib_path) - .map_err(|err| UninstallError::FailedToDeleteFile(lib_path, err.to_string()))?; - } - - for exe in installed_executables { - match &exe.version { - Authority::Cargo { package, .. } => { - let package_name = package.as_deref().unwrap_or(exe.name.as_ref()); - uninstall_executable(package_name, toolchain_dir)?; - }, - Authority::Git { crate_name, .. } => { - uninstall_executable(crate_name, toolchain_dir)?; - }, - Authority::Path { crate_name, .. } => { - uninstall_executable(crate_name, toolchain_dir)?; - }, - } +#[derive(Debug)] +enum InstallationMethod { + Cargo, + Artifact +} +// Returns how the component got installed +// NOTE: Only intended to be used for Executable +// Components that came from a Local Manifest. +fn figure_installation_method_out(comp: &Component) -> InstallationMethod { + if !comp.artifacts.is_none() { + InstallationMethod::Artifact + } else { + InstallationMethod::Cargo } - - Ok(()) } +// pub fn uninstall_executable( +// name: impl AsRef + Display, +// root_dir: impl AsRef, +// ) -> Result<(), UninstallError> { +// todo!() +// } + pub fn uninstall_executable( name: impl AsRef + Display, root_dir: impl AsRef, -) -> Result<(), UninstallError> { +) -> Result<(), CargoError> { let mut remove_exe = std::process::Command::new("cargo") .arg("uninstall") .arg(&name) @@ -193,11 +138,11 @@ pub fn uninstall_executable( .stderr(std::process::Stdio::inherit()) .stdout(std::process::Stdio::inherit()) .spawn() - .map_err(|err| UninstallError::InternalCargoError(err.to_string()))?; + .map_err(|err| CargoError::InternalCargoError(err.to_string()))?; let status = remove_exe .wait() - .map_err(|err| UninstallError::InternalCargoError(err.to_string()))?; + .map_err(|err| CargoError::InternalCargoError(err.to_string()))?; if !status.success() { let error = remove_exe.stderr.take(); @@ -215,7 +160,7 @@ pub fn uninstall_executable( String::from("") }; - return Err(UninstallError::FailedToUninstallPackage( + return Err(CargoError::FailedToUninstallPackage( name.to_string(), status.code().unwrap_or(1), error_msg, @@ -224,3 +169,132 @@ pub fn uninstall_executable( Ok(()) } + +// /// Uninstalls a [Component] that was installed via an artifact +// fn uninstall_binary_artifact(config: &Config, local_channel: &Channel, component: Component) { +// } + +fn uninstall_channel(config: &Config, local_channel: &Channel) -> Result<(), UninstallError> { + let installed_toolchains_dir = config.midenup_home.join("toolchains"); + + let toolchain_dir = installed_toolchains_dir.join(format!("{}", &local_channel.name)); + if !toolchain_dir.exists() { + return Err(UninstallError::ChannelNotInstalled(local_channel.name.clone())); + }; + let lib_dir = toolchain_dir.join("lib"); + let bin_dir = toolchain_dir.join("bin"); + + for comp in local_channel.components.iter() { + match comp.get_installed_file() { + InstalledFile::Executable { binary_name, ..} => { + let installation_method = figure_installation_method_out(comp); + match installation_method { + InstallationMethod::Artifact => { + let artifact_path = bin_dir.join(binary_name); + std::fs::remove_file(artifact_path).unwrap(); + }, + InstallationMethod::Cargo => { + // Even if uninstallation fails, we will remove the + // entire directory regardless. + let _uninstall_result = match &comp.version { + Authority::Cargo { package, .. } => { + let package_name = package.as_deref() + .unwrap_or(comp.name.as_ref()); + uninstall_executable(package_name, &toolchain_dir) + }, + Authority::Git { crate_name, .. } => { + uninstall_executable(crate_name, &toolchain_dir) + }, + Authority::Path { crate_name, .. } => { + uninstall_executable(crate_name, &toolchain_dir) + }, + }?; + }, + } + }, + InstalledFile::Library {..} => { + let lib_path = lib_dir.join(comp.name.as_ref()).with_extension("masp"); + std::fs::remove_file(&lib_path) + .map_err(|err| UninstallError::FailedToDeleteFile(lib_path, err.to_string()))?; + } + } + } + + // We now remove the toolchain directory with all the remaining files. + std::fs::remove_dir_all(&toolchain_dir) + .map_err(|e| UninstallError::FailedToDeleteDirectory(toolchain_dir, e))?; + + // let installed_components_path = { + // let installed_successfully = toolchain_dir.join("installation-successful"); + // let installation_in_progress = toolchain_dir.join(".installation-in-progress"); + // if installed_successfully.exists() { + // installed_successfully + // } else if installation_in_progress.exists() { + // // If this file exists, it means that installation got cut off before finishing. + // // In this case, we simply delete the components that managed to get installed. + // installation_in_progress + // } else { + // // If neither of those files are present, then we will rely on remove_dir_all to handle + // // deletion + // return Err(UninstallError::MissingInstalledComponentsFile( + // toolchain_dir.to_path_buf(), + // )); + // } + // }; + + // // This is the channel.json at the time of installation. We use this to reconstruct the + // // Component struct and thus figure out how the component was installed, i.e git, cargo, path. + // let channel_content_path = toolchain_dir.join(".installed_channel.json"); + // let channel_content = std::fs::read_to_string(&channel_content_path).map_err(|err| { + // UninstallError::ChannelJsonMissing(channel_content_path.clone(), err.to_string()) + // })?; + + // let channel = serde_json::from_str::(&channel_content).map_err(|err| { + // UninstallError::IllFormedChannelJson(channel_content_path, channel_content, err.to_string()) + // })?; + + // // We check the existance above + // let components: Vec<&Component> = std::fs::read_to_string(&installed_components_path) + // .unwrap() + // .lines() + // .map(String::from) + // .map(|channel_name| channel.get_component(channel_name)) + // .collect::>>() + // .expect("Couldn't find installed component in channel"); + + // // Right after reading the components list, we delete the file. This way, if anything goes wrong + // // during uninstallation, a user can simply re-install to get back to a "stable" state. + // // + // // NOTE: We are ignoring errors when deleting this file, since it will (hopefully) get deleted + // // at the end of this function. + // let _ = std::fs::remove_file(installed_components_path); + + // let (installed_libraries, installed_executables): (Vec<&Component>, Vec<&Component>) = + // components + // .iter() + // .partition(|c| matches!(c.get_installed_file(), InstalledFile::Library { .. })); + + // for lib in installed_libraries { + // let lib_path = toolchain_dir.join("lib").join(lib.name.as_ref()).with_extension("masp"); + // std::fs::remove_file(&lib_path) + // .map_err(|err| UninstallError::FailedToDeleteFile(lib_path, err.to_string()))?; + // } + + // for exe in installed_executables { + // match &exe.version { + // Authority::Cargo { package, .. } => { + // let package_name = package.as_deref().unwrap_or(exe.name.as_ref()); + // uninstall_executable(package_name, toolchain_dir)?; + // }, + // Authority::Git { crate_name, .. } => { + // uninstall_executable(crate_name, toolchain_dir)?; + // }, + // Authority::Path { crate_name, .. } => { + // uninstall_executable(crate_name, toolchain_dir)?; + // }, + // } + // } + + Ok(()) +} + diff --git a/src/commands/update.rs b/src/commands/update.rs index fb0abea9..87fe9bc1 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -7,7 +7,7 @@ use crate::{ channel::{ Channel, Component, InstalledFile, MigrationStrategy, UserChannel, is_toolchain_deleted, }, - commands::{self, uninstall::uninstall_executable}, + commands::{self, uninstall::{CargoError, uninstall_executable}}, config::Config, manifest::Manifest, options::{PathUpdate, UpdateOptions}, @@ -242,7 +242,7 @@ fn update_channel( // `toolchain_dir`. If the package is not found, then the new, updated, version can // simply be placed in the directory without any issues. This discrepancy can be // caused when an update is cut mid-way through. - Err(commands::uninstall::UninstallError::FailedToUninstallPackage(name, ..)) => { + Err(CargoError::FailedToUninstallPackage(name, ..)) => { println!( "INFO: Failed to uninstall old version of {name}. Proceeding regardless." ); From 3196fb7df61b2eae638e12a3dc3b17920d3b37c7 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 4 May 2026 17:30:44 -0300 Subject: [PATCH 06/16] feat: (WIP) atomic install --- src/commands/install.rs | 76 +++++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 10 deletions(-) diff --git a/src/commands/install.rs b/src/commands/install.rs index 0125feb6..2438145a 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -39,14 +39,40 @@ pub fn install( bail!("the '{}' toolchain is already installed", &channel.name); } - if !toolchain_dir.exists() { - std::fs::create_dir_all(&toolchain_dir).with_context(|| { - format!("failed to create toolchain directory: '{}'", toolchain_dir.display()) + let staging_root = config.midenup_home.join("tmp"); + if !staging_root.exists() { + std::fs::create_dir_all(&staging_root).with_context(|| { + format!("failed to create staging root directory: '{}'", staging_root.display()) + })?; + } + // Install everything into a staging directory first. Only once the install script + // finishes successfully do we atomically move the staging directory into its final + // location under `toolchains/`. This prevents leaving a half-installed toolchain in + // the live directory if anything goes wrong mid-install. + let staging_dir = staging_root.join(format!("{}", &channel.name)); + + // If there exists a leftover staging directory from a previous install, we + // re-use it in order to save time. + if !staging_dir.exists() { + std::fs::create_dir_all(&staging_dir).with_context(|| { + format!("failed to create staging directory: '{}'", staging_dir.display()) + })?; + } + + // If the toolchain directory is already installed, we copy the files in order + // to save time. + if toolchain_dir.exists() { + copy_dir_recursive(&toolchain_dir, &staging_dir).with_context(|| { + format!( + "failed to seed staging directory '{}' from partial install at '{}'", + staging_dir.display(), + toolchain_dir.display() + ) })?; } // `bin/` directory which holds binaries. - let bin_dir = toolchain_dir.join("bin"); + let bin_dir = staging_dir.join("bin"); if !bin_dir.exists() { std::fs::create_dir_all(&bin_dir).with_context(|| { format!("failed to create toolchain directory: '{}'", bin_dir.display()) @@ -54,7 +80,7 @@ pub fn install( } // `lib/` directory which holds MASP libraries. - let lib_dir = toolchain_dir.join("lib"); + let lib_dir = staging_dir.join("lib"); if !lib_dir.exists() { std::fs::create_dir_all(&lib_dir).with_context(|| { format!("failed to create toolchain directory: '{}'", lib_dir.display()) @@ -70,14 +96,14 @@ pub fn install( // Then, when `miden` is invoked, it uses these symlinks to execute the underlying binary. With // this setup, `clap` displays the name as: `miden ` instead of just // `binary_name` when displaying help messages. - let opt_dir = toolchain_dir.join("opt"); + let opt_dir = staging_dir.join("opt"); if !opt_dir.exists() { std::fs::create_dir_all(&opt_dir).with_context(|| { format!("failed to create toolchain directory: '{}'", opt_dir.display()) })?; } - let install_file_path = toolchain_dir.join("install").with_extension("rs"); + let install_file_path = staging_dir.join("install").with_extension("rs"); // NOTE: Even when performing an update, we still need to re-generate the install script. // This is because, the versions that will be installed are written directly into the file; so // the file can't be "re-used". @@ -85,16 +111,16 @@ pub fn install( format!("failed to create file for install script at '{}'", install_file_path.display()) })?; - let install_script_contents = generate_install_script(config, channel, options, &toolchain_dir); + let install_script_contents = generate_install_script(config, channel, options, &staging_dir); install_file.write_all(&install_script_contents.into_bytes()).with_context(|| { format!("failed to write install script at '{}'", install_file_path.display()) })?; let mut child = std::process::Command::new("cargo") - .env("MIDEN_SYSROOT", &toolchain_dir) + .env("MIDEN_SYSROOT", &staging_dir) // HACK(pauls): This is for the benefit of the compiler, until it moves to using // MIDEN_SYSROOT instead. - .env("MIDENC_SYSROOT", &toolchain_dir) + .env("MIDENC_SYSROOT", &staging_dir) .args(["+nightly", "-Zscript"]) .arg(&install_file_path) .stderr(std::process::Stdio::inherit()) @@ -235,6 +261,36 @@ pub fn install( Ok(()) } +/// Recursively copy every entry from `src` into `dst`, preserving the directory layout and +/// following symlinks. `dst` is expected to already exist. +fn copy_dir_recursive(src: &Path, dst: &Path) -> anyhow::Result<()> { + for entry in std::fs::read_dir(src) + .with_context(|| format!("failed to read directory '{}'", src.display()))? + { + let entry = entry + .with_context(|| format!("failed to read entry in '{}'", src.display()))?; + let file_type = entry.file_type().with_context(|| { + format!("failed to stat entry '{}'", entry.path().display()) + })?; + let target = dst.join(entry.file_name()); + if file_type.is_dir() { + std::fs::create_dir_all(&target).with_context(|| { + format!("failed to create directory '{}'", target.display()) + })?; + copy_dir_recursive(&entry.path(), &target)?; + } else { + std::fs::copy(entry.path(), &target).with_context(|| { + format!( + "failed to copy '{}' to '{}'", + entry.path().display(), + target.display() + ) + })?; + } + } + Ok(()) +} + /// This function generates the install script that will later be saved in /// `midenup/toolchains//install.rs`. /// From 58c059cdddb58b1175b6c81f96070d05b6e45598 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 5 May 2026 16:15:50 -0300 Subject: [PATCH 07/16] feat: check if the file is a symlink when copying --- src/commands/install.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/commands/install.rs b/src/commands/install.rs index 2438145a..c57bc4ed 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -273,7 +273,18 @@ fn copy_dir_recursive(src: &Path, dst: &Path) -> anyhow::Result<()> { format!("failed to stat entry '{}'", entry.path().display()) })?; let target = dst.join(entry.file_name()); - if file_type.is_dir() { + if file_type.is_symlink() { + let link_target = std::fs::read_link(entry.path()).with_context(|| { + format!("failed to read symlink '{}'", entry.path().display()) + })?; + utils::fs::symlink(&target, &link_target).with_context(|| { + format!( + "failed to recreate symlink '{}' -> '{}'", + target.display(), + link_target.display() + ) + })?; + } else if file_type.is_dir() { std::fs::create_dir_all(&target).with_context(|| { format!("failed to create directory '{}'", target.display()) })?; From f7cd9812cc116775f70e101f0166bb4822366955 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 5 May 2026 16:16:19 -0300 Subject: [PATCH 08/16] feat: use relative pathes when symlinking --- src/commands/install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/install.rs b/src/commands/install.rs index c57bc4ed..4aff5768 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -519,7 +519,7 @@ fn main() { {%- for link in symlinks %} let new_link = opt_dir.join("{{ link.alias }}"); - let executable = bin_dir.join("{{ link.binary }}"); + let executable = std::path::Path::new("../bin").join("{{ link.binary }}"); if std::fs::read_link(&new_link).is_err() { utility::symlink(&new_link, &executable); } From 97151f5af66e58c6876d891be951e53e16f43577 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 5 May 2026 18:31:41 -0300 Subject: [PATCH 09/16] TMP: atomic remove --- src/commands/install.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/commands/install.rs b/src/commands/install.rs index 4aff5768..ff16e6e6 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -140,6 +140,29 @@ pub fn install( ) } + let had_existing = toolchain_dir.exists(); + if had_existing { + std::fs::rename(&toolchain_dir, &backup_dir).with_context(|| { + format!( + "failed to move existing toolchain '{}' aside to '{}'", + toolchain_dir.display(), + backup_dir.display() + ) + })?; + } + std::fs::rename(&staging_dir, &toolchain_dir).with_context(|| { + format!( + "failed to move staged toolchain from '{}' to '{}'", + staging_dir.display(), + toolchain_dir.display() + ) + })?; + if had_existing { + std::fs::remove_dir_all(&backup_dir).with_context(|| { + format!("failed to remove backup toolchain '{}'", backup_dir.display()) + })?; + } + let is_latest_stable = config.manifest.is_latest_stable(channel); // If this channel is the new stable, we update the symlink From 8352d5b0190168a9e10364b274400c8a2e782bd1 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 5 May 2026 19:09:36 -0300 Subject: [PATCH 10/16] feat: recover .bak files --- src/commands/init.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/commands/init.rs b/src/commands/init.rs index e90981e0..41500ebc 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -19,6 +19,48 @@ fn cargo_bin_dir() -> anyhow::Result { anyhow::bail!("Could not determine cargo bin directory. Set CARGO_HOME or HOME.") } +/// Check if there are any `` directories in `toolchain/` that could've been left out by an interrumpted install. +/// +/// If the `` (old_channel) has a `.bak` copy, two possible scenarios exist: +/// - If the real `` directory exists, then the only step that was missing was to remove the `.bak` file. +/// - If `/` is missing, then the rename wasn't executed. +fn recover_toolchains(toolchains_dir: &std::path::Path) -> anyhow::Result<()> { + for entry in std::fs::read_dir(toolchains_dir).with_context(|| { + format!("failed to read toolchains directory '{}'", toolchains_dir.display()) + })? { + let entry = entry.with_context(|| { + format!("failed to read entry in '{}'", toolchains_dir.display()) + })?; + let file_name = entry.file_name(); + let Some(file_name) = file_name.to_str() else { + continue; + }; + + let Some(old_channel) = file_name.strip_suffix(".bak") else { + continue; + }; + + let backup_dir = entry.path(); + let old_channel_dir = toolchains_dir.join(old_channel); + + if old_channel_dir.exists() { + std::fs::remove_dir_all(&backup_dir).with_context(|| { + format!("failed to remove leftover backup directory '{}'", backup_dir.display()) + })?; + } else { + std::fs::rename(&backup_dir, &old_channel_dir).with_context(|| { + format!( + "failed to restore toolchain from backup '{}' to '{}'", + backup_dir.display(), + old_channel_dir.display() + ) + })?; + } + } + Ok(()) +} + + /// This functions bootstrap the `midenup` environment, if not already initialized. /// /// Initialization is comprised of: @@ -95,6 +137,8 @@ pub fn setup_midenup(config: &Config) -> anyhow::Result { already_initialized = false; } + recover_toolchains(&toolchains_dir)?; + // We check if the `miden` executable is accessible via the $PATH. This is most certainly not // going to be the case the first time `midenup` is initialized. let miden_is_accessible = std::process::Command::new("miden") From 96737068e5a30e927146c70eedb546d7d144d0ba Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 6 May 2026 11:44:05 -0300 Subject: [PATCH 11/16] Revert "feat: recover .bak files" This reverts commit 8352d5b0190168a9e10364b274400c8a2e782bd1. --- src/commands/init.rs | 44 -------------------------------------------- 1 file changed, 44 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index 41500ebc..e90981e0 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -19,48 +19,6 @@ fn cargo_bin_dir() -> anyhow::Result { anyhow::bail!("Could not determine cargo bin directory. Set CARGO_HOME or HOME.") } -/// Check if there are any `` directories in `toolchain/` that could've been left out by an interrumpted install. -/// -/// If the `` (old_channel) has a `.bak` copy, two possible scenarios exist: -/// - If the real `` directory exists, then the only step that was missing was to remove the `.bak` file. -/// - If `/` is missing, then the rename wasn't executed. -fn recover_toolchains(toolchains_dir: &std::path::Path) -> anyhow::Result<()> { - for entry in std::fs::read_dir(toolchains_dir).with_context(|| { - format!("failed to read toolchains directory '{}'", toolchains_dir.display()) - })? { - let entry = entry.with_context(|| { - format!("failed to read entry in '{}'", toolchains_dir.display()) - })?; - let file_name = entry.file_name(); - let Some(file_name) = file_name.to_str() else { - continue; - }; - - let Some(old_channel) = file_name.strip_suffix(".bak") else { - continue; - }; - - let backup_dir = entry.path(); - let old_channel_dir = toolchains_dir.join(old_channel); - - if old_channel_dir.exists() { - std::fs::remove_dir_all(&backup_dir).with_context(|| { - format!("failed to remove leftover backup directory '{}'", backup_dir.display()) - })?; - } else { - std::fs::rename(&backup_dir, &old_channel_dir).with_context(|| { - format!( - "failed to restore toolchain from backup '{}' to '{}'", - backup_dir.display(), - old_channel_dir.display() - ) - })?; - } - } - Ok(()) -} - - /// This functions bootstrap the `midenup` environment, if not already initialized. /// /// Initialization is comprised of: @@ -137,8 +95,6 @@ pub fn setup_midenup(config: &Config) -> anyhow::Result { already_initialized = false; } - recover_toolchains(&toolchains_dir)?; - // We check if the `miden` executable is accessible via the $PATH. This is most certainly not // going to be the case the first time `midenup` is initialized. let miden_is_accessible = std::process::Command::new("miden") From 3a815281932977024a91c41a1f4e1579fe1da8a2 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 6 May 2026 16:27:38 -0300 Subject: [PATCH 12/16] feat: atomic install with symlink + rename --- src/commands/init.rs | 23 +++++++-- src/commands/install.rs | 106 ++++++++++++++++------------------------ 2 files changed, 61 insertions(+), 68 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index e90981e0..6e1bfa71 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -33,14 +33,16 @@ fn cargo_bin_dir() -> anyhow::Result { /// /// ```text,ignore /// $MIDENUP_HOME -/// |- opt/ -/// | |- symlinks -/// |- toolchains -/// | |- stable/ --> / -/// | |- / +/// |- toolchains/ +/// | |- stable --> +/// | |- --> ../installed_toolchains/- +/// |- installed_toolchains/ +/// | |- -/ /// | | |- bin/ /// | | |- lib/ /// | | | |- std.masp +/// | | |- opt/ +/// | | |- var/ /// |- config.toml /// |- manifest.json /// ``` @@ -95,6 +97,17 @@ pub fn setup_midenup(config: &Config) -> anyhow::Result { already_initialized = false; } + let installed_toolchains_dir = config.midenup_home.join("installed_toolchains"); + if !installed_toolchains_dir.exists() { + std::fs::create_dir_all(&installed_toolchains_dir).with_context(|| { + format!( + "failed to initialize MIDENUP_HOME subdirectory: '{}'", + installed_toolchains_dir.display() + ) + })?; + already_initialized = false; + } + // We check if the `miden` executable is accessible via the $PATH. This is most certainly not // going to be the case the first time `midenup` is initialized. let miden_is_accessible = std::process::Command::new("miden") diff --git a/src/commands/install.rs b/src/commands/install.rs index ff16e6e6..ae8063fa 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -22,8 +22,12 @@ pub fn install( ) -> anyhow::Result<()> { commands::setup_midenup(config)?; - let installed_toolchains_dir = config.midenup_home.join("toolchains"); - let toolchain_dir = installed_toolchains_dir.join(format!("{}", &channel.name)); + let toolchains_dir = config.midenup_home.join("toolchains"); + let installed_toolchains_dir = config.midenup_home.join("installed_toolchains"); + let toolchain_dir = toolchains_dir.join(format!("{}", &channel.name)); + + let install_dir_name = format!("{}-{}", &channel.name, channel.content_hash()); + let install_dir = installed_toolchains_dir.join(&install_dir_name); let installation_indicator = toolchain_dir.join("installation-successful"); let is_partial = local_manifest @@ -39,48 +43,23 @@ pub fn install( bail!("the '{}' toolchain is already installed", &channel.name); } - let staging_root = config.midenup_home.join("tmp"); - if !staging_root.exists() { - std::fs::create_dir_all(&staging_root).with_context(|| { - format!("failed to create staging root directory: '{}'", staging_root.display()) - })?; - } - // Install everything into a staging directory first. Only once the install script - // finishes successfully do we atomically move the staging directory into its final - // location under `toolchains/`. This prevents leaving a half-installed toolchain in - // the live directory if anything goes wrong mid-install. - let staging_dir = staging_root.join(format!("{}", &channel.name)); - - // If there exists a leftover staging directory from a previous install, we - // re-use it in order to save time. - if !staging_dir.exists() { - std::fs::create_dir_all(&staging_dir).with_context(|| { - format!("failed to create staging directory: '{}'", staging_dir.display()) - })?; - } - - // If the toolchain directory is already installed, we copy the files in order - // to save time. - if toolchain_dir.exists() { - copy_dir_recursive(&toolchain_dir, &staging_dir).with_context(|| { - format!( - "failed to seed staging directory '{}' from partial install at '{}'", - staging_dir.display(), - toolchain_dir.display() - ) + // If the install directory already exists; then that means we are re-issuing + // an install. That's probably because the installation got interrumpted + // mid way through. + if !install_dir.exists() { + std::fs::create_dir_all(&install_dir).with_context(|| { + format!("failed to create install directory: '{}'", install_dir.display()) })?; } - // `bin/` directory which holds binaries. - let bin_dir = staging_dir.join("bin"); + let bin_dir = install_dir.join("bin"); if !bin_dir.exists() { std::fs::create_dir_all(&bin_dir).with_context(|| { format!("failed to create toolchain directory: '{}'", bin_dir.display()) })?; } - // `lib/` directory which holds MASP libraries. - let lib_dir = staging_dir.join("lib"); + let lib_dir = install_dir.join("lib"); if !lib_dir.exists() { std::fs::create_dir_all(&lib_dir).with_context(|| { format!("failed to create toolchain directory: '{}'", lib_dir.display()) @@ -96,31 +75,28 @@ pub fn install( // Then, when `miden` is invoked, it uses these symlinks to execute the underlying binary. With // this setup, `clap` displays the name as: `miden ` instead of just // `binary_name` when displaying help messages. - let opt_dir = staging_dir.join("opt"); + let opt_dir = install_dir.join("opt"); if !opt_dir.exists() { std::fs::create_dir_all(&opt_dir).with_context(|| { format!("failed to create toolchain directory: '{}'", opt_dir.display()) })?; } - let install_file_path = staging_dir.join("install").with_extension("rs"); - // NOTE: Even when performing an update, we still need to re-generate the install script. - // This is because, the versions that will be installed are written directly into the file; so - // the file can't be "re-used". + let install_file_path = install_dir.join("install").with_extension("rs"); let mut install_file = std::fs::File::create(&install_file_path).with_context(|| { format!("failed to create file for install script at '{}'", install_file_path.display()) })?; - let install_script_contents = generate_install_script(config, channel, options, &staging_dir); + let install_script_contents = generate_install_script(config, channel, options, &install_dir); install_file.write_all(&install_script_contents.into_bytes()).with_context(|| { format!("failed to write install script at '{}'", install_file_path.display()) })?; let mut child = std::process::Command::new("cargo") - .env("MIDEN_SYSROOT", &staging_dir) + .env("MIDEN_SYSROOT", &install_dir) // HACK(pauls): This is for the benefit of the compiler, until it moves to using // MIDEN_SYSROOT instead. - .env("MIDENC_SYSROOT", &staging_dir) + .env("MIDENC_SYSROOT", &install_dir) .args(["+nightly", "-Zscript"]) .arg(&install_file_path) .stderr(std::process::Stdio::inherit()) @@ -140,39 +116,43 @@ pub fn install( ) } - let had_existing = toolchain_dir.exists(); - if had_existing { - std::fs::rename(&toolchain_dir, &backup_dir).with_context(|| { - format!( - "failed to move existing toolchain '{}' aside to '{}'", - toolchain_dir.display(), - backup_dir.display() - ) + // Atomically publish the install via a symlink rename. The temp symlink + rename is the + // standard trick: rename(2) on Unix replaces an existing destination atomically when + // both source and destination are non-directories (symlinks count). + let relative_install_target = PathBuf::from("..").join("installed_toolchains").join(&install_dir_name); + let temp_symlink = installed_toolchains_dir.join(format!("{}.new", &channel.name)); + if std::fs::symlink_metadata(&temp_symlink).is_ok() { + std::fs::remove_file(&temp_symlink).with_context(|| { + format!("failed to remove stale temp symlink '{}'", temp_symlink.display()) })?; } - std::fs::rename(&staging_dir, &toolchain_dir).with_context(|| { + // For further reference on atomic directory updates, see: + // https://axialcorps.wordpress.com/2013/07/03/atomically-replacing-files-and-directories/ + // tmp_link is a symlink file that points to relative_install_target. Even + // if tmp_link file is moved, it will still point to relative_install_target. + utils::fs::symlink(&temp_symlink, &relative_install_target)?; + + // we now rename tmp_link to toolchain_dir. When renamed, it will still be + // pointing to relative_install_target. And if it existed, it will overwrite + // the file. This is what makes the install as completed. + std::fs::rename(&temp_symlink, &toolchain_dir).with_context(|| { format!( - "failed to move staged toolchain from '{}' to '{}'", - staging_dir.display(), - toolchain_dir.display() + "failed to publish toolchain symlink '{}' -> '{}'", + toolchain_dir.display(), + relative_install_target.display() ) })?; - if had_existing { - std::fs::remove_dir_all(&backup_dir).with_context(|| { - format!("failed to remove backup toolchain '{}'", backup_dir.display()) - })?; - } let is_latest_stable = config.manifest.is_latest_stable(channel); // If this channel is the new stable, we update the symlink if is_latest_stable { - // NOTE: This is an absolute file path, maybe a relative symlink would be more suitable - let stable_dir = installed_toolchains_dir.join("stable"); + let stable_dir = toolchains_dir.join("stable"); if stable_dir.exists() { std::fs::remove_file(&stable_dir).context("Couldn't remove stable symlink")?; } - utils::fs::symlink(&stable_dir, &toolchain_dir).expect("Couldn't create stable dir"); + let relative_channel_target = PathBuf::from(format!("{}", &channel.name)); + utils::fs::symlink(&stable_dir, &relative_channel_target).expect("Couldn't create stable dir"); } // Update local manifest From f25a5d6cb86d3c0553cca1b6e8c5c14d6280756a Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 6 May 2026 16:52:52 -0300 Subject: [PATCH 13/16] feat: re-use components --- src/commands/install.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/commands/install.rs b/src/commands/install.rs index ae8063fa..707b4b8c 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -50,6 +50,16 @@ pub fn install( std::fs::create_dir_all(&install_dir).with_context(|| { format!("failed to create install directory: '{}'", install_dir.display()) })?; + // If a previous install of this channel exists, reuse the components. + if toolchain_dir.exists() { + copy_dir_recursive(&toolchain_dir, &install_dir).with_context(|| { + format!( + "failed to seed install directory '{}' from previous install at '{}'", + install_dir.display(), + toolchain_dir.display() + ) + })?; + } } let bin_dir = install_dir.join("bin"); From 26fce3472115e80e58fb31d369d96eecb0549d22 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 7 May 2026 09:46:43 -0300 Subject: [PATCH 14/16] WIP: comment out Component hash uses --- src/channel.rs | 34 +++++++-------- src/commands/update.rs | 93 +++++++++++++++++++++--------------------- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/src/channel.rs b/src/channel.rs index 2b64019f..e5315fd1 100644 --- a/src/channel.rs +++ b/src/channel.rs @@ -243,14 +243,14 @@ impl PartialEq for Component { } } -impl Hash for Component { - fn hash(&self, state: &mut H) - where - H: Hasher, - { - self.name.hash(state) - } -} +// impl Hash for Component { +// fn hash(&self, state: &mut H) +// where +// H: Hasher, +// { +// self.name.hash(state) +// } +// } impl PartialEq for Channel { fn eq(&self, other: &Self) -> bool { @@ -268,17 +268,19 @@ impl PartialEq for Channel { return false; } - let my_components: std::collections::HashSet = - self.components.clone().into_iter().collect(); + // TODO: CHANGE + panic!(); + // let my_components: std::collections::HashSet = + // self.components.clone().into_iter().collect(); - let other_components: std::collections::HashSet = - self.components.clone().into_iter().collect(); + // let other_components: std::collections::HashSet = + // self.components.clone().into_iter().collect(); - let equal_components = other_components == my_components; + // let equal_components = other_components == my_components; - if !equal_components { - return false; - } + // if !equal_components { + // return false; + // } true } diff --git a/src/commands/update.rs b/src/commands/update.rs index 87fe9bc1..52b97536 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -326,52 +326,53 @@ pub enum UpdateMotive { /// There is one notable exception to this rule which is when a channel is migrated into a different /// channel. In that case, every component is marked for update. pub fn components_to_update(older: &Channel, newer: &Channel) -> Vec<(Component, UpdateMotive)> { - let new_channel: HashSet<&Component> = HashSet::from_iter(newer.components.iter()); - let current = HashSet::from_iter(older.components.iter()); - - // This is the subset of new components present in the channel since last sync. - // - // NOTE: Equality between components is done via their name, see [Component::eq]. - let new_components = new_channel.difference(¤t).map(|comp| (comp, UpdateMotive::Added)); - - // This is the subset of old components that need to be removed. - let old_components = current.difference(&new_channel).map(|comp| (comp, UpdateMotive::Removed)); - - // These are the elements that are present in boths sets. We are only interested in those which - // need updating. - let components_to_update = current - .iter() - .filter(|comp| new_channel.contains(**comp)) - .filter_map(|current_component| { - let new_component = new_channel.get(*current_component); - match new_component { - // This should't be possible, but if somehow the component is missing, then we - // trigger an update for said component regardless. - None => Some((current_component, UpdateMotive::Added)), - // If the new channel was migrated, then every component should be deleted; unless - // explicitely told otherwise by the users (for example in components which were - // compile from a path at a given time). - Some(new_component) => { - if let Some(strategy) = newer.migrated_into() { - Some(( - current_component, - UpdateMotive::Migrated { strategy: strategy.clone() }, - )) - } else if !current_component.is_up_to_date(new_component) { - Some((current_component, UpdateMotive::NewerVersion)) - } else { - None - } - }, - } - }); - - let components = new_components - .chain(old_components) - .chain(components_to_update) - .map(|(comp, motive)| ((*comp).clone(), motive)); - - Vec::from_iter(components) + todo!(); + // let new_channel: HashSet<&Component> = HashSet::from_iter(newer.components.iter()); + // let current = HashSet::from_iter(older.components.iter()); + + // // This is the subset of new components present in the channel since last sync. + // // + // // NOTE: Equality between components is done via their name, see [Component::eq]. + // let new_components = new_channel.difference(¤t).map(|comp| (comp, UpdateMotive::Added)); + + // // This is the subset of old components that need to be removed. + // let old_components = current.difference(&new_channel).map(|comp| (comp, UpdateMotive::Removed)); + + // // These are the elements that are present in boths sets. We are only interested in those which + // // need updating. + // let components_to_update = current + // .iter() + // .filter(|comp| new_channel.contains(**comp)) + // .filter_map(|current_component| { + // let new_component = new_channel.get(*current_component); + // match new_component { + // // This should't be possible, but if somehow the component is missing, then we + // // trigger an update for said component regardless. + // None => Some((current_component, UpdateMotive::Added)), + // // If the new channel was migrated, then every component should be deleted; unless + // // explicitely told otherwise by the users (for example in components which were + // // compile from a path at a given time). + // Some(new_component) => { + // if let Some(strategy) = newer.migrated_into() { + // Some(( + // current_component, + // UpdateMotive::Migrated { strategy: strategy.clone() }, + // )) + // } else if !current_component.is_up_to_date(new_component) { + // Some((current_component, UpdateMotive::NewerVersion)) + // } else { + // None + // } + // }, + // } + // }); + + // let components = new_components + // .chain(old_components) + // .chain(components_to_update) + // .map(|(comp, motive)| ((*comp).clone(), motive)); + + // Vec::from_iter(components) } fn display_warnings( From cbc5e3e76250bbf0a41f3c3131c6fa23d58241c6 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 7 May 2026 11:15:17 -0300 Subject: [PATCH 15/16] feat: add Channel hashing --- src/artifact.rs | 4 ++-- src/channel.rs | 38 +++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/artifact.rs b/src/artifact.rs index 2f3a302d..52028d5c 100644 --- a/src/artifact.rs +++ b/src/artifact.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; /// All the artifacts that the [Component] contains. -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] pub struct Artifacts { artifacts: Vec, } @@ -16,7 +16,7 @@ impl Artifacts { /// Holds a URI used to fetch an artifact. /// /// These URIs have the following format: `(https://|file://)/(-|.masp)` -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] struct Artifact(String); #[derive(Debug, PartialEq)] diff --git a/src/channel.rs b/src/channel.rs index e5315fd1..0e992e01 100644 --- a/src/channel.rs +++ b/src/channel.rs @@ -1,6 +1,6 @@ use std::{ borrow::Cow, - collections::HashMap, + collections::{BTreeMap, HashMap, hash_map::DefaultHasher}, fmt::{self, Display}, hash::{Hash, Hasher}, path::{Path, PathBuf}, @@ -18,14 +18,23 @@ use crate::{ version::{Authority, GitTarget}, }; -#[derive(Serialize, Deserialize, Debug, Clone)] +/// Hash created to distinguish one installed channel from the another. +pub struct ChannelHash(String); + +impl Display for ChannelHash { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] #[serde(untagged)] pub enum MigrationStrategy { NameChange { old_channel: semver::Version }, } /// Tags used to identify special qualities of a specific channel. -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] #[serde(rename_all = "snake_case")] pub enum Tags { /// The channel is partially installed, i.e. only a subset of components @@ -42,7 +51,7 @@ pub enum Tags { /// /// Different channels have different stability guarantees. See the specific details for the /// channel you are interested in to learn more. -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] pub struct Channel { /// Channels are identified by their name. The name corresponds to the channel's version. /// The version can contain suffixes such as "-custom", "-beta". @@ -137,6 +146,12 @@ impl Channel { installed_toolchains_dir.join(format!("{}", self.name)) } + pub fn content_hash(&self) -> ChannelHash { + let mut h = DefaultHasher::new(); + self.hash(&mut h); + ChannelHash(format!("{:016x}", h.finish())) + } + /// Get all the aliases that the Channel is aware of pub fn get_aliases(&self) -> HashMap { self.components.iter().fold(HashMap::new(), |mut acc, component| { @@ -302,7 +317,7 @@ impl Display for Channel { } /// A special alias/tag that a channel can posses. For more information see [`Channel::alias`]. -#[derive(Serialize, Debug, PartialEq, Eq, Clone)] +#[derive(Serialize, Debug, PartialEq, Eq, Clone, Hash)] #[serde(rename_all = "snake_case")] pub enum ChannelAlias { /// Represents `stable`. Only one [Channel] can be marked as `stable` at a time. @@ -349,7 +364,7 @@ impl core::str::FromStr for ChannelAlias { } /// Represents the file that the [Component] will install in the system. -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash)] #[serde(rename_all = "snake_case")] pub enum InstalledFile { /// The component installs an executable. @@ -412,7 +427,7 @@ impl Display for InstalledFile { /// Represents each possible "word" variant that is passed to the command line. /// /// These are used to resolve an [Alias] to its associated command. -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] #[serde(rename_all = "snake_case")] pub enum CliCommand { /// Resolve the command to a [Component]'s corresponding executable. @@ -519,7 +534,7 @@ pub type Alias = String; pub type CliCommands = Vec; /// An installable component of a toolchain -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash)] pub struct Component { /// The canonical name of this toolchain component. pub name: Cow<'static, str>, @@ -578,8 +593,8 @@ pub struct Component { /// }, /// ``` #[serde(default)] - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub aliases: HashMap, + #[serde(skip_serializing_if = "BTreeMap::is_empty")] + pub aliases: BTreeMap, /// The file used by midenup's 'miden' to call the components executable. /// /// If `None`, then the component's file will be saved as `miden `. This distinction @@ -605,7 +620,7 @@ impl Component { call_format: vec![], rustup_channel: None, installed_file: None, - aliases: HashMap::new(), + aliases: BTreeMap::new(), symlink_name: None, initialization: Vec::new(), artifacts: None, @@ -843,3 +858,4 @@ impl core::str::FromStr for UserChannel { } } } + From dfd82340fcd76646d287d5bd5f358e596d7a9b2c Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 7 May 2026 12:48:01 -0300 Subject: [PATCH 16/16] refactor: add ComponentByName struct, avoiding Component's Hash and Eq impls --- src/channel.rs | 21 ------- src/commands/update.rs | 135 ++++++++++++++++++++++++----------------- 2 files changed, 81 insertions(+), 75 deletions(-) diff --git a/src/channel.rs b/src/channel.rs index 0e992e01..be69b2ce 100644 --- a/src/channel.rs +++ b/src/channel.rs @@ -246,27 +246,6 @@ impl Channel { } } -impl Eq for Component {} - -/// NOTE: Two component are "partially equal" if their names are the same. -/// -/// This does not mean that they're equal, since they could differ in fields like versions. This is -/// implmented manually, in order to make use of HashSets with components. -impl PartialEq for Component { - fn eq(&self, other: &Self) -> bool { - self.name == other.name - } -} - -// impl Hash for Component { -// fn hash(&self, state: &mut H) -// where -// H: Hasher, -// { -// self.name.hash(state) -// } -// } - impl PartialEq for Channel { fn eq(&self, other: &Self) -> bool { // NOTE: To channels are equal regardless of their aliases diff --git a/src/commands/update.rs b/src/commands/update.rs index 52b97536..05c98897 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -1,4 +1,7 @@ -use std::collections::HashSet; +use std::{ + collections::HashSet, + hash::{Hash, Hasher}, +}; use anyhow::{Context, bail}; use colored::Colorize; @@ -38,11 +41,12 @@ midenup install stable .context("ERROR: No stable channel found in upstream")?; if upstream_stable.name > local_stable.name { - let component_subset = if local_stable.is_partially_installed() { - Some(local_stable.components.clone()) - } else { - None - }; + let component_subset: Option> = + if local_stable.is_partially_installed() { + Some(local_stable.components.iter().map(ComponentByName).collect()) + } else { + None + }; let channel_to_install = { let components = upstream_stable @@ -50,7 +54,7 @@ midenup install stable .iter() .filter(|comp| { if let Some(component_subset) = &component_subset { - component_subset.contains(comp) + component_subset.contains(&ComponentByName(*comp)) } else { true } @@ -310,6 +314,26 @@ pub enum UpdateMotive { Migrated { strategy: MigrationStrategy }, } +/// Wrapper around `&Component` that defines `Hash`/`Eq` by name only, so we can +/// use `HashSet` set operations (difference, intersection, contains) keyed on +/// names. This is not a property of component themselves, hence the wrapper +/// type. +/// +/// See https://stackoverflow.com/a/65671830 as a reference. +struct ComponentByName<'a>(&'a Component); + +impl PartialEq for ComponentByName<'_> { + fn eq(&self, other: &Self) -> bool { + self.0.name == other.0.name + } +} +impl Eq for ComponentByName<'_> {} +impl Hash for ComponentByName<'_> { + fn hash(&self, state: &mut H) { + self.0.name.hash(state); + } +} + /// This functions compares the Channel &older, with a newer channel [newer] and returns the list /// of [Components] that need to be updated. /// @@ -326,53 +350,56 @@ pub enum UpdateMotive { /// There is one notable exception to this rule which is when a channel is migrated into a different /// channel. In that case, every component is marked for update. pub fn components_to_update(older: &Channel, newer: &Channel) -> Vec<(Component, UpdateMotive)> { - todo!(); - // let new_channel: HashSet<&Component> = HashSet::from_iter(newer.components.iter()); - // let current = HashSet::from_iter(older.components.iter()); - - // // This is the subset of new components present in the channel since last sync. - // // - // // NOTE: Equality between components is done via their name, see [Component::eq]. - // let new_components = new_channel.difference(¤t).map(|comp| (comp, UpdateMotive::Added)); - - // // This is the subset of old components that need to be removed. - // let old_components = current.difference(&new_channel).map(|comp| (comp, UpdateMotive::Removed)); - - // // These are the elements that are present in boths sets. We are only interested in those which - // // need updating. - // let components_to_update = current - // .iter() - // .filter(|comp| new_channel.contains(**comp)) - // .filter_map(|current_component| { - // let new_component = new_channel.get(*current_component); - // match new_component { - // // This should't be possible, but if somehow the component is missing, then we - // // trigger an update for said component regardless. - // None => Some((current_component, UpdateMotive::Added)), - // // If the new channel was migrated, then every component should be deleted; unless - // // explicitely told otherwise by the users (for example in components which were - // // compile from a path at a given time). - // Some(new_component) => { - // if let Some(strategy) = newer.migrated_into() { - // Some(( - // current_component, - // UpdateMotive::Migrated { strategy: strategy.clone() }, - // )) - // } else if !current_component.is_up_to_date(new_component) { - // Some((current_component, UpdateMotive::NewerVersion)) - // } else { - // None - // } - // }, - // } - // }); - - // let components = new_components - // .chain(old_components) - // .chain(components_to_update) - // .map(|(comp, motive)| ((*comp).clone(), motive)); - - // Vec::from_iter(components) + let new_channel: HashSet = + newer.components.iter().map(ComponentByName).collect(); + let current: HashSet = + older.components.iter().map(ComponentByName).collect(); + + // This is the subset of new components present in the channel since last sync. + let new_components = new_channel + .difference(¤t) + .map(|&ComponentByName(comp)| (comp, UpdateMotive::Added)); + + // This is the subset of old components that need to be removed. + let old_components = current + .difference(&new_channel) + .map(|&ComponentByName(comp)| (comp, UpdateMotive::Removed)); + + // These are the elements that are present in boths sets. We are only interested in those which + // need updating. + let components_to_update = current + .iter() + .filter(|comp| new_channel.contains(*comp)) + .filter_map(|&ComponentByName(current_component)| { + let new_component = new_channel.get(&ComponentByName(current_component)); + match new_component { + // This should't be possible, but if somehow the component is missing, then we + // trigger an update for said component regardless. + None => Some((current_component, UpdateMotive::Added)), + // If the new channel was migrated, then every component should be deleted; unless + // explicitely told otherwise by the users (for example in components which were + // compile from a path at a given time). + Some(&ComponentByName(new_component)) => { + if let Some(strategy) = newer.migrated_into() { + Some(( + current_component, + UpdateMotive::Migrated { strategy: strategy.clone() }, + )) + } else if !current_component.is_up_to_date(new_component) { + Some((current_component, UpdateMotive::NewerVersion)) + } else { + None + } + }, + } + }); + + let components = new_components + .chain(old_components) + .chain(components_to_update) + .map(|(comp, motive)| (comp.clone(), motive)); + + Vec::from_iter(components) } fn display_warnings(