Skip to content

dockerTools: use closureInfo to load hashes and closure sizes into the Nix DB#46592

Closed
dingxiangfei2009 wants to merge 3 commits intoNixOS:masterfrom
dingxiangfei2009:closure-info-dockertools
Closed

dockerTools: use closureInfo to load hashes and closure sizes into the Nix DB#46592
dingxiangfei2009 wants to merge 3 commits intoNixOS:masterfrom
dingxiangfei2009:closure-info-dockertools

Conversation

@dingxiangfei2009
Copy link
Contributor

@dingxiangfei2009 dingxiangfei2009 commented Sep 13, 2018

Motivation for this change

Since closureInfo is a good extension of exportReferencesGraph, we can use it to bootstrap Nix DB with this and possibly eliminates the need to recompute the hashes.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Sep 13, 2018
@dingxiangfei2009
Copy link
Contributor Author

I just realized that closureInfo is only available for nix language version at least 5.

@nlewo
Copy link
Member

nlewo commented Sep 13, 2018

@dingxiangfei2009 I think you could also have a look at #39716.

@dingxiangfei2009
Copy link
Contributor Author

dingxiangfei2009 commented Oct 29, 2018

@grahamc @nlewo When are we moving towards 2.0?

Also, if this PR is going to break some build, can we have a statistics on how many users are affected? If the proportion is low, we can still move to 2.0 while the affected users can pin their nixpkgs to the older revisions.

Notice that dockerTools is using the new nix toolchain anyway, switching from nixUnstable to nix which is 2.0+ at this point of time.

@nlewo
Copy link
Member

nlewo commented Oct 29, 2018

I close this PR in favor of #49414.
Note we can now rely on Nix2.0 because it is the default since 2 releases.

@nlewo nlewo closed this Oct 29, 2018
@nlewo
Copy link
Member

nlewo commented Oct 29, 2018

@dingxiangfei2009 If we consider we have to use Nix 2.0, I don't think these PRs will break any builds. But... maybe you have an example? :/

@dingxiangfei2009
Copy link
Contributor Author

dingxiangfei2009 commented Oct 30, 2018

@nlewo That is what I was thinking. I do not have any statistics on other users of this function, but I do not see any builds being re-triggered by this change according to the CI here. I would guess that there are no builds to be broken at all. :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants