Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nix-prefetch-git: Remove some pack file non-determinism #67805

Merged
merged 1 commit into from Sep 6, 2019
Merged

nix-prefetch-git: Remove some pack file non-determinism #67805

merged 1 commit into from Sep 6, 2019

Conversation

infinisil
Copy link
Member

Motivation for this change

While packaging invidious in #67664, I found a piece of non-determinism in git, which nix-prefetch-git didn't account for. This PR works around this.

Ping @jonringer @bjornfor

Things done

No hashes needed updating with this change. By default the .git directory is removed, only when leaveDotGit = true; the function for making it deterministic is called. I applied this little patch:

diff --git a/pkgs/build-support/fetchgit/default.nix b/pkgs/build-support/fetchgit/default.nix
index 256c86748d2..0d984b8d5de 100644
--- a/pkgs/build-support/fetchgit/default.nix
+++ b/pkgs/build-support/fetchgit/default.nix
@@ -49,7 +49,7 @@ assert deepClone -> leaveDotGit;
 if md5 != "" then
   throw "fetchgit does not support md5 anymore, please use sha256"
 else
-stdenvNoCC.mkDerivation {
+let drv = stdenvNoCC.mkDerivation {
   inherit name;
   builder = ./builder.sh;
   fetcher = "${./nix-prefetch-git}";  # This must be a string to ensure it's called with bash.
@@ -68,4 +68,4 @@ stdenvNoCC.mkDerivation {
   ];
 
   inherit preferLocalBuild;
-}
+}; in if leaveDotGit then builtins.trace drv.drvPath drv else drv

Then I grepped for leaveDotGit and deepClone (which turns on leaveDotGit) to find potential packages. Each of those packages I instantiated with

nix-instantiate -A <attribute>

so the patch would trace the .drvs that have leaveDotGit = true;, then I did

nix-store -r <.drv>
nix-store -r <.drv> --check

on each of them to verify the hash is still correct.

@infinisil infinisil mentioned this pull request Aug 31, 2019
Closed
2 tasks
infinisil
infinisil commented Aug 31, 2019
View reviewed changes
pkgs/build-support/fetchgit/nix-prefetch-git
git gc --prune=all
# Note: --keep-largest-pack prevents non-deterministic ordering of packs
# listed in .git/objects/info/packs by only using a single pack
git gc --prune=all --keep-largest-pack
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative suggested in #git was to just remove .git/objects/info/packs because this file seems to be only used for dumb http serves or so. That solution however would force an update to all existing hashes (with keepDotGit = true), so I didn't do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, that sounds like the more correct solution for removing non-determinism. Maybe do a PR against staging?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the problem isn't only hashes in nixpkgs, but also hashes for users outside nixpkgs, which is probably a whole lot more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workaround will only affect derivations which were nondeterministic before

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You seem to be more confident in this area than I am. Maybe someone else will have a more informed opinion than me :)

@infinisil
Copy link
Member Author

Unless somebody is quicker or has feedback, I'll merge this soon

@infinisil infinisil merged commit da72765 into NixOS:master Sep 6, 2019
@infinisil infinisil deleted the more-git-determinism branch September 6, 2019 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer left review comments

Assignees
No one assigned
Labels
6.topic: fetch 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@infinisil @jonringer