Merge pull request #5975 from roc-lang/fix-exists

Make downloading packages more resilient
This commit is contained in:
Ayaz 2023-11-12 21:02:11 -06:00 committed by GitHub
commit 1c1231bc11
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -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")]