Remove lock during downloads

src/command_add.rs

@@ -1,8 +1,10 @@
-use crate::config_file::{load_mut_config_db, save_config_db, JuliaupConfigChannel};
+use std::path::PathBuf;
+
+use crate::config_file::{load_mut_config_db, save_config_db, JuliaupConfigChannel, load_config_db, JuliaupConfigVersion};
 use crate::global_paths::GlobalPaths;
 #[cfg(not(windows))]
 use crate::operations::create_symlink;
-use crate::operations::{identify_nightly, install_nightly, install_version, update_version_db};
+use crate::operations::{identify_nightly, install_nightly, download_version_to_temp_folder, update_version_db};
 use crate::versions_file::load_versions_db;
 use anyhow::{anyhow, Context, Result};
 
@@ -26,23 +28,71 @@
  })?
  .version;
 
+ let we_need_to_download: bool;
+
+ {
+ let config_file = load_config_db(paths)
+ .with_context(|| "`add` command failed to load configuration data.")?;
+
+ if config_file.data.installed_channels.contains_key(channel) {
+ bail!("'{}' is already installed.", &channel);
+ }
+
+ we_need_to_download = !config_file.data.installed_versions.contains_key(required_version);
+ }
+
+ let mut temp_version_folder: Option<tempfile::TempDir> = None;
+
+ // Only download a new version if it isn't on the system already
+ if we_need_to_download {
+ temp_version_folder = Some(download_version_to_temp_folder(required_version, &version_db, paths)?);
+ }
+
  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 the version was added to the db while we downloaded it, we don't do anything. The temporary folder
+ // for our download should be auto-deleted when the variable goes out of scope.
+ if !config_file.data.installed_versions.contains_key(required_version) {
+ // This is a very specific corner case: it means that between releasing the read lock and acquiring
+ // the write lock the version was removed. In that case we re-download it while holding the write
+ // lock. That is not ideal, but should be rare enough and guarantees success.
+ if temp_version_folder.is_none() {
+ temp_version_folder = Some(download_version_to_temp_folder(required_version, &version_db, paths)?);
+ }
+
+ let child_target_foldername = format!("julia-{}", required_version);
+ let target_path = paths.juliauphome.join(&child_target_foldername);
+ let temp_dir = temp_version_folder.unwrap();
+ let source_path = temp_dir.path();
+ std::fs::rename(&source_path, &target_path)
+ .with_context(|| format!("Failed to rename temporary Julia download '{:?}' to '{:?}'", source_path, target_path))?;
+
+ let mut rel_path = PathBuf::new();
+ rel_path.push(".");
+ rel_path.push(&child_target_foldername);
+
+ config_file.data.installed_versions.insert(
+ required_version.clone(),
+ JuliaupConfigVersion {
+ path: rel_path.to_string_lossy().into_owned(),
+ },
+ );
+ }
+
+ if !config_file.data.installed_channels.contains_key(channel) {
+ config_file.data.installed_channels.insert(
+ channel.to_string(),
+ JuliaupConfigChannel::SystemChannel {
+ version: required_version.clone(),
+ },
+ ); 
+ }
+ else {
  eprintln!("'{}' is already installed.", &channel);
  return Ok(());
  }
 
- install_version(required_version, &mut config_file.data, &version_db, 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());
  }

src/command_update.rs

@@ -1,11 +1,11 @@
-use crate::config_file::JuliaupConfig;
+use crate::config_file::{JuliaupConfig, load_config_db};
 use crate::config_file::{load_mut_config_db, save_config_db, JuliaupConfigChannel};
 use crate::global_paths::GlobalPaths;
 use crate::jsonstructs_versionsdb::JuliaupVersionDB;
 #[cfg(not(windows))]
 use crate::operations::create_symlink;
 use crate::operations::{garbage_collect_versions, install_from_url};
-use crate::operations::{install_version, update_version_db};
+use crate::operations::{download_version_to_temp_folder, update_version_db};
 use crate::versions_file::load_versions_db;
 use anyhow::{anyhow, bail, Context, Result};
 use std::path::PathBuf;
@@ -26,7 +26,7 @@
 
  if let Some(should_version) = should_version {
  if &should_version.version != version {
- install_version(&should_version.version, config_db, version_db, paths)
+ download_version_to_temp_folder(&should_version.version, config_db, version_db, paths)
  .with_context(|| {
  format!(
  "Failed to install '{}' while updating channel '{}'.",
@@ -117,30 +117,63 @@
  let version_db =
  load_versions_db(paths).with_context(|| "`update` command failed to load versions db.")?;
 
- let mut config_file = load_mut_config_db(paths)
- .with_context(|| "`update` command failed to load configuration data.")?;
-
- match channel {
- None => {
- for (k, _) in config_file.data.installed_channels.clone() {
- update_channel(&mut config_file.data, &k, &version_db, true, paths)?;
+ let channels_to_update;
+
+ {
+ let config_file = load_config_db(paths)
+ .with_context(|| "`update` command failed to load configuration data.")?;
+
+
+
+ channels_to_update = match channel {
+ None => {
+ config_file
+ .data
+ .installed_channels 
+ .iter()
+ .filter_map(|i| {
+ match i.1 {
+ JuliaupConfigChannel::SystemChannel {version } => Some((i.0, version)),
+ JuliaupConfigChannel::LinkedChannel { command, args } => None,
+ }
+ })
+ .collect::<Vec<_>>()
+
+ // for (k, _) in config_file.data.installed_channels.clone() {
+ // update_channel(&mut config_file.data, &k, &version_db, true, paths)?;
+ // }
  }
- }
- Some(channel) => {
- if !config_file.data.installed_channels.contains_key(&channel) {
- bail!(
- "'{}' cannot be updated because it is currently not installed.",
- channel
- );
+ Some(channel) => {
+ if !config_file.data.installed_channels.contains_key(&channel) {
+ bail!(
+ "'{}' cannot be updated because it is currently not installed.",
+ channel
+ );
+ }
+
+ config_file
+ .data
+ .installed_channels 
+ .iter()
+ .filter(|i| i.0==&channel)
+ .filter_map(|i| {
+ match i.1 {
+ JuliaupConfigChannel::SystemChannel {version } => Some((i.0, version)),
+ JuliaupConfigChannel::LinkedChannel { command, args } => None,
+ }
+ })
+ .collect::<Vec<_>>()
  }
+ };
+ }
 
-  update_channel(&mut config_file.data, &channel, &version_db, false, paths)?;
- }
- };
+ for (k,v) in channels_to_update {
+ update_channel(&mut config_file.data, &channel, &version_db, false, paths)?;
+ }
 
  garbage_collect_versions(&mut config_file.data, paths)?;
 
  save_config_db(&mut config_file)
  .with_context(|| "`update` command failed to save configuration db.")?;
 
  Ok(())

src/operations.rs

@@ -2,7 +2,6 @@ use crate::config_file::load_mut_config_db;
 use crate::config_file::save_config_db;
 use crate::config_file::JuliaupConfig;
 use crate::config_file::JuliaupConfigChannel;
-use crate::config_file::JuliaupConfigVersion;
 use crate::get_bundled_dbversion;
 use crate::get_bundled_julia_version;
 use crate::get_juliaup_target;
@@ -278,17 +277,11 @@ pub fn download_versiondb(url: &str, path: &Path) -> Result<()> {
  Ok(())
 }
 
-pub fn install_version(
+pub fn download_version_to_temp_folder(
  fullversion: &String,
- config_data: &mut JuliaupConfig,
  version_db: &JuliaupVersionDB,
  paths: &GlobalPaths,
-) -> Result<()> {
- // Return immediately if the version is already installed.
- if config_data.installed_versions.contains_key(fullversion) {
- return Ok(());
- }
-
+) -> Result<tempfile::TempDir> {
  // TODO At some point we could put this behind a conditional compile, we know
  // that we don't ship a bundled version for some platforms.
  let full_version_string_of_bundled_version = get_bundled_julia_version();
@@ -298,15 +291,16 @@ pub fn install_version(
  .unwrap() // unwrap OK because we can't get a path that does not have a parent
  .join("BundledJulia");
 
- let child_target_foldername = format!("julia-{}", fullversion);
- let target_path = paths.juliauphome.join(&child_target_foldername);
- std::fs::create_dir_all(target_path.parent().unwrap())?;
+ let target_path = tempfile::Builder::new()
+ .prefix("tmp-")
+ .rand_bytes(6)
+ .tempdir_in(&paths.juliauphome)?;
 
  if fullversion == full_version_string_of_bundled_version && path_of_bundled_version.exists() {
  let mut options = fs_extra::dir::CopyOptions::new();
  options.overwrite = true;
  options.content_only = true;
- fs_extra::dir::copy(path_of_bundled_version, target_path, &options)?;
+ fs_extra::dir::copy(path_of_bundled_version, &target_path.path(), &options)?;
  } else {
  let juliaupserver_base =
  get_juliaserver_base_url().with_context(|| "Failed to get Juliaup server base URL.")?;
@@ -337,21 +331,10 @@ pub fn install_version(
  fullversion
  );
 
- download_extract_sans_parent(download_url.as_ref(), &target_path, 1)?;
- }
-
- let mut rel_path = PathBuf::new();
- rel_path.push(".");
- rel_path.push(&child_target_foldername);
+ download_extract_sans_parent(download_url.as_ref(), &target_path.path(), 1)?;
+ } 
 
- config_data.installed_versions.insert(
- fullversion.clone(),
- JuliaupConfigVersion {
- path: rel_path.to_string_lossy().into_owned(),
- },
- );
-
- Ok(())
+ Ok(target_path)
 }
 
 // which nightly arch to default to when simply using the `nightly` channel