Re: [PATCH] gnu: make-bootstrap: Produce the correct %glibc-bootstrap-tarball for Hurd systems.

[ludo@gnu.org (Ludovic Courtès)]

Hello!

Manolis Ragkousis <manolis837@gmail.com> skribis:

> From a8541a554f9e1653c78b6b45f323426e330d5215 Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <manolis837@gmail.com>
> Date: Mon, 25 Jul 2016 16:53:40 +0300
> Subject: [PATCH] gnu: make-bootstrap: Produce the correct
>  %glibc-bootstrap-tarball for Hurd systems.
>
> * gnu/packages/make-bootstrap.scm (%glibc-bootstrap-tarball): Make it a procedure.
>   (%glibc-stripped): Make it a procedure and move the kernel specific part from
>   here to ...
> * guix/build/make-bootstrap.scm (make-stripped-libc): ... here. New file.
> * Makefile.am (MODULES): Add it.

I like this new (guix build make-bootstrap) module!

It would be ideal if the part that introduces this module were a patch
separate from the Hurd part.  However, that’s too much of a trouble to
split the patch, it’s fine this way.

> +(define (make-stripped-libc output libc kernel-headers)
> +  "Copy to OUTPUT the subset of LIBC and KERNEL-HEADERS that is needed
> +   when producing a bootstrap libc."
      ^
Please align to the left.

> +      (for-each (lambda (file)
> +                  (copy-file (string-append kernel-headers "/include/linux/" file)
> +                             (string-append incdir "/linux/"
> +                                            (basename file))))

This could be written as:

  (install-file (string-append kernel-headers "/include/linux/" file)
                (string-append incdir "/linux"))

> +              (find-files (string-append libc "/lib")
> +                          "^(crt.*|ld.*|lib(c|m|dl|rt|pthread|nsl|util).*\\.so(\\..*)?|\
> +lib(machuser|hurduser).so.*|libc(rt|)_nonshared\\.a)$"))

Maybe move the regexp to a separate variable for clarity, like:

  (define %libc-object-files-rx "^…")

Otherwise LGTM!

Thanks!

Ludo’.