Repository: typst/typst
File: crates/typst-kit/src/package.rs (Rust)

Code before:
                }
            }
            Err(err) => {
                return Err(PackageError::NetworkFailed(Some(eco_format!("{err}"))))
            }
        };

        // Temporary place where the package will be downloaded before being moved
        // to the target directory;
        let package_download_dir = download_dir.join(format!(
            "{}-{}-{}",
            spec.name,
            spec.version,
            std::process::id() // Make directory name unique.
        ));

        let create_dir = |dir| {
            fs::create_dir_all(dir).map_err(|err| {
                PackageError::Other(Some(eco_format!(
                    "failed to create download directory ({err})"
                )))
            })
        };

        create_dir(package_download_dir.as_path())?;
        create_dir(package_dir)?;

        let decompressed = flate2::read::GzDecoder::new(data.as_slice());
        tar::Archive::new(decompressed)
            .unpack(&package_download_dir)
            .map_err(|err| {
                fs::remove_dir_all(package_dir).ok();
                PackageError::MalformedArchive(Some(eco_format!("{err}")))
            })?;

        let removed_download_dir = || {
            fs::remove_dir_all(&package_download_dir).map_err(|err| {
                PackageError::Other(Some(eco_format!(
                    "failed to delete temporary package directory: {err}"
                )))
            })
        };

        // - Assuming both paths are from the same mount point.
        // - DirectoryNotEmpty error is assumed to happen only when another instance
        //   already moved the package to the destination directory, therefore it is
        //   safe to ignore it.
        match fs::rename(&package_download_dir, package_dir) {
            Ok(()) => Ok(()),
            Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {
                removed_download_dir()

Review comment (bug):
If this fails it will say `failed to create downlaod directory (...)`, which is not quite true, the closure should probably get this as an additional parameter.

Code after:

        // The directory in which the package's version lives.
        let base_dir = cache_dir.join(format!("{}/{}", spec.namespace, spec.name));

        // The place at which the specific package version will live in the end.
        let package_dir = base_dir.join(format!("{}", spec.version));

        // To prevent multiple Typst instances from interferring, we download
        // into a temporary directory first and then move this directory to
        // its final destination.
        //
        // In the `rename` function's documentation it is stated:
        // > This will not work if the new name is on a different mount point.
        //
        // By locating the temporary directory directly next to where the
        // package directory will live, we are (trying our best) making sure
        // that `tempdir` and `package_dir` are on the same mount point.
        let tempdir = Tempdir::create(base_dir.join(format!(
            ".tmp-{}-{}",
            spec.version,
            fastrand::u32(..),
        )))
        .map_err(|err| error("failed to create temporary package directory", err))?;

        // Decompress the archive into the temporary directory.
        let decompressed = flate2::read::GzDecoder::new(data.as_slice());
        tar::Archive::new(decompressed)
            .unpack(&tempdir)
            .map_err(|err| PackageError::MalformedArchive(Some(eco_format!("{err}"))))?;

        // When trying to move (i.e., `rename`) the directory from one place to
        // another and the target/destination directory is empty, then the
        // operation will succeed (if it's atomic, or hardware doesn't fail, or
        // power doesn't go off, etc.). If however the target directory is not
        // empty, i.e., another instance already successfully moved the package,
        // then we can safely ignore the `DirectoryNotEmpty` error.
        //
        // This means that we do not check the integrity of an existing moved
        // package, just like we don't check the integrity if the package
        // directory already existed in the first place. If situations with
        // broken packages still occur even with the rename safeguard, we might
        // consider more complex solutions like file locking or checksums.
        match fs::rename(&tempdir, &package_dir) {
            Ok(()) => Ok(()),
            Err(err) if err.kind() == io::ErrorKind::DirectoryNotEmpty => Ok(()),
            Err(err) => Err(error("failed to move downloaded package directory", err)),
        }
    }
}

/// Minimal information required about a package to determine its latest