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: point bootstrap sources to tarballs.nixos.org #232576

Closed
wants to merge 1 commit into from

Conversation

emilytrau
Copy link
Member

Description of changes

The purpose of this PR is to move the initial source tarball to an official location as discussed in #227914 (comment). To facilitate I have exposed the packaging steps as the minimal-bootstrap-sources package.

Related #227914

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.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label May 18, 2023
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels May 18, 2023
@lovesegfault
Copy link
Member

Hi @emilytrau!

Thrilled to see this work progress :)

Could you take a look at the format used here to request tarball uploads? #183487

If you could make a comment in that shape, it'd help me a lot in uploading this quickly

Thanks!

@emilytrau
Copy link
Member Author

@lovesegfault thanks so much!

@lovesegfault lovesegfault requested review from trofi and a user May 18, 2023 11:32
@lovesegfault
Copy link
Member

(Might take a couple of days, I'm travelling and didn't bring my upload keys)

@ofborg ofborg bot requested a review from 06kellyjac May 18, 2023 12:22
@trofi trofi removed their request for review May 18, 2023 12:44
@ghost
Copy link

ghost commented Jun 8, 2023

(I am back and close to conquering my backlog)

Quick glance LGTM; will do a closer review this weekend if this hasn't been merged by then.

@Ericson2314
Copy link
Member

Is this undrafted because the tarballs are uploaded now?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This has to be done in two separately-merged PRs, with a Hydra build occurring after the first but before the second. See below. Yeah, it's annoying, I know, I went through it four or five times.

Comment on lines +27 to +41

#
# Files came from this Hydra build:
#
# https://hydra.nixos.org/build/<placeholder>
#
# Which used nixpkgs revision <placeholder>
# to instantiate:
#
# /nix/store/<placeholder>.drv
#
# and then built:
#
# /nix/store/<placeholder>
#
Copy link

Choose a reason for hiding this comment

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

Suggested change
#
# Files came from this Hydra build:
#
# https://hydra.nixos.org/build/<placeholder>
#
# Which used nixpkgs revision <placeholder>
# to instantiate:
#
# /nix/store/<placeholder>.drv
#
# and then built:
#
# /nix/store/<placeholder>
#

@@ -27102,6 +27102,7 @@ with pkgs;
inherit (stdenv) buildPlatform hostPlatform;
inherit lib config;
});
minimal-bootstrap-sources = callPackage ../os-specific/linux/minimal-bootstrap/stage0-posix/make-bootstrap-sources.nix { };
Copy link

Choose a reason for hiding this comment

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

There are no Hydra builds of minimal-bootstrap-sources yet.

In order to get a Hydra build you need to:

  1. merge a PR that has the line above
  2. wait for Hydra to build it
  3. then submit a separate PR which references that Hydra build.

The second PR is the one that should use the template @lovesegfault referenced.

Also, the provenance of builds on tarballs.nixos.org is a big deal. You should delete the fetchurl of https://github.com/emilytrau/bootstrap-tools-nar-mirror/ so it isn't anywhere in the nixpkgs repo at the commit from which Hydra builds the narball. Otherwise it's going to raise circularity suspicions.

Comment on lines 42 to 43
src = import <nix/fetchurl.nix> {
inherit name;
Copy link

Choose a reason for hiding this comment

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

Suggested change
src = import <nix/fetchurl.nix> {
inherit name;

Comment on lines +44 to 48
url = "https://github.com/emilytrau/bootstrap-tools-nar-mirror/releases/download/2023-05-18/${name}.nar.xz";
hash = "sha256-FpMp7z+B3cR3LkQ+PooH/b1/NlxH8NHVJNWifaPWt4U=";
unpack = true;
};
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
url = "https://github.com/emilytrau/bootstrap-tools-nar-mirror/releases/download/2023-05-18/${name}.nar.xz";
hash = "sha256-FpMp7z+B3cR3LkQ+PooH/b1/NlxH8NHVJNWifaPWt4U=";
unpack = true;
};
}

Please delete this, wait for Hydra to do the build, then re-add it using the tarballs.nixos.org url. That way there won't be any doubts raised about the commit at which Hydra does the build having access to this narball of questionable provenance.

@@ -9,23 +8,30 @@
#
# To build:
#
# nix-build pkgs/os-specific/linux/minimal-bootstrap/stage0-posix/make-bootstrap-sources.nix
# nix-build . -A minimal-bootstrap-sources
Copy link

Choose a reason for hiding this comment

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

Suggested change
# nix-build . -A minimal-bootstrap-sources
# nix-build '<nixpkgs>' -A minimal-bootstrap-sources

@ghost
Copy link

ghost commented Jun 15, 2023

I have to say it is kind of a bummer that we still end up embedding a "magic hash" of some Hydra build. I was kinda hoping the minimal-bootstrap would let us avoid doing that.

Of course, it's less-bad since that Hydra build contains no binaries, but still...

@ghost

This comment was marked as outdated.

@emilytrau
Copy link
Member Author

@amjoseph-nixpkgs iirc builtins.fetchGit isn't available in restricted evaluation environments even with a hash declared.

Also I suppose now that I think of it again, the purpose of uploading this source tarball to tarballs.nixos.org is really just to establish the process in which others can update it with stage0-posix releases, as opposed to having it on my personal GitHub repo. As it's all source code it wouldn't differ in terms of security than fetching a tarball for any other package from any 3rd party source, and the "clean room" benefits of building in hydra maybe isn't integral?

Maybe persuading upstream to publish release tarballs in nar or shar formats would also solve the issue of maintenance.

@ghost

This comment was marked as outdated.

@ghost ghost mentioned this pull request Jun 15, 2023
8 tasks
@emilytrau
Copy link
Member Author

emilytrau commented Jun 15, 2023

@amjoseph-nixpkgs another suggestion might be to move the tarball hosting repo to nix-community. The process of generating a nar from a stage0-posix release is almost already automated and could just need someone to open a PR bumping the version whenever needed. It would definitely be easier than going through this tarballs.nixos.org process every time.

@ghost
Copy link

ghost commented Jun 15, 2023

Ah yes, now I remember why my scheme is doomed to fail:

It isn't documented in the manual, but builder="builtins:fetchurl" is the "cheat code" that lets you skip the restricted-mode allowed-uris check. If you use builtins.fetchurl or builtins.fetchGit or builtins.fetchTarball you don't get to skip that check.

There is no builder="builtins:fetchgit" or builder="builtins:fetchtarball".

In fact, there is no "cheat-code" that lets you access any of the libarchive unpacking functionality that is built into Nix (except for xz-decompression).

That became a problem for NixOS's channels, so another "cheat-code" was added for that use case: builder="builtins:unpack-channel". But it barfs if the tarball contains more than one file.

So yeah, we have to put a nar somewhere.

Anyways, the split-it-into-two-PRs part of my review still stands; if you could please address that I'll approve this, we can merge it, and start waiting for Hydra to do the build.

@ghost ghost mentioned this pull request Jun 15, 2023
12 tasks
@ghost
Copy link

ghost commented Jun 15, 2023

@amjoseph-nixpkgs another suggestion might be to move the tarball hosting repo to nix-community.

Or have somebody run a streaming HTTP GET tarball-to-narball converter and point ./pkgs/build-support/fetchurl/boot.nix at that.

curl -o stage0-posix.nar \
  http://nixos.org/tar2nar/https://github.com/oriansj/stage0-posix/archive/3189b5f325b7ef8b88e3edec7c1cde4fce73c76c.tar

@Ericson2314
Copy link
Member

I am not knowledgeable about this Hydra policy tarball stuff, and @amjoseph-nixpkgs is, so I defer to him.

@Ericson2314
Copy link
Member

#238357 does make sense to me though. Does it make sense to you @emilytrau as a replacement?

@emilytrau
Copy link
Member Author

#238357 makes a lot of sense to me as well :)

@emilytrau emilytrau closed this Jun 24, 2023
@emilytrau emilytrau deleted the minimal-bootstrap-sources branch June 24, 2023 23:11
mccurdyc pushed a commit to mccurdyc/nixpkgs that referenced this pull request Jul 3, 2023
This commit adjusts NixOS#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants