Skip to content

Commit

Permalink
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
Browse files Browse the repository at this point in the history
Only try to modify file times of a writable file

- First commit fixes a failure that I was running into locally trying to build a stage 1 `./x build library --stage 1` on windows due to trying to modify file times of a read-only file.
- Second commit introduces a `set_file_times` helper which opens a given path as a file in r+w mode and then sets file times. This should hopefully make setting file times less error prone, since trying to set file times on read-only file also happened in rust-lang#127850. (And apparently it only fails locally on Windows or something weird like that.)
  • Loading branch information
matthiaskrgr authored Aug 12, 2024
2 parents 4211fa8 + 13c36f1 commit dd3fe6a
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
4 changes: 1 addition & 3 deletions src/bootstrap/src/core/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,7 @@ download-rustc = false
let file_times = fs::FileTimes::new().set_accessed(now).set_modified(now);

let llvm_config = llvm_root.join("bin").join(exe("llvm-config", self.build));
let llvm_config_file = t!(File::options().write(true).open(llvm_config));

t!(llvm_config_file.set_times(file_times));
t!(crate::utils::helpers::set_file_times(llvm_config, file_times));

if self.should_fix_bins_and_dylibs() {
let llvm_lib = llvm_root.join("lib");
Expand Down
12 changes: 6 additions & 6 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ use crate::core::builder;
use crate::core::builder::{Builder, Kind};
use crate::core::config::{flags, DryRun, LldMode, LlvmLibunwind, Target, TargetSelection};
use crate::utils::exec::{command, BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode};
use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir};
use crate::utils::helpers::{
self, dir_is_empty, exe, libdir, mtime, output, set_file_times, symlink_dir,
};

mod core;
mod utils;
Expand Down Expand Up @@ -1777,9 +1779,8 @@ Executed at: {executed_at}"#,
}
}
if let Ok(()) = fs::hard_link(&src, dst) {
// Attempt to "easy copy" by creating a hard link
// (symlinks don't work on windows), but if that fails
// just fall back to a slow `copy` operation.
// Attempt to "easy copy" by creating a hard link (symlinks are priviledged on windows),
// but if that fails just fall back to a slow `copy` operation.
} else {
if let Err(e) = fs::copy(&src, dst) {
panic!("failed to copy `{}` to `{}`: {}", src.display(), dst.display(), e)
Expand All @@ -1790,8 +1791,7 @@ Executed at: {executed_at}"#,
.set_accessed(t!(metadata.accessed()))
.set_modified(t!(metadata.modified()));

let dst_file = t!(fs::File::open(dst));
t!(dst_file.set_times(file_times));
t!(set_file_times(dst, file_times));
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/src/utils/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,3 +544,9 @@ pub fn get_closest_merge_base_commit(

Ok(output_result(git.as_command_mut())?.trim().to_owned())
}

/// Sets the file times for a given file at `path`.
pub fn set_file_times<P: AsRef<Path>>(path: P, times: fs::FileTimes) -> io::Result<()> {
let f = fs::File::options().write(true).open(path)?;
f.set_times(times)
}
20 changes: 19 additions & 1 deletion src/bootstrap/src/utils/helpers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use std::io::Write;
use std::path::PathBuf;

use crate::utils::helpers::{
check_cfg_arg, extract_beta_rev, hex_encode, make, program_out_of_date, symlink_dir,
check_cfg_arg, extract_beta_rev, hex_encode, make, program_out_of_date, set_file_times,
symlink_dir,
};
use crate::Config;

Expand Down Expand Up @@ -90,3 +91,20 @@ fn test_symlink_dir() {
#[cfg(not(windows))]
fs::remove_file(link_path).unwrap();
}

#[test]
fn test_set_file_times_sanity_check() {
let config = Config::parse(&["check".to_owned(), "--config=/does/not/exist".to_owned()]);
let tempfile = config.tempdir().join(".tmp-file");

{
File::create(&tempfile).unwrap().write_all(b"dummy value").unwrap();
assert!(tempfile.exists());
}

// This might only fail on Windows (if file is default read-only then we try to modify file
// times).
let now = std::time::SystemTime::now();
let file_times = fs::FileTimes::new().set_accessed(now).set_modified(now);
set_file_times(&tempfile, file_times).unwrap();
}

0 comments on commit dd3fe6a

Please sign in to comment.