Skip to content

Commit 701217d

Browse files
authored
fix: rename temp downloaded file to given cache path (#332)
resolves #330 This simply keeps the downloaded temp file in the same directory as the destination of the downloaded file. When download is done, the file is renamed to the specified file name. This also removes the `tempfile` dependency from production code. It is still used heavily in tests. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved error handling for download operations with clearer, contextual messages when temporary file or filesystem operations fail. * **Breaking Changes** * The shape of download error reporting was changed; integrations that inspect specific download error variants may need updates. * **Chores** * Test/dev setup updated (added a dev dependency for temporary-file utilities) and CI triggers expanded to include installer-related files and manifests. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 8e2fcad commit 701217d

3 files changed

Lines changed: 19 additions & 7 deletions

File tree

.github/workflows/run-dev-tests.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@ on:
55
branches: [main]
66
paths:
77
- cpp-linter/**
8+
- clang-installer/**
89
- Cargo.toml
910
- Cargo.lock
1011
- .github/workflows/run-dev-tests.yml
1112
pull_request:
1213
branches: [main]
1314
paths:
1415
- cpp-linter/**
16+
- clang-installer/**
1517
- Cargo.toml
1618
- Cargo.lock
1719
- .github/workflows/run-dev-tests.yml

clang-installer/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ tokio = { workspace = true, features = ["macros"], optional = true }
3636
url = "2.5.8"
3737
which = "8.0.2"
3838
zip = { version = "8.6.0", default-features = false, features = ["deflate"] }
39-
tempfile = { workspace = true }
4039

4140
[dev-dependencies]
4241
colored = { workspace = true }
4342
mockito = { workspace = true }
4443
reqwest = { workspace = true, features = ["default-tls"] }
44+
tempfile = { workspace = true }
4545
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
4646

4747
[features]

clang-installer/src/downloader/mod.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ pub enum DownloadError {
3131
#[error("Hash mismatch for downloaded file. Expected: {expected}, Actual: {actual}")]
3232
HashMismatch { expected: String, actual: String },
3333

34-
/// An error that occurred while moving/persisting a temporary file into a cache path.
35-
#[error("Error persisting temporary file: {0}")]
36-
TempFilePersistence(#[from] tempfile::PersistError),
34+
/// An error that occurred while creating or persisting a temporary file into a cache path.
35+
#[error("Error when {0}: {1}")]
36+
TempFile(&'static str, #[source] std::io::Error),
3737
}
3838

3939
/// Downloads data from the specified URL and returns the response.
@@ -58,7 +58,15 @@ async fn download(url: &Url, cache_path: &Path, timeout: u64) -> Result<(), Down
5858
}
5959
return Err(e.into());
6060
}
61-
let mut tmp_file = tempfile::NamedTempFile::new()?;
61+
let tmp_file_path = cache_path.with_extension("tmp");
62+
let mut tmp_file = fs::OpenOptions::new()
63+
.write(true)
64+
.truncate(true)
65+
.create(true)
66+
.open(&tmp_file_path)
67+
.map_err(|e| {
68+
DownloadError::TempFile("creating temporary file for download placeholder", e)
69+
})?;
6270
let content_len = response.content_length().and_then(NonZero::new);
6371
let mut progress_bar = ProgressBar::new(
6472
content_len,
@@ -79,8 +87,10 @@ async fn download(url: &Url, cache_path: &Path, timeout: u64) -> Result<(), Down
7987
}
8088
progress_bar.finish()?;
8189
tmp_file.flush()?;
82-
tmp_file.as_file_mut().set_modified(SystemTime::now())?;
83-
tmp_file.persist(cache_path)?;
90+
tmp_file.set_modified(SystemTime::now())?;
91+
drop(tmp_file); // ensure the file is closed before renaming
92+
fs::rename(tmp_file_path, cache_path)
93+
.map_err(|e| DownloadError::TempFile("renaming temporary file after download", e))?;
8494
Ok(())
8595
}
8696

0 commit comments

Comments
 (0)