Correctly set filetime for copied LLVM #57871
Conversation
|
cc @eddyb |
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/tools/compiletest/src/main.rs
Outdated
| })); | ||
|
|
||
| // Compiletest itself. | ||
| inputs.extend(collect_timestamps(&rust_src_dir.join("src/tools/compiletest/"))); | ||
| //inputs.extend(Stamp::from_dir(&rust_src_dir.join("src/tools/compiletest/"))); |
There was a problem hiding this comment.
some leftover commented out code
|
I don't really know this code in particular. r? @oli-obk since they left a review already =) |
This also makes compiletest no longer always retest everything.
Mark-Simulacrum
force-pushed
the
fix-compiletest-stamp
branch
from
690148d to
82fae2b
Compare
|
@oli-obk Fixed up the code. FWIW the compiletest changes could be dropped entirely (they're mostly to ease debugging): none of them are actually necessary to fix the bug, but I decided to leave them in to help future bug-hunting. |
| t!(fs::set_permissions(&dst, metadata.permissions())); | ||
| let atime = FileTime::from_last_access_time(&metadata); | ||
| let mtime = FileTime::from_last_modification_time(&metadata); | ||
| t!(filetime::set_file_times(&dst, atime, mtime)); |
There was a problem hiding this comment.
So... if I'm seeing this right, the only actual change of this PR is this file.
Let me see if I get this right:
The old code copied e.g. rustc over from where it was built in stage 1 to where it is used to build the tests. This copy operation nuked all metadata and timestamps, thus causing compiletest to think rustc changed and causing a full rebuild.
The new code instead reads the metadata, copies the file and writes back the relevant old metadata. So even if there's a new file, its timestamp didn't change.
If I got everything right, please add a few comments explaining why we don't just do fs::copy and be done with it. r=me with that
There was a problem hiding this comment.
As far as I can tell, fs::copy doesn't copy the timestamp (just the permission bits); this code is copied from the copy method on Build (in this file) anyway, so I'd prefer to not modify it in this PR.
There was a problem hiding this comment.
To clarify, the reason we don't call that copy method instead is that it prefers to create a hardlink instead of copying if it can, and we don't want that here.
|
@bors r+ |
|
📌 Commit 82fae2b has been approved by |
bors
added
S-waiting-on-bors
|
Adjusting queue order for rollup, @bors p=1 |
|
⌛ Testing commit 82fae2b with merge 30d2b0866bf8ac48cb3e429ddd4c21c4eb050498... |
|
@bors retry Prioritizing bootstrap update... |
Correctly set filetime for copied LLVM This also makes compiletest no longer always retest everything. Fixes #57864
|
☀️ Test successful - checks-travis, status-appveyor |
|
☀️ Test successful - checks-travis, status-appveyor |
This also makes compiletest no longer always retest everything.
Fixes #57864