Fix concurrent add dropping downloaded version

devdocs/locking_and_update_flows.md

@@ -148,13 +148,20 @@ flowchart TD
     inst -->|yes| ret["print + return"]
     inst -->|no| dl["download_version_to_temp<br/><b>NO lock held</b>"]
     dl --> wl["load_mut_config_db<br/><b>exclusive lock</b>"]
-    wl --> recheck{"installed by<br/>another process?"}
-    recheck -->|yes| ret2["print + return"]
-    recheck -->|no| commit["commit_version_install<br/>(rename temp → final)"]
+    wl --> recheck{"concurrent channel<br/>with same name?"}
+    recheck -->|"same system version<br/>or explicit non-system channel"| ret2["discard download<br/>print + return"]
+    recheck -->|"different system version<br/>or no channel"| commit["commit_version_install<br/>(rename temp → final)"]
     commit --> cfg["insert channel + save_config_db"]
     cfg --> done(["release"])
 ```
 
+The commit re-check is version-aware. A concurrent system channel at the same
+version makes the download redundant, while a system channel at a different
+version is moved to the version resolved by this `add` and the download is
+registered for reuse. Concurrent linked, direct-download, and alias channels
+are explicit claims on the channel name, so `add` preserves them and discards
+its database-version download.
+
 For nightly/PR channels (`add_non_db`), `install_non_db_version` →
 `install_from_url` downloads into a temp dir and atomically renames into place
 with **no lock held**; the exclusive lock is taken afterwards only to write the

src/command_add.rs

@@ -1,5 +1,5 @@
 use crate::config_file::{
-    load_config_db, load_mut_config_db, save_config_db, JuliaupConfigChannel,
+    load_config_db, load_mut_config_db, save_config_db, JuliaupConfig, JuliaupConfigChannel,
 };
 use crate::global_paths::GlobalPaths;
 #[cfg(not(windows))]
@@ -12,6 +12,45 @@ use crate::utils::{print_juliaup_style, JuliaupMessageType};
 use crate::versions_file::load_versions_db;
 use anyhow::{anyhow, Context, Result};
 use regex::Regex;
+use tempfile::TempDir;
+
+#[derive(Debug, PartialEq)]
+enum AddChannelOutcome {
+    Installed,
+    AlreadyInstalled,
+}
+
+/// Commits a downloaded database version after re-checking the channel under
+/// the exclusive configuration lock.
+///
+/// A concurrent system-channel install only wins when it selected the same
+/// version. If it selected a different version, keep the completed download
+/// and move the channel to the version this `add` resolved before downloading.
+/// Non-system channels are explicit name claims and are never overwritten.
+fn commit_downloaded_channel(
+    channel: &str,
+    required_version: &str,
+    downloaded: TempDir,
+    config_data: &mut JuliaupConfig,
+    paths: &GlobalPaths,
+) -> Result<AddChannelOutcome> {
+    match config_data.installed_channels.get(channel) {
+        Some(JuliaupConfigChannel::SystemChannel { version }) if version != required_version => {}
+        Some(_) => return Ok(AddChannelOutcome::AlreadyInstalled),
+        None => {}
+    }
+
+    commit_version_install(downloaded, required_version, config_data, paths)?;
+
+    config_data.installed_channels.insert(
+        channel.to_string(),
+        JuliaupConfigChannel::SystemChannel {
+            version: required_version.to_string(),
+        },
+    );
+
+    Ok(AddChannelOutcome::Installed)
+}
 
 pub fn run_command_add(channel: &str, paths: &GlobalPaths) -> Result<()> {
     // This regex is dynamically compiled, but its runtime is negligible compared to downloading Julia
@@ -58,20 +97,18 @@ pub fn run_command_add(channel: &str, paths: &GlobalPaths) -> Result<()> {
     let mut config_file = load_mut_config_db(paths)
         .with_context(|| "`add` command failed to load configuration data.")?;
 
-    if config_file.data.installed_channels.contains_key(channel) {
+    if commit_downloaded_channel(
+        channel,
+        required_version,
+        downloaded,
+        &mut config_file.data,
+        paths,
+    )? == AddChannelOutcome::AlreadyInstalled
+    {
         eprintln!("'{}' is already installed.", channel);
         return Ok(());
     }
 
-    commit_version_install(downloaded, required_version, &mut config_file.data, paths)?;
-
-    config_file.data.installed_channels.insert(
-        channel.to_string(),
-        JuliaupConfigChannel::SystemChannel {
-            version: required_version.clone(),
-        },
-    );
-
     if config_file.data.default.is_none() {
         config_file.data.default = Some(channel.to_string());
     }
@@ -174,3 +211,146 @@ fn add_non_db(channel: &str, paths: &GlobalPaths) -> Result<()> {
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::config_file::JuliaupConfigVersion;
+    use std::path::Path;
+    use tempfile::Builder;
+
+    fn test_paths(dir: &Path) -> GlobalPaths {
+        GlobalPaths {
+            juliauphome: dir.to_path_buf(),
+            juliaupconfig: dir.join("juliaup.json"),
+            lockfile: dir.join(".juliaup-lock"),
+            versiondb: dir.join("versiondb-test.json"),
+            #[cfg(feature = "selfupdate")]
+            juliaupselfhome: dir.to_path_buf(),
+            #[cfg(feature = "selfupdate")]
+            juliaupselfconfig: dir.join("juliaupself.json"),
+            #[cfg(feature = "selfupdate")]
+            juliaupselfbin: dir.to_path_buf(),
+        }
+    }
+
+    fn downloaded_install(dir: &Path, marker: &str) -> Result<TempDir> {
+        let downloaded = Builder::new().prefix("julia-temp-").tempdir_in(dir)?;
+        std::fs::create_dir_all(downloaded.path().join("bin"))?;
+        std::fs::write(downloaded.path().join("bin/julia"), marker)?;
+        Ok(downloaded)
+    }
+
+    fn installed_version(path: &str) -> JuliaupConfigVersion {
+        JuliaupConfigVersion {
+            path: path.to_string(),
+            binary_path: None,
+        }
+    }
+
+    #[test]
+    fn concurrent_different_system_version_commits_download() -> Result<()> {
+        let dir = tempfile::tempdir()?;
+        let paths = test_paths(dir.path());
+        let downloaded = downloaded_install(dir.path(), "downloaded")?;
+        let mut config = JuliaupConfig::default();
+
+        config.installed_versions.insert(
+            "1.10.11+0.test".to_string(),
+            installed_version("./julia-1.10.11+0.test"),
+        );
+        config.installed_channels.insert(
+            "1.10".to_string(),
+            JuliaupConfigChannel::SystemChannel {
+                version: "1.10.11+0.test".to_string(),
+            },
+        );
+
+        let outcome =
+            commit_downloaded_channel("1.10", "1.10.12+0.test", downloaded, &mut config, &paths)?;
+
+        assert_eq!(outcome, AddChannelOutcome::Installed);
+        assert!(config.installed_versions.contains_key("1.10.12+0.test"));
+        assert!(matches!(
+            config.installed_channels.get("1.10"),
+            Some(JuliaupConfigChannel::SystemChannel { version })
+                if version == "1.10.12+0.test"
+        ));
+        assert_eq!(
+            std::fs::read_to_string(paths.juliauphome.join("julia-1.10.12+0.test/bin/julia"))?,
+            "downloaded"
+        );
+        Ok(())
+    }
+
+    #[test]
+    fn concurrent_same_system_version_discards_download() -> Result<()> {
+        let dir = tempfile::tempdir()?;
+        let paths = test_paths(dir.path());
+        let downloaded = downloaded_install(dir.path(), "duplicate")?;
+        let downloaded_path = downloaded.path().to_path_buf();
+        let target = paths.juliauphome.join("julia-1.10.12+0.test/bin");
+        std::fs::create_dir_all(&target)?;
+        std::fs::write(target.join("julia"), "existing")?;
+
+        let mut config = JuliaupConfig::default();
+        config.installed_versions.insert(
+            "1.10.12+0.test".to_string(),
+            installed_version("./julia-1.10.12+0.test"),
+        );
+        config.installed_channels.insert(
+            "1.10".to_string(),
+            JuliaupConfigChannel::SystemChannel {
+                version: "1.10.12+0.test".to_string(),
+            },
+        );
+
+        let outcome =
+            commit_downloaded_channel("1.10", "1.10.12+0.test", downloaded, &mut config, &paths)?;
+
+        assert_eq!(outcome, AddChannelOutcome::AlreadyInstalled);
+        assert!(!downloaded_path.exists());
+        assert_eq!(std::fs::read_to_string(target.join("julia"))?, "existing");
+        Ok(())
+    }
+
+    fn assert_concurrent_explicit_channel_is_preserved(
+        explicit_channel: JuliaupConfigChannel,
+    ) -> Result<()> {
+        let dir = tempfile::tempdir()?;
+        let paths = test_paths(dir.path());
+        let mut config = JuliaupConfig::default();
+        config
+            .installed_channels
+            .insert("1.10".to_string(), explicit_channel.clone());
+        let downloaded = downloaded_install(dir.path(), "downloaded")?;
+        let downloaded_path = downloaded.path().to_path_buf();
+
+        let outcome =
+            commit_downloaded_channel("1.10", "1.10.12+0.test", downloaded, &mut config, &paths)?;
+
+        assert_eq!(outcome, AddChannelOutcome::AlreadyInstalled);
+        assert!(!downloaded_path.exists());
+        assert!(config.installed_channels.get("1.10") == Some(&explicit_channel));
+        assert!(!config.installed_versions.contains_key("1.10.12+0.test"));
+        Ok(())
+    }
+
+    #[test]
+    fn concurrent_explicit_channels_preserve_name_claim() -> Result<()> {
+        assert_concurrent_explicit_channel_is_preserved(JuliaupConfigChannel::LinkedChannel {
+            command: "/custom/julia".to_string(),
+            args: None,
+        })?;
+        assert_concurrent_explicit_channel_is_preserved(
+            JuliaupConfigChannel::DirectDownloadChannel {
+                path: "./julia-1.10".to_string(),
+                url: "https://example.com/julia.tar.gz".to_string(),
+                local_etag: "etag".to_string(),
+                server_etag: "etag".to_string(),
+                version: "1.10.99-DEV".to_string(),
+                binary_path: None,
+            },
+        )
+    }
+}