http_client: Fix GitHub download unpack failures on some filesystems (#53286)

Smit Barmase created 4 days ago

Disable mtime preservation when unpacking tar archives, as some
filesystems error when asked to set it. Follows how
[cargo](https://github.com/rust-lang/cargo/blob/1ad92f77a819953bcef75a24019b66681ff28b1c/src/cargo/ops/cargo_package/verify.rs#L59
) and
[uv](https://github.com/astral-sh/uv/blob/0da0cd8b4310d3ac4be96223bd1e24ada109af9e/crates/uv-extract/src/stream.rs#L658)
handle it.

> Caused by:
    0: extracting
https://github.com/microsoft/vscode-eslint/archive/refs/tags/release%2F3.0.24.tar.gz
to "/Users/user-name-here/Library/Application
Support/Zed/languages/eslint/.tmp-github-download-pYkrYP"
    1: failed to unpack `/Users/user-name-here/Library/Application
Support/Zed/languages/eslint/.tmp-github-download-pYkrYP/vscode-eslint-release-3.0.24/package-lock.json`
    2: failed to set mtime for
`/Users/user-name-here/Library/Application
Support/Zed/languages/eslint/.tmp-github-download-pYkrYP/vscode-eslint-release-3.0.24/package-lock.json`
    3: No such file or directory (os error 2)

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- N/A

Change summary

crates/http_client/src/github_download.rs | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)

Detailed changes

crates/http_client/src/github_download.rs

@@ -207,11 +207,7 @@ async fn extract_tar_gz(
     from: impl AsyncRead + Unpin,
 ) -> Result<(), anyhow::Error> {
     let decompressed_bytes = GzipDecoder::new(BufReader::new(from));
-    let archive = async_tar::Archive::new(decompressed_bytes);
-    archive
-        .unpack(&destination_path)
-        .await
-        .with_context(|| format!("extracting {url} to {destination_path:?}"))?;
+    unpack_tar_archive(destination_path, url, decompressed_bytes).await?;
     Ok(())
 }
 
@@ -221,7 +217,21 @@ async fn extract_tar_bz2(
     from: impl AsyncRead + Unpin,
 ) -> Result<(), anyhow::Error> {
     let decompressed_bytes = BzDecoder::new(BufReader::new(from));
-    let archive = async_tar::Archive::new(decompressed_bytes);
+    unpack_tar_archive(destination_path, url, decompressed_bytes).await?;
+    Ok(())
+}
+
+async fn unpack_tar_archive(
+    destination_path: &Path,
+    url: &str,
+    archive_bytes: impl AsyncRead + Unpin,
+) -> Result<(), anyhow::Error> {
+    // We don't need to set the modified time. It's irrelevant to downloaded
+    // archive verification, and some filesystems return errors when asked to
+    // apply it after extraction.
+    let archive = async_tar::ArchiveBuilder::new(archive_bytes)
+        .set_preserve_mtime(false)
+        .build();
     archive
         .unpack(&destination_path)
         .await