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.

Sign up for GitHub

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

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

Closed
wants to merge 3 commits into from
Closed

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

wants to merge 3 commits into from

Conversation

dingxiangfei2009
Copy link
Contributor

@dingxiangfei2009 dingxiangfei2009 commented Sep 13, 2018 •
edited

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.

@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 •
edited

@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 •
edited

@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
Reviewers
No reviews
Assignees
No one assigned
Labels
8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dingxiangfei2009 @nlewo @GrahamcOfBorg