Subject: [bug#55227] [PATCH core-updates 2/8] commencement: Add stage0-posix.

From: Thiago Jung Bauermann <[email-masked]>
Date: Mon, 09 May 2022 00:49:44 -0300

Hello Jan,

This is great work! This is not an area I have experience in, so I
didn't read it closely, at least as of yet. I have only one minor
comment:

"Jan (janneke) Nieuwenhuizen" <[email-masked]> writes:

> Stage0-posix is a skeleton for bootstrapping all of Stage0 for POSIX systems.
> On x86-linux, from the 357-byte hex0-seed binary from the bootstrap-seeds, it
> builds hex0, kaem, hex1, catm, hex2, M0, cc_x86, M1, M2,
> get_machine, (mescc-tools), and M2-Planet.
>
> * gnu/packages/commencement.scm (stage0-posix): New variable.
> ---
>  gnu/packages/commencement.scm | 109 ++++++++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
>
> diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
> index 663ffacd56..24e1e6ec8c 100644
> --- a/gnu/packages/commencement.scm
> +++ b/gnu/packages/commencement.scm
> @@ -306,6 +306,115 @@ (define bootstrap-seeds
>  M1, and M2-Planet.")
>      (license license:gpl3+)))
>  
> +(define stage0-posix
> +  ;; The initial bootstrap package: no binary inputs except those from
> +  ;; `bootstrap-seeds, for x86 a 357 byte binary seed: `x86/hex0-seed'.
> +  (let ((mescc-tools-version "1.4.0")
> +        (m2-planet-version "1.9.0"))
> +    (package
> +      (name "stage0-posix")
> +      (version "1.4")
> +      (source (bootstrap-origin
> +               (origin
> +                 (method url-fetch)
> +                 (uri (string-append
> +                       "https://lilypond.org/janneke/guix/20220502/"
> +                       "stage0-posix-" version ".tar.gz"))
> +                 (sha256
> +                  (base32
> +                   "1ammifkj33205qrpfm84yb1c99lwgbn4jsl1hd08aab8c9ffz6p4")))))
> +      (native-inputs
> +       `(("bootstrap-seeds" ,bootstrap-seeds)
> +         ("mescc-tools"
> +          ,(bootstrap-origin
> +            (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://lilypond.org/janneke/guix/20220502/"
> +                    "mescc-tools-" mescc-tools-version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "1xi6f48pf5bhajhfis189gpizxij7nbp1vzvsb1aafhz4skkiqvg")))))
> +         ("m2-planet"
> +          ,(bootstrap-origin
> +            (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://lilypond.org/janneke/guix/20220502/"
> +                    "M2-Planet-" m2-planet-version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "1xrn69sc5nz4hwaishqyrcidp1ncxwib9zswl45x378ddz3mmk7g")))))
> +         ,@(%boot-gash-inputs)))
> +      (build-system trivial-build-system)
> +      (arguments
> +       `(#:guile ,%bootstrap-guile
> +         #:modules ((guix build utils))
> +         #:builder
> +         (begin
> +           (use-modules (guix build utils))
> +           (let* ((bootstrap-seeds (assoc-ref %build-inputs "bootstrap-seeds"))
> +                  (mescc-tools (assoc-ref %build-inputs "mescc-tools"))
> +                  (m2-planet (assoc-ref %build-inputs "m2-planet"))
> +                  (source (assoc-ref %build-inputs "source"))
> +                  (tar (assoc-ref %build-inputs "bootar"))
> +                  (bash (assoc-ref %build-inputs "bash"))
> +                  (coreutils (assoc-ref %build-inputs "coreutils"))
> +                  (guile (assoc-ref %build-inputs "guile"))
> +                  (out (assoc-ref %outputs "out"))
> +                  (bindir (string-append out "/bin"))
> +                  (target (or ,(%current-target-system)
> +                              ,(%current-system)))
> +                  (stage0-cpu
> +                   (cond
> +                    ((or (string-prefix? "x86_64-linux" target)

IMHO the code is clearer and less error prone if the (guix utils)
convenience functions are used to test the target rather than directly
doing string pattern matching.

In this case, ‘target-x86-64?’ is the one. There's no need to even pass
an argument to it since the default is what you need.

The check for Linux could be done separately in the body of the builder
with ‘target-linux?’. What do you think?

> +                         (string-prefix? "i686-linux" target))

Here it would be ‘target-x86-32?’.

> +                     "x86")
> +                    ((or (string-prefix? "aarch64-linux" target)
> +                         (string-prefix? "armhf-linux" target))

This is a good case in point. There's a bug above: “armhf-linux” matches
32-bit ARM when building natively because that's what's in
%current-target, but when cross-building %current-target-system will
contain “arm-unknown-linux-gnueabihf” and this branch won't be taken.

Using ‘target-arm32?’ avoids the bug.

> +                     "armv7l")
> +                    (else
> +                     (error "stage0-posix: system not supported" target))))

-- 
Thanks
Thiago