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

minimal-bootstrap: make sources a non-tarballs.nixos.org FOD #238357

Merged
merged 3 commits into from Jun 26, 2023
Merged

minimal-bootstrap: make sources a non-tarballs.nixos.org FOD #238357

merged 3 commits into from Jun 26, 2023

Conversation

ghost
Copy link

@ghost ghost commented Jun 18, 2023

Alternative to #232576, does not require manual uploads to tarballs.nixos.org.

Description of changes

This commit adjusts #232576 to break the fetchurl<->minimal-bootstrap-sources dependency cycle without needing an upload to tarballs.nixos.org. It does this by appending the low-level FOD attributes onto the runCommand derivation.

https://nixos.org/manual/nix/unstable/language/advanced-attributes.html#adv-attr-outputHash

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ghost
Copy link
Author

ghost commented Jun 18, 2023

The gigantic comment should be moved into the manual once we switch over to using minimal-bootstrap as our source of bootstrapFiles.

Copy link
Member

@emilytrau emilytrau left a comment

Choose a reason for hiding this comment

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

This is awesome! Super creative way of solving the bootstrap sources issue! 😍

Have you got a method to validate that this won't have infinite recursion once we integrate into the bootstrap-tools dependency chain? I'm not sure how to test that

@emilytrau
Copy link
Member

@amjoseph-nixpkgs would you be interested in adding yourself the minimal-bootstrap maintainers team as well?

@ghost
Copy link
Author

ghost commented Jun 21, 2023

Squashed.

Have you got a method to validate that this won't have infinite recursion once we integrate into the bootstrap-tools dependency chain? I'm not sure how to test that

Yes, we can have a test which initializes a completely empty store and does a nix-instantiate, like this

@amjoseph-nixpkgs would you be interested in adding yourself the minimal-bootstrap maintainers team as well?

Sure; I probably need to add myself to maintainer-list.nix first though. Procrastination, procrastination...

@ghost ghost requested a review from emilytrau June 21, 2023 05:07
Copy link
Member

@emilytrau emilytrau left a comment

Choose a reason for hiding this comment

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

One last thing and very happy to go! 🚀🚀

specifically designed for bit-exact reproducibility, none of the
requirements above apply to `minimal-bootstrap-sources`.
*/
minimal-bootstrap-sources = make-minimal-bootstrap-sources.overrideAttrs(_: {
Copy link
Member

@Ericson2314 Ericson2314 Jun 24, 2023

Choose a reason for hiding this comment

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

This will eventually not work because the drv files are cyclic (cyclic hashing problem) even if the input-addressing isn't cyclic. But that's fine. We can use a stub failing FOD with the same hash at that point, just like the fetchers for proprietary things.

Copy link
Author

@ghost ghost Jun 26, 2023

Choose a reason for hiding this comment

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

You're absolutely right. We should just do it now and get it over with. @Ericson2314 @emilytrau, please take a look at c8080b0 which implements this. Here's what the failure looks like on the console:

bootfail2

Copy link
Author

Choose a reason for hiding this comment

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

I tested this by making bootstrapTools depend on minimal-bootstrap-tools as shown below, and eval succeeded. The build began, but obviously it won't complete.

diff --git a/pkgs/stdenv/linux/bootstrap-tools/default.nix b/pkgs/stdenv/linux/bootstrap-tools/default.nix
index 569f0c6f31e2..6aa8e6fbc6cf 100644
--- a/pkgs/stdenv/linux/bootstrap-tools/default.nix
+++ b/pkgs/stdenv/linux/bootstrap-tools/default.nix
@@ -3,7 +3,7 @@
 derivation ({
   name = "bootstrap-tools";

-  builder = bootstrapFiles.busybox;
+  builder = "/bin/sh";

   args = [ "ash" "-e" ./scripts/unpack-bootstrap-tools.sh ];

diff --git a/pkgs/stdenv/linux/default.nix b/pkgs/stdenv/linux/default.nix
index 9cfe21e3640d..e475787287d6 100644
--- a/pkgs/stdenv/linux/default.nix
+++ b/pkgs/stdenv/linux/default.nix
@@ -55,6 +55,8 @@
 #     $ nix-tree --derivation $(nix-instantiate -A stdenv)
 { lib
 , localSystem, crossSystem, config, overlays, crossOverlays ? []
+, minimal-bootstrap-tools ?
+  (import ../../os-specific/linux/minimal-bootstrap/stage0-posix/bootstrap-sources.nix {}).minimal-bootstrap-sources

 , bootstrapFiles ?
   let table = {
@@ -92,7 +94,10 @@
     or (abort "unsupported libc for the pure Linux stdenv");
   files = archLookupTable.${localSystem.system} or (if getCompatibleTools != null then getCompatibleTools
     else (abort "unsupported platform for the pure Linux stdenv"));
-  in files
+  in {
+    bootstrapTools = minimal-bootstrap-tools;
+    busybox = null;
+  }
 }:

 assert crossSystem == localSystem;

@emilytrau
Copy link
Member

Linking back to the thread #227914

This commit adjusts #232576 to break the
fetchurl<->minimal-bootstrap-sources dependency cycle without
needing an upload to tarballs.nixos.org.  It does this by appending
the low-level FOD attributes onto the `runCommand` derivation.

  https://nixos.org/manual/nix/unstable/language/advanced-attributes.html#adv-attr-outputHash
@ghost ghost requested review from Ericson2314 and emilytrau June 26, 2023 08:18
@ghost
Copy link
Author

ghost commented Jun 26, 2023

I squashed all of the suggested-changes.

c8080b0 has new content; please take a look. See also #238357 (comment)

Ericson2314 observed that although FODs break the build-time cycle,
they don't break the eval-time cycle.  This cycle must be broken in
order to compute the derivation hash.

make-minimal-bootstrap-sources does and always has depended on the
full nixpkgs (including fetchFromGitHub).  This commit completely
separates minimal-bootstrap-sources from it, so that
minimal-bootstrap-sources now does not depend on
make-minimal-bootstrap-sources (or nixpkgs) in any way.

minimal-bootstrap-sources is now a "bare Nix" derivation.  It is an
FOD whose builder always fails with an error message.
@ofborg ofborg bot requested a review from siraben June 26, 2023 08:37
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Ericson2314 Ericson2314 merged commit d409d42 into NixOS:master Jun 26, 2023
4 checks passed
@Ericson2314
Copy link
Member

Uh oh, did I merge to soon? the hex1 build seems to be spinning in a loop for me.

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

Successfully merging this pull request may close these issues.

2 participants