[staging] stdenv: trim random seed to avoid reference cycles #106954
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofborg
bot
added
10.rebuild-darwin-stdenv
|
cc @andir would appreciate your thoughts on this - hoping to get this fixed soon since it is breaking a good number of cross packages |
andir
reviewed
Dec 23, 2020
| @@ -1,4 +1,7 @@ | |||
| # Use the last part of the out path as hash input for the build. | |||
| # This should ensure that it is deterministic across rebuilds of the same | |||
| # derivation and not easily collide with other builds. | |||
| export NIX_CFLAGS_COMPILE+=" -frandom-seed=${out##*/}" | |||
| # We also truncate the hash so that it cannot cause reference cycles. | |||
| local outbase="${out##*/}" | |||
There was a problem hiding this comment.
Does local really just span this file? Setup hooks are sourced via stdenv, right?
There was a problem hiding this comment.
Ah I think you are right. How about putting this in a getRandomSeed function? I can't just inline the basename + trimming because bash won't do the nested expansion
There was a problem hiding this comment.
Ok, I ended up using a command substitution. Looks a little odd but avoids any namespace pollution
|
On 13:24 23.12.20, r-burns wrote:
@r-burns commented on this pull request.
> @@ -1,4 +1,7 @@
# Use the last part of the out path as hash input for the build.
# This should ensure that it is deterministic across rebuilds of the same
# derivation and not easily collide with other builds.
-export NIX_CFLAGS_COMPILE+=" -frandom-seed=${out##*/}"
+# We also truncate the hash so that it cannot cause reference cycles.
+local outbase="${out##*/}"
+local randomseed="${outbase:0:10}"
I was a little concerned that the seed is the same for all files, but gcc recommends each file has a unique seed. Is it important for the seed to differ for each build too?
Our initial assesment was that it is enough if it differs per build and
that is exactly what the drv hash gives us.
|
Using the full store hash as the random seed occasionally caused reference cycles when the invocation was stored in output artifacts. For example, cross-compiled gcc was failing due to this: https://hydra.nixos.org/eval/1631713#tabs-now-fail Simply truncating the hash is sufficient to avoid this.
r-burns
changed the title
|
cc @dezgeg as you are the maintainer for release-cross.nix bootstrapTools |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using the full store hash as the random seed occasionally caused
reference cycles when the compiler invocation was stored in output artifacts.
For example, cross-compiled gcc was failing due to this:
https://hydra.nixos.org/eval/1631713#tabs-now-fail
Simply truncating the hash is sufficient to avoid this.
cc @symphorien: #102251 (comment)
Motivation for this change
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)