diff --git a/crates/packaging/src/cache.rs b/crates/packaging/src/cache.rs index bafd1bd8a18..90af81c9ba3 100644 --- a/crates/packaging/src/cache.rs +++ b/crates/packaging/src/cache.rs @@ -70,6 +70,8 @@ pub fn install_package<'a>( roc_cache_dir: RocCacheDir<'_>, url: &'a str, ) -> Result<(PathBuf, Option<&'a str>), Problem> { + use std::io::ErrorKind; + let PackageMetadata { cache_subdir, content_hash, @@ -110,10 +112,24 @@ pub fn install_package<'a>( // Now that we've verified the hash, rename the tempdir to the real dir. // Create the destination dir's parent dir, since it may not exist yet. - fs::create_dir_all(parent_dir).map_err(Problem::IoErr)?; + fs::create_dir_all(parent_dir).or_else(|err| match err.kind() { + // It's fine if the destination dir's parent already exists + ErrorKind::AlreadyExists => Ok(()), + _ => Err(Problem::IoErr(err)), + })?; // This rename should be super cheap if it succeeds - just an inode change. - if fs::rename(tempdir_path, &dest_dir).is_err() { + let rename_err_kind = fs::rename(tempdir_path, &dest_dir) + .err() + .map(|err| err.kind()); + + // It's okay if the rename failed because the destination already existed. + // This could be a race condition between multiple downloads happening concurrently. + // (This has happened in our test suite, for example!) Both downloads should have + // the same content, so the rename failing for that reason should be no problem. + if rename_err_kind.is_some() + && rename_err_kind != Some(ErrorKind::AlreadyExists) + { // If the rename failed, try a recursive copy - // it could have failed due to std::io::ErrorKind::CrossesDevices // (e.g. if the source an destination directories are on different disks) @@ -122,7 +138,12 @@ pub fn install_package<'a>( // but if that's what happened, this should work! // fs_extra::dir::copy needs the destination directory to exist already. - fs::create_dir(&dest_dir).map_err(Problem::IoErr)?; + fs::create_dir(&dest_dir).or_else(|err| match err.kind() { + // It's fine if the destination dir already exists + ErrorKind::AlreadyExists => Ok(()), + _ => Err(Problem::IoErr(err)), + })?; + fs_extra::dir::copy( tempdir_path, &dest_dir, @@ -131,7 +152,12 @@ pub fn install_package<'a>( ..Default::default() }, ) - .map_err(Problem::FsExtraErr)?; + .or_else(|err| match err.kind { + // It's fine if the destination file already exists; this could be the same + // as the rename race condition mentioned above. + fs_extra::error::ErrorKind::AlreadyExists => Ok(0), + _ => Err(Problem::FsExtraErr(err)), + })?; } #[cfg(target_os = "linux")]