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

freshBootstrapTools: remove overlay, prune tools, use tar.xz archive #295557

Merged
4 commits merged into from Mar 20, 2024
Merged

freshBootstrapTools: remove overlay, prune tools, use tar.xz archive #295557

4 commits merged into from Mar 20, 2024

Conversation

ghost
Copy link

@ghost ghost commented Mar 13, 2024

Description of changes

  • unpin LLVM11. fix discrepancy with freshBootstrapTools and the tools
    built on hydra. pinning the stdenv for the hydra build doesn't pin the
    tools as the included packages are able to change.

  • remove unused LLVM tools & libs which reduces the uncompressed and
    compressed file sizes by more than 1/2. compressed tarball is now 40M
    and uncompressed is around 200M. list of accessed bootstrap files

  • remove CoreFoundation and libobjc stubs as they are not required by the bootstrap

  • add @loader_path/. to dylibs that reference other libs in the archive.
    this is needed for libraries with re-exports.

  • validate shared objects with @rpath references contain the reference
    in lib

  • add a test to verify that the @loader_path/ works for libc++ as it
    re-exports libc++abi

  • stdenv.darwin: bootstrap darwin using updated tools #295558 will incorporate the bootstrap archive

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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.

Add a 👍 reaction to pull requests you find important.

@ghost
Copy link
Author

ghost commented Mar 13, 2024

@ofborg build freshBootstrapTools.test

@ofborg ofborg bot added 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 Mar 13, 2024
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

I can review in more detail later but I noticed a fairly important change to address first.

pkgs/stdenv/darwin/make-bootstrap-tools.nix Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Mar 13, 2024

aarch64 build fails

error: output '/nix/store/rh4fmn3k232k3j4vmxnr5fxkd4qpd8g3-stdenv-bootstrap-tools' is not allowed to refer to the following paths:
         /nix/store/cii11dfrz126yb3d03aqyahwq3fanl1g-libobjc-11.0.0

perhaps i shouldn't have deleted these lines:

sed -i -e 's|/nix/store/.*/libobjc.A.dylib|@executable_path/../libobjc.A.dylib|g' \
          $out/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation.tbd

(though @executable_path/../libobjc.A.dylib doesn't seem correct. if @excecutable_path is in $out/bin this should read @executable_path/../lib/libobjc.A.dylib). I like using @loader_path but not sure what that resolves to in a tbd file. the install-name is listed as /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation

@ghost ghost changed the title freshBootstrapTools: remove overlay, add tools, use cpio.xz archive freshBootstrapTools: remove overlay, add tools, use tar.xz archive Mar 14, 2024
@ghost
Copy link
Author

ghost commented Mar 15, 2024

cc @trofi for refresh-tarballs.bash edits. tested by setting $outpath to local build rather than using hydra as the files will not exist until this change is merged.

sample output
# Autogenerated by maintainers/scripts/bootstrap-files/refresh-tarballs.bash as:
# $ ./refresh-tarballs.bash --targets=x86_64-apple-darwin
#
# Metadata:
# - nixpkgs revision: a343533bccc62400e8a9560423486a3b6c11a23b
# - hydra build: https://hydra.nixos.org/job/nixpkgs/trunk/stdenvBootstrapTools.x86_64-apple-darwin.build/latest
# - resolved hydra build: https://hydra.nixos.org/build/252571090
# - instantiated derivation: /nix/store/hdk8xb0jx47295sxk3b89jv49h5chva4-stdenv-bootstrap-tools.drv
# - output directory: /nix/store/dkd06x1idgx3a8q7qc4rw28ds403gkzn-stdenv-bootstrap-tools
# - build time: Fri, 08 Mar 2024 14:27:32 +0000
{
  bootstrapTools = import <nix/fetchurl.nix> {
    url = "http://tarballs.nixos.org/stdenv/x86_64-apple-darwin/a343533bccc62400e8a9560423486a3b6c11a23b/bootstrap-tools.tar.xz";
    hash = "sha256-PzrSLI2VSgplcw5MvYiA1hqglC5imDwInXtSy9zZKIY=";
  };
  unpack = import <nix/fetchurl.nix> {
    url = "http://tarballs.nixos.org/stdenv/x86_64-apple-darwin/a343533bccc62400e8a9560423486a3b6c11a23b/unpack.nar.xz";
    hash = "sha256-43WE2K5VsiGvibJnS1wjtwRbswdIqiezyq2KBsCcJT0=";
    unpack = true;
  };
}

Copy link
Contributor

@trofi trofi left a comment

Choose a reason for hiding this comment

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

refresh-tarballs.bash changes look good to me. Thank you!

@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: rust 6.topic: golang 6.topic: vim 6.topic: ocaml 6.topic: nodejs 6.topic: lua labels Mar 17, 2024
@ghost ghost marked this pull request as ready for review March 17, 2024 07:58
@ghost ghost requested review from reckenrode and stephank March 17, 2024 08:00
@ghost ghost removed the 2.status: work-in-progress This PR isn't done label Mar 17, 2024
@vcunat vcunat added the 6.topic: darwin Running or building packages on Darwin label Mar 17, 2024
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

How often is refresh-tarballs.bash run? I'm hesitant about causing stdenv rebuilds on Darwin more often because CI already has such a hard time keeping up.

pkgs/stdenv/darwin/make-bootstrap-tools.nix Show resolved Hide resolved
pkgs/stdenv/darwin/make-bootstrap-tools.nix Show resolved Hide resolved
pkgs/stdenv/darwin/make-bootstrap-tools.nix Outdated Show resolved Hide resolved
pkgs/stdenv/darwin/make-bootstrap-tools.nix Show resolved Hide resolved
pkgs/stdenv/darwin/make-bootstrap-tools.nix Show resolved Hide resolved
pkgs/stdenv/darwin/make-bootstrap-tools.nix Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Mar 17, 2024

How often is refresh-tarballs.bash run? I'm hesitant about causing stdenv rebuilds on Darwin more often because CI already has such a hard time keeping up.

refresh-tarballs.bash is run manually . this change will reduce pressure on hydra as the stdenvBootstrapTools job will not have to build all the tools under the llvm11 stdenv but can use the cached ones already built with the default stdenv.

@ghost ghost mentioned this pull request Mar 17, 2024
13 tasks
@ghost ghost changed the title freshBootstrapTools: remove overlay, add tools, use tar.xz archive freshBootstrapTools: remove overlay, prune tools, use tar.xz archive Mar 18, 2024
@ghost
Copy link
Author

ghost commented Mar 18, 2024

moved the unpack/patch script into the archive unpackScript so it is possible to make edits to the script and and test without triggering a stdenv rebuild.

@toonn
Copy link
Contributor

toonn commented Mar 18, 2024

My question re the frequency of refresh-tarballs.bash was based on the mistaken assumption that it's run regularly for Linux and when run would always run for Darwin too but it seems the target platform is explicitly specified?

@ghost ghost mentioned this pull request Mar 19, 2024
13 tasks
annalee added 4 commits March 19, 2024 17:28
- allow for fetching and expanding nar archives
- add targets for aarch64 and x86_64 darwin
- unpin LLVM11. fix discrepancy with freshBootstrapTools and the tools
  built on hydra. pinning the stdenv for the hydra build doesn't pin the
  tools as the included packages are able to change.

- remove unused LLVM tools & libs which reduces the uncompressed and
  compressed file sizes by more than 1/2. compressed tarball is now 40M
  and uncompressed is around 200M

- add @loader_path/. to dylibs that reference other libs in the archive.
  this is needed for libraries with re-exports.

- validate shared objects with @rpath references contain the reference
  in lib

- add a test to verify that the @loader_path/ works for libc++ as it
  re-exports libc++abi
@ghost
Copy link
Author

ghost commented Mar 19, 2024

I think all the issues are resolved and i'd like to move forward in getting the tools built on hydra so i can do the second part of this change to use the new tools. the code has been in an inconsistent state for over a year where it was not possible, without changes, to use newly built tools for the bootstrap.

i'll let this sit for another 24 hours or so and if no objections will merge. thanks for all the reviews and matrix discussions!

@vcunat
Copy link
Member

vcunat commented Mar 20, 2024

@ofborg build freshBootstrapTools.test

Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

LGTM
Are we just waiting on the tests to pass?

Copy link
Member

@wegank wegank left a comment

Choose a reason for hiding this comment

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

Probably for the 24 hours' notice.

@vcunat
Copy link
Member

vcunat commented Mar 20, 2024

aarch64-darwin on ofBorg tends to be too overloaded, surely not worth waiting for.

@ghost
Copy link
Author

ghost commented Mar 20, 2024

the last commit was just a rebase with no changes and the ofborg tests passed prior -- and tests passed on the aarch64 community builder -- so wasn't planning on waiting for test to finish. just didn't want to merge without approvals without giving ample notice -- but now there are approvals seems fine.

thanks for the reviews!

@ghost ghost merged commit 1856b32 into NixOS:staging-next Mar 20, 2024
21 of 22 checks passed
@ghost ghost deleted the darwin-bootstrap-tools branch March 20, 2024 12:04
@rrbutani rrbutani added 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related and removed 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related labels May 27, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment 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.

6 participants