Use different hashing functions for different file types#197
Use different hashing functions for different file types#197metajack wants to merge 1 commit intomozilla:mainfrom
Conversation
|
r? @luser |
payload
left a comment
There was a problem hiding this comment.
Zeroing looks wrong. Please add tests.
src/util.rs
Outdated
| }, | ||
| Ok(()) => { | ||
| // timestamp | ||
| zero_slice(&mut buffer[16..28], 12); |
There was a problem hiding this comment.
I think you missed the first 16 bytes of ar_name here. You zero ar_name and parts of ar_date. uid and gid are left intact. See man ar(5)
This adds a new special hasher for static libraries, which contain timestamps and other info that prevent them from being cachable in some cases.
35ae228 to
1f8a95d
Compare
|
I did a bit of rework on this patch. It's passing tests now. I'd like to add a real-world cargo test for it alongside the existing cargo tests we have and I also need to verify that it works for windows-msvc, but otherwise I think this is pretty close to landable. |
|
I tested this on windows-msvc and it works (apparently MSVC .lib files are the same archive format, TIL), but there's a slight complication in that MSVC generates timestamps in object files unless you pass the undocumented |
|
What's left to land this then? |
|
I forget what I was hung up on with this. Maybe writing tests? We had disabled the cargo integration tests on Mac because they were flaky: sccache/tests/sccache_cargo.rs Line 21 in bf41588 ...and I think I wanted to fix that before landing this, since otherwise we wouldn't have test coverage of the changes on the platform where we actually care about them? In any event I'm no longer actively working on sccache. If you want to get this landed feel free to take my changes and work with @chmanchester to get it done. |
|
@chmanchester what's required to get this landed :) |
|
FYI the |
In order to avoid timestamp presence in static lib files, making them non-deterministic. alexcrichton/cc-rs@555e773 mozilla/sccache#197
In order to avoid timestamp presence in static lib files, making them non-deterministic. alexcrichton/cc-rs@555e773 mozilla/sccache#197
|
Hey Jack, this patch has withstanded the weather out here for quite some time, woah 😮 |
|
Going to mark this as |
|
Here is a fork that rebases the PR on top of the latest main. It compiles, but the |
|
|
|
I opened a separate PR in #1135 for my rebased version. |
|
Thanks metajack, and thanks FooBarWidget for rebasing this 👍 :) |
|
Thanks for picking this up! I can't believe this is 4.5 years old now. |
This adds a new special hasher for static libraries, which contain timestamps
and other info that prevent them from being cachable in some cases.