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

Code before:
        let url = format!(
            "{DEFAULT_REGISTRY}/{DEFAULT_NAMESPACE}/{}-{}.tar.gz",
            spec.name, spec.version
        );

        let data = match self.downloader.download_with_progress(&url, progress) {
            Ok(data) => data,
            Err(ureq::Error::Status(404, _)) => {
                if let Ok(version) = self.determine_latest_version(&spec.versionless()) {
                    return Err(PackageError::VersionNotFound(spec.clone(), version));
                } else {
                    return Err(PackageError::NotFound(spec.clone()));
                }
            }
            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, dir_name| {
            fs::create_dir_all(dir).map_err(|err| {
                PackageError::Other(Some(eco_format!(
                    "failed to create a {dir_name} directory ({err})"
                )))
            })
        };

        create_dir(package_download_dir.as_path(), "download")?;
        create_dir(package_dir, "package")?;

        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}"

Review comment (suggestion):
Not a big fan of this, especially given we're in `typst-kit` and not `typst-cli`, so relying too much on the environment like this isn't particularly appropriate. Especially considering that process IDs can be reused, so in principle this directory name isn't actually unique and could already exist. At least, it could be interesting to allow `typst-kit` users to override this behavior depending on their environment's needs.

However, the ultimately more appropriate solution would be to use something like `tempdir()` to guarantee an absence of conflicts. But then it's the chances that they will be in the same volume or filesystem will be very low as pointed out by tinger. In NixOS, for example, they would still be in the same filesystem by default (`/tmp` is not cleaned up and is part of the system's FS in the default settings), but not in most Linux distros.

Code after:
            Ok(data) => data,
            Err(ureq::Error::Status(404, _)) => {
                if let Ok(version) = self.determine_latest_version(&spec.versionless()) {
                    return Err(PackageError::VersionNotFound(spec.clone(), version));
                } else {
                    return Err(PackageError::NotFound(spec.clone()));
                }
            }
            Err(err) => {
                return Err(PackageError::NetworkFailed(Some(eco_format!("{err}"))))
            }
        };

        // 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