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

aarch64: enable rebootstrap, upgrade to gcc11 #209462

Closed
wants to merge 4 commits into from
Closed

aarch64: enable rebootstrap, upgrade to gcc11 #209462

wants to merge 4 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 7, 2023

Description of changes

I would like to emphasize that the point of this PR is avoiding technical debt in stdenv. The fact that it saves rebuilds in some situations is just an added bonus.

This PR is a two-line change, trivial to revert. Its parent PR, #209459, is independently useful, and in any event doesn't muck around with the internals of the stdenv stages.

This PR has a child, #209601, which greatly reduces the amount of aarch64 rebuilds, at the expense of complexity. I'm not sure the complexity is worth it; the point of that PR is to show that there are various complexity/performance tradeoff points available to choose from.

Motivation:

Incorporates changes from:

Should fix:

Alternative to:

Things done

@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Jan 7, 2023
@ghost
Copy link
Author

ghost commented Jan 7, 2023

@ofborg build protobuf

The protobuf package was said to be failing on aarch64 with gcc>9.

@vcunat
Copy link
Member

vcunat commented Jan 7, 2023

I'm not a fan of this "rebootstrap" principle. Maybe I'm confused. You considered adding one gcc rebuild too heavy, but this seems to add a rebuild of all bootstrap tools (including much more than gcc). Yes, it could be selected by platform, but from my point of view that adds even more complexity.

Rather than this, I'd consider relatively simple update of bootstrap tools for aarch64-linux.

  • Details: trivial switch of default gcc version in the source allows building bootstrap tools including gcc 11 or 12, and we could then switch to bootstrapping from those new tools. (So far we always let Hydra build new bootstrap tools from some recent-ish nixpkgs master, but this wouldn't be such a significant deviation.)

@ghost
Copy link
Author

ghost commented Jan 7, 2023

this seems to add a rebuild of all bootstrap tools

Only on aarch64.

(including much more than gcc).

Not really; most of it is stuff that gcc already depends on. Especially if you measure by build time (which is what matters) rather than package count. Basically it's just busybox.

You considered adding one gcc rebuild too heavy

No, you are misrepresenting my position there. Both this PR and #209063 are temporary fixes; either way they get reverted down the line when we implement the proper fix by making gcc.stage1 its own derivation.

The difference is that this PR is a two-line change, trivial to revert. Its parent PR, #209459, is independently useful, and in any event doesn't muck around with the internals of the stdenv stages. No technical debt.

I'd consider relatively simple update of bootstrap tools for aarch64-linux.

That works too.

@ghost
Copy link
Author

ghost commented Jan 7, 2023

@ofborg build protobuf

@trofi
Copy link
Contributor

trofi commented Jan 7, 2023

If we look at aarch64 rebuild counts:

Before:

$ nix-store --query --graph $(nix-instantiate -A stdenv --argstr system aarch64-linux) | grep -P " -> " | awk '{print $3}' | sort -u | sed 's/"[0-9a-z]\{32\}-/"/g' | sort | uniq -c | sort -n
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
      1 "acl-2.3.1.drv"
      1 "attr-2.5.1.drv"
      1 "autoconf-archive-2022.09.03.drv"
      1 "bison-3.8.2.drv"
      1 "bootstrap-stage0-binutils-wrapper-.drv"
      1 "bootstrap-stage0-glibc-bootstrap.drv"
      1 "bootstrap-stage0-glibc-iconv-bootstrap.drv"
      1 "bootstrap-stage0-stdenv-linux.drv"
      1 "bootstrap-stage1-gcc-wrapper-.drv"
      1 "bootstrap-stage2-gcc-wrapper-.drv"
      1 "bootstrap-stage3-gcc-wrapper-.drv"
      1 "bootstrap-stage4-gcc-wrapper-9.5.0.drv"
      1 "bootstrap-tools.drv"
      1 "coreutils-9.1.drv"
      1 "diffutils-3.8.drv"
      1 "ed-1.18.drv"
      1 "expat-2.5.0.drv"
      1 "findutils-4.9.0.drv"
      1 "gawk-5.1.1.drv"
      1 "gcc-9.5.0.drv"
      1 "gcc-wrapper-9.5.0.drv"
      1 "glibc-2.35-224.drv"
      1 "glibc-iconv-2.35.drv"
      1 "gmp-with-cxx-6.2.1.drv"
      1 "gmp-with-cxx-stage3-6.2.1.drv"
      1 "gmp-with-cxx-stage4-6.2.1.drv"
      1 "gnugrep-3.7.drv"
      1 "gnum4-1.4.19.drv"
      1 "gnumake-4.4.drv"
      1 "gnused-4.8.drv"
      1 "gnutar-1.34.drv"
      1 "gzip-1.12.drv"
      1 "isl-stage3-0.20.drv"
      1 "libffi-3.4.4.drv"
      1 "libidn2-2.3.2.drv"
      1 "libmpc-stage3-1.2.1.drv"
      1 "libunistring-1.0.drv"
      1 "linux-headers-6.0.drv"
      1 "lzip-1.23.drv"
      1 "mpfr-4.1.1.drv"
      1 "mpfr-stage3-4.1.1.drv"
      1 "nuke-references.drv"
      1 "patch-2.7.6.drv"
      1 "pcre-8.45.drv"
      1 "pkg-config-0.29.2.drv"
      1 "pkg-config-wrapper-0.29.2.drv"
      1 "python3-minimal-3.10.9.drv"
      1 "python-setup-hook.sh.drv"
      1 "setup-hook.drv"
      1 "stdenv-linux.drv"
      1 "which-2.21.drv"
      2 "autoconf-2.71.drv"
      2 "automake-1.16.5.drv"
      2 "bootstrap-stage1-stdenv-linux.drv"
      2 "bootstrap-stage2-stdenv-linux.drv"
      2 "bootstrap-stage3-stdenv-linux.drv"
      2 "bootstrap-stage4-stdenv-linux.drv"
      2 "bzip2-1.0.8.drv"
      2 "file-5.43.drv"
      2 "help2man-1.49.2.drv"
      2 "libtool-2.4.7.drv"
      2 "libxcrypt-4.4.33.drv"
      2 "patchelf-0.15.0.drv"
      2 "perl5.36.0-gettext-1.07.drv"
      3 "binutils-2.39.drv"
      3 "binutils-wrapper-2.39.drv"
      3 "expand-response-params.drv"
      3 "gettext-0.21.drv"
      3 "gnu-config-2021-01-25.drv"
      3 "perl-5.36.0.drv"
      3 "texinfo-6.8.drv"
      3 "zlib-1.2.13.drv"
      4 "bash-5.1-p16.drv"
      4 "xz-5.2.9.drv"
      5 "hook.drv"

After:

$ nix-store --query --graph $(nix-instantiate -A stdenv --argstr system aarch64-linux) | grep -P " -> " | awk '{print $3}' | sort -u | sed 's/"[0-9a-z]\{32\}-/"/g' | sort | uniq -c | sort -n
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
      1 "06-initialize-the-symlink-flag.patch.drv"
      1 "300-relative.patch.drv"
      1 "autoconf-2.71.tar.xz.drv"
      1 "autoconf-archive-2022.09.03.tar.xz.drv"
      1 "automake-1.16.5.tar.xz.drv"
      1 "bison-3.8.2.tar.gz.drv"
      1 "bootstrap-tools.tar.xz.drv"
      1 "busybox-1.35.0.drv"
      1 "busybox-1.35.0.tar.bz2.drv"
      1 "busybox.drv"
      1 "cpio-2.13.drv"
      1 "cpio-2.13.tar.bz2.drv"
      1 "curl-7.86.0.drv"
      1 "CVE-2019-13232-1.patch.drv"
      1 "CVE-2019-13232-2.patch.drv"
      1 "CVE-2019-13232-3.patch.drv"
      1 "CVE-2021-38185-1.patch.drv"
      1 "CVE-2021-38185-2.patch.drv"
      1 "CVE-2021-38185-3.patch.drv"
      1 "CVE-2022-30065.patch.drv"
      1 "expat-2.5.0.tar.xz.drv"
      1 "file-5.43.tar.gz.drv"
      1 "fno-common-fix.patch.drv"
      1 "gettext-0.21.tar.gz.drv"
      1 "gettext-1.07.tar.gz.drv"
      1 "glibc-2.35.tar.xz.drv"
      1 "glibc-locales-2.35-224.drv"
      1 "gmp-6.2.1.tar.bz2.drv"
      1 "help2man-1.49.2.tar.xz.drv"
      1 "iconv.c.drv"
      1 "?id=751bee0ee727e8d8b003c87cff77ac76f1dbecd6.drv"
      1 "keyutils-1.6.3.drv"
      1 "libffi-3.4.4.tar.gz.drv"
      1 "libkrb5-1.20.drv"
      1 "libmpc-1.2.1.drv"
      1 "libssh2-1.10.0.drv"
      1 "libtool-2.4.7.tar.gz.drv"
      1 "linux-6.0.tar.xz.drv"
      1 "locales-setup-hook.sh.drv"
      1 "m4-1.4.19.tar.bz2.drv"
      1 "make-4.4.tar.gz.drv"
      1 "mirrors-list.drv"
      1 "mpc-1.2.1.tar.gz.drv"
      1 "mpfr-4.1.1.tar.xz.drv"
      1 "musl-1.2.3.drv"
      1 "musl-1.2.3.tar.gz.drv"
      1 "nghttp2-1.49.0.drv"
      1 "openssl-3.0.7.drv"
      1 "patch01.drv"
      1 "patchutils-0.3.3.drv"
      1 "patchutils-0.3.3.tar.xz.drv"
      1 "perl-5.36.0.tar.gz.drv"
      1 "pkg-config-0.29.2.tar.gz.drv"
      1 "Python-3.10.9.tar.xz.drv"
      1 "source.drv"
      1 "stdenv-bootstrap-tools.drv"
      1 "sys-cdefs.h.drv"
      1 "sys-queue.h.drv"
      1 "sys-tree.h.drv"
      1 "texinfo-6.8.tar.xz.drv"
      1 "unzip-6.0.drv"
      1 "unzip60.tar.gz.drv"
      2 "acl-2.3.1.drv"
      2 "attr-2.5.1.drv"
      2 "bootstrap-stage0-binutils-wrapper-.drv"
      2 "bootstrap-stage0-glibc-bootstrap.drv"
      2 "bootstrap-stage0-glibc-iconv-bootstrap.drv"
      2 "bootstrap-stage0-stdenv-linux.drv"
      2 "bootstrap-stage1-gcc-wrapper-.drv"
      2 "bootstrap-stage2-gcc-wrapper-.drv"
      2 "bootstrap-stage3-gcc-wrapper-.drv"
      2 "bootstrap-stage4-gcc-wrapper-11.3.0.drv"
      2 "bootstrap-tools.drv"
      2 "CVE-2022-28391.patch.drv"
      2 "diffutils-3.8.drv"
      2 "ed-1.18.drv"
      2 "findutils-4.9.0.drv"
      2 "gawk-5.1.1.drv"
      2 "gcc-wrapper-11.3.0.drv"
      2 "glibc-2.35-224.drv"
      2 "glibc-iconv-2.35.drv"
      2 "gmp-with-cxx-stage3-6.2.1.drv"
      2 "gmp-with-cxx-stage4-6.2.1.drv"
      2 "gnugrep-3.7.drv"
      2 "gnused-4.8.drv"
      2 "gzip-1.12.drv"
      2 "isl-stage3-0.20.drv"
      2 "libidn2-2.3.2.drv"
      2 "libmpc-stage3-1.2.1.drv"
      2 "libunistring-1.0.drv"
      2 "lzip-1.23.drv"
      2 "mpfr-stage3-4.1.1.drv"
      2 "patch-2.7.6.drv"
      2 "pcre-8.45.drv"
      2 "setup-hook.drv"
      2 "which-2.21.drv"
      3 "autoconf-archive-2022.09.03.drv"
      3 "bison-3.8.2.drv"
      3 "expat-2.5.0.drv"
      3 "gcc-11.3.0.drv"
      3 "gmp-with-cxx-6.2.1.drv"
      3 "gnum4-1.4.19.drv"
      3 "gnumake-4.4.drv"
      3 "gnutar-1.34.drv"
      3 "libffi-3.4.4.drv"
      3 "linux-headers-6.0.drv"
      3 "mpfr-4.1.1.drv"
      3 "nuke-references.drv"
      3 "python3-minimal-3.10.9.drv"
      3 "python-setup-hook.sh.drv"
      3 "stdenv-linux.drv"
      4 "bootstrap-stage1-stdenv-linux.drv"
      4 "bootstrap-stage2-stdenv-linux.drv"
      4 "bootstrap-stage3-stdenv-linux.drv"
      4 "bootstrap-stage4-stdenv-linux.drv"
      4 "bzip2-1.0.8.drv"
      4 "coreutils-9.1.drv"
      4 "patchelf-0.15.0.drv"
      4 "pkg-config-0.29.2.drv"
      4 "pkg-config-wrapper-0.29.2.drv"
      5 "autoconf-2.71.drv"
      5 "automake-1.16.5.drv"
      5 "file-5.43.drv"
      5 "help2man-1.49.2.drv"
      5 "libtool-2.4.7.drv"
      5 "perl5.36.0-gettext-1.07.drv"
      6 "binutils-wrapper-2.39.drv"
      6 "expand-response-params.drv"
      6 "gnu-config-2021-01-25.drv"
      6 "zlib-1.2.13.drv"
      7 "binutils-2.39.drv"
      7 "gettext-0.21.drv"
      7 "libxcrypt-4.4.33.drv"
      7 "texinfo-6.8.drv"
      8 "bash-5.1-p16.drv"
      8 "xz-5.2.9.drv"
      9 "perl-5.36.0.drv"
     13 "hook.drv"

gcc gets rebuilt 3 times. It's even bigger than #209063. 9 perl rebuilds is also curious, but maybe expected if we do roughly a 3x now.

[UPDATE: included single-time builds to make lists more comparable in length to avoid false impression]

@ghost
Copy link
Author

ghost commented Jan 7, 2023

> 2 "CVE-2022-28391.patch.drv"

Do you really think it's reasonable to count how many times a downloaded patch is used as a build input?

@trofi
Copy link
Contributor

trofi commented Jan 7, 2023

> 2 "CVE-2022-28391.patch.drv"

Do you really think it's reasonable to count how many times a downloaded patch is used as a build input?

I don't think it is, but I did not intend to remove them. I removed filters completely and updated the comment.

@ghost
Copy link
Author

ghost commented Jan 7, 2023

The extra gcc build is because glibc can't do static linking. So the bootstrap busybox is built with musl, which gets its own build of gcc.

It's easy to avoid by copying the statically-linked busybox from the old bootstrap files to the new one. I am too tired to add this tonight, I will do it in the morning.

@trofi
Copy link
Contributor

trofi commented Jan 7, 2023

The extra gcc build is because glibc can't do static linking. So the bootstrap busybox is built with musl, which gets its own build of gcc.

It's easy to avoid by copying the statically-linked busybox from the old bootstrap files to the new one. I am too tired to add this tonight, I will do it in the morning.

Heh, that will make it notQuiteFreshBootstrapTools.

Presence of busybox means that busybox update will trigger world rebuild on aarch64 while previously it was mostly nix and it's reverse dependencies. maintainers/scripts/rebuild-amount.sh says it was affecting 134 packages.

It also might actually make pulling in patches as derivations slightly more significant: curl gets pulled in for the similar reason. Don't know if it's actually needed to be built if fixed output derivation is already present in local store. And it's not very important as curl update triggers large rebuilds anyway. But will make curl update tests more annoying on aarch64-linux.

@ghost
Copy link
Author

ghost commented Jan 7, 2023

It also might actually make pulling in patches as derivations slightly more significant: curl gets pulled in for the similar reason.

No, everything upstream of gcc has to use nix's built-in fetchurl, because curl depends on gcc. Try this command on the nixpkgs source:

rg 'cannot use fetchpatch' nixpkgs

The long-term fix is #188587, but it needs nix features that are experimental (and also not very well-tested yet).

@ghost ghost changed the base branch from master to staging January 7, 2023 20:38
@ghost
Copy link
Author

ghost commented Jan 7, 2023

I would like to emphasize that the point of this PR is avoiding technical debt in stdenv. The fact that it saves rebuilds in some situations is just an added bonus.

Latest push has the same number of gcc rebuilds as #209063 on aarch64 with stale bootstrap-tools.xz. This has two fewer gcc rebuilds on all other platforms, as well as on aarch64 after the bootstrap-tools is updated and rebootstrap switched back off for isAarch64.

The script at the end of this comment is useful for comparisons. Whatever arguments you pass to it, it will pass on to nix-instantiate.

For example, to see the rebuilds that this PR avoids on every platform except aarch64:

#!/usr/bin/env bash
set -euo pipefail

BASELINE_COMMIT=4acd5a978ba5047d6a638e310753ae2cf650c439
REBOOTSTRAP_COMMIT=77c21730c6ce6a0476803978c3e3781112b850e5 # PR209601
STAGE5GCC_COMMIT=6e67eab46ff503bbb99bf51801dd0cd9c4027ae8   # PR209063

find_builds() {
    DRV=$(nix-instantiate $@)
    echo $DRV > /dev/stderr
    nix-store --query --graph $DRV \
        | grep -P " -> " \
        | awk '{print $3}' \
        | sort -u \
        | sed 's/"[0-9a-z]\{32\}-/"/g' \
        | sed 's/^"//g' \
        | sed 's/"$//g' \
        | grep -ve "-wrapper-.*.drv$" \
        | grep -ve "nuke-references.drv$" \
        | grep -ve "^bootstrap-stage.*-stdenv-linux.drv$" \
        | grep -ve "\.patch\.drv$" \
        | grep -ve "hook.drv$" \
        | grep -ve "tar.xz.drv$" \
        | grep -ve "tar.gz.drv$" \
        | grep -ve "^busybox.drv$" \
        | grep -ve "^expand-response-params.drv$" \
        | grep -ve "bootstrap-tools.drv$" \
        | grep -ve "^bootstrap-stage0-" \
        | grep -ve "^linux-headers-" \
        | grep -ve "^python-setup-hook.sh.drv$" \
        | grep -ve "^stdenv-linux.drv$" \
        | grep -ve "^gnu-config" \
        | grep -ve "-patchelfed-" \
        | grep -ve "^mirrors-list.drv$" \
        | sort
}

git switch --detach ${BASELINE_COMMIT}; find_builds $@ > baseline-builds.txt
git switch --detach ${REBOOTSTRAP_COMMIT}; find_builds $@ > rebootstrap-builds.txt
git switch --detach ${STAGE5GCC_COMMIT}; find_builds $@ > stage5gcc-builds.txt

set +e
diff -u baseline-builds.txt rebootstrap-builds.txt | grep ^+ > rebootstrap-added-builds.txt
diff -u baseline-builds.txt stage5gcc-builds.txt | grep ^+ > stage5gcc-added-builds.txt
set -e

#git switch pr/aarch64/gcc11
colordiff -U0 stage5gcc-builds.txt rebootstrap-builds.txt | grep -v \@\@

@ghost
Copy link
Author

ghost commented Jan 7, 2023

@ofborg build icu protobuf

@tpwrules
Copy link
Contributor

tpwrules commented Jan 7, 2023

On my 8 core aarch64-linux VM running 2 jobs in parallel, building the entire stdenv from scratch with no substitutions takes over 150% longer wallclock time with the latest edition of this PR (3c29ce0) as compared to the staging commit used before (4acd5a97), which is a substantial and probably unreasonable amount of additional time. That is ~103 and ~40 minutes respectively. Note that I was unable to pre-download the source and the GCC versions are different so this is pessimistic by at least a few minutes.

If I understand correctly as well, any change whatsoever to the stdenv will require the full time to build a stdenv (40 minutes above) plus whatever partial bootstrap rebuild is possible. The bootstrap tools have more programs than the stdenv AFAIK, so there are also more changes which will result in stdenv rebuilds. I would much rather upgrade the bootstrap tools than use this (except possibly in the upgrading of the bootstrap tools).

I did verify though this successfully builds the two test packages (icu and dejagnu) using GCC 11 without linker errors or runtime aborts.

# for each commit above, with system running on nixos-22.11 branch
# get rid of stdenv
nix-collect-garbage
# build stdenv
time nix-build -A pkgs.stdenv --option substituters '' -j2

@ghost ghost mentioned this pull request Jan 8, 2023
13 tasks
@ghost
Copy link
Author

ghost commented Jan 8, 2023

building the entire stdenv from scratch with no substitutions

You need to use #209601 for build-time experiments.

@ghost
Copy link
Author

ghost commented Jan 8, 2023

I fixed a few bugs in the compare.sh script.

Here are the build diffs of #209063 vs #209601 (this PR plus rebuild optimizations).

Personally I think focusing on added aarch64 rebuilds, while ignoring added rebuilds on all other platforms is just kooky. That's why I kept the rebuild optimizations separate from this PR.

Lines starting with a - are rebuilds that happen in #209063 but do not happen in the optimized version of this PR. Lines starting with a + are rebuilds that are added by the optimized version of this PR.

Bottom line is that it's the same number of gcc builds (two removed, two added), one fewer gettext build, one extra binutils build, and a bunch of tiny stuff.

Frankly I think the fixation on rebuilds (and only aarch64 rebuilds, ignoring all other platforms) is pretty silly. But I will play along.

+acl-2.3.1.drv
+attr-2.5.1.drv
+autoconf-2.71.drv
+autoconf-2.71.drv
+automake-1.16.5.drv
+automake-1.16.5.drv
+bash-5.2-p15.drv
+bash-5.2-p15.drv
+bash-5.2-p15.drv
+binutils-2.39.drv
+bison-3.8.2.drv
+bzip2-1.0.8.drv
+coreutils-9.1.drv
+coreutils-9.1.drv
+diffutils-3.8.drv
+ed-1.18.drv
+file-5.43.drv
+file-5.43.drv
+findutils-4.9.0.drv
-gcc-9.5.0.drv
-gcc-9.5.0.drv
-gettext-0.21.drv
+gawk-5.2.1.drv
+gcc-11.3.0.drv
+gcc-11.3.0.drv
+glibc-2.35-224.drv
+glibc-iconv-2.35.drv
+gmp-with-cxx-6.2.1.drv
-gmp-with-cxx-stage2-6.2.1.drv
+gmp-with-cxx-stage3-6.2.1.drv
+gmp-with-cxx-stage4-6.2.1.drv
+gnugrep-3.7.drv
+gnum4-1.4.19.drv
+gnum4-1.4.19.drv
+gnumake-4.4.drv
+gnumake-4.4.drv
+gnused-4.9.drv
+gnutar-1.34.drv
+gnutar-1.34.drv
+gzip-1.12.drv
-isl-stage2-0.20.drv
+isl-stage3-0.20.drv
-libmpc-stage2-1.2.1.drv
+libidn2-2.3.2.drv
+libmpc-1.2.1.drv
+libmpc-stage3-1.2.1.drv
+libtool-2.4.7.drv
+libtool-2.4.7.drv
+libunistring-1.0.drv
+libxcrypt-4.4.33.drv
+lzip-1.23.drv
-mpfr-stage2-4.1.1.drv
+mpfr-4.1.1.drv
+mpfr-stage3-4.1.1.drv
+patch-2.7.6.drv
+patchelf-0.15.0.drv
+patchelf-0.15.0.drv
+pcre-8.45.drv
+perl-5.36.0.drv
+texinfo-6.8.drv
+xz-5.4.0.drv
+xz-5.4.0.drv
+xz-5.4.0.drv
+zlib-1.2.13.drv

@ghost ghost marked this pull request as ready for review January 8, 2023 01:11
@ghost ghost requested a review from Ericson2314 as a code owner January 8, 2023 01:11
@ghost ghost requested a review from matthewbauer as a code owner January 8, 2023 01:11
@ghost ghost mentioned this pull request Jan 9, 2023
4 tasks
@ghost ghost marked this pull request as draft March 10, 2023 06:22
@ghost ghost closed this Jun 21, 2023
@ghost ghost deleted the pr/aarch64/gcc11 branch June 21, 2023 05:10
This pull request was closed.
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.

3 participants