Build system tweaks for reproducible builds#447
Build system tweaks for reproducible builds#447threema-danilo wants to merge 3 commits intoranisalt:masterfrom
Conversation
| const env = { | ||
| ...process.env, | ||
| // Avoid embedding timestamps into linked binary on macOS | ||
| ZERO_AR_DATE: "1", |
There was a problem hiding this comment.
Can we set this as an environment variable directly in bindings.gyp? I would rather not involve another script if it just changes the environment like this.
Also, is this something we want for all builds, or is reproducible builds something for specific scenarios?
There was a problem hiding this comment.
If you find a way to do it in a cross-platform way, I'd be happy to change it! However, setting env variables on Linux/macOS is with ZERO_AR_DATE=1, and on Windows it's $env:ZERO_AR_DATE=1 (at least in PowerShell, I'm not sure about CMD). I'm not aware of a cross-platform syntax unfortunately.
Also, is this something we want for all builds, or is reproducible builds something for specific scenarios?
I don't see any benefit in having timestamps embedded in the binaries. As such, I'd always set this variable (unless there's a scenario where embedded timestamps would be needed).
There was a problem hiding this comment.
Would it work if it was set to the GitHub workflow step environment? That would be available on build when released, but local builds will still contain timestamps unless the user also sets ZERO_AR_DATE=1
Alternatively, I believe that npm scripts are portable and setting the build script to
"install": "ZERO_AR_DATE=1 node-gyp-build",
works (at least it works with yarn)
(sorry for the late reply)
There was a problem hiding this comment.
Would it work if it was set to the GitHub workflow step environment? That would be available on build when released, but local builds will still contain timestamps unless the user also sets ZERO_AR_DATE=1
In our case we have node-argon2 as a dependency (pulled in through the source code, not npm builds), and we want to build our product reproducibly. So setting a GitHub workflow env variable wouldn't work.
Alternatively, I believe that npm scripts are portable and setting the build script to
That's a misunderstanding, the npm scripts will run in your local shell environment. So this will work on Linux and macOS, but not on Windows.
The portable approach is having a .js file called through the npm script.
There was a problem hiding this comment.
Alright, in that case I think it would be better to use cross-env which does the trick for all platforms and it's quite a small dependency.
"install": "cross-env ZERO_AR_DATE=1 node-gyp-build",
and add it to the dependencies and I'll merge it if that's OK :)
There was a problem hiding this comment.
TIL about cross-env, that package looks nice! Yes, feel free to update my branch as desired 🙂
a14237b to
6b4f234
Compare
binding.gyp
Outdated
| "defines+": ["_HAS_EXCEPTIONS=1"], | ||
| "msvs_settings": { | ||
| "VCCLCompilerTool": { "ExceptionHandling": 1 } | ||
| "VCCLCompilerTool": {"ExceptionHandling": 1}, |
There was a problem hiding this comment.
Keep unchanged
| "VCCLCompilerTool": {"ExceptionHandling": 1}, | |
| "VCCLCompilerTool": { "ExceptionHandling": 1 }, |
There was a problem hiding this comment.
The line will be modified in any case due to the trailing comma, but I updated the commit to remove the whitespace change.
(I also rebased against current master.)
- The /Brepro flag is undocumented by Microsoft, but can be found on the internet¹ and it removes timestamps from the resulting binary. I tested that it works as intended - timestamps are gone. - Omit debug info to avoid mismatching paths or RSDS signature - Disable incremental building (since it's a potential source of non-determinism) ¹ https://nikhilism.com/post/2020/windows-deterministic-builds/#fix-dates-and-times-in-portable-executables
...to achieve local determinism.
Previously the file system timestamp of the object file (`argon2.o`) would influence the output binary. The env variable ZERO_AR_DATE=1 avoids this.
6b4f234 to
e6ccb10
Compare
|
I didn't have permission to push to your branch, so I merged it through the command line 0d88d0d Waiting for CI to pass to release v0.44 with the changes :) |
Hello, we have spent some time to make builds of this library reproducible (at least to a certain degree, we have not yet finished testing and it's possible that we missed some edge cases).
Is this something that you're interested in integrating into your upstream project?
In case of questions, I'm happy to help.