Re: 04/05: gnu: autoconf: Support cross-build.

[Ludovic Courtès]

Hullo!

Jan Nieuwenhuizen <address@hidden> skribis:

>>> +    (arguments
>>> +     `(#:tests? #f
>>
>> Nope.  :-)
>
> Ahum, yes, well...this made me blush for a moment ("I should not rely on
> a review by Ludovic for catching such a development hack") but look at
> the three preceeding lines:
>
>      ;; XXX: testsuite: 209 and 279 failed. The latter is an impurity. It
>      ;; should use our own "cpp" instead of "/lib/cpp".
> -    (arguments `(#:tests? #f))
>
> Tests are disabled currently (even on master!), so I guess that
> inserting this line break is fine :-)

Damn it, this is terrible!  But not your fault, for sure.  :-)

>>> +       ,@(if (%current-target-system)
>>> +             `(#:phases
>>> +               (modify-phases %standard-phases
>>> +                 ;; Autoconf cannot be cross-built properly: it lacks the
>>> +                 ;; concept of <tool>-for-build.  It even runs the host
>>> +                 ;; `autom4te' (a perl script) during build.
>>> +                 (add-after 'install 'fake-cross-build
>>> +                   (lambda* (#:key build inputs outputs #:allow-other-keys)
>>> +                     (let ((m4 (assoc-ref inputs "m4"))
>>> +                           (perl (assoc-ref inputs "perl"))
>>> +                           (out  (assoc-ref outputs "out")))
>>> +                      (substitute* (find-files (string-append out "/bin"))
>>> +                        (("/gnu/store/[^/]*-m4-[^/]*") m4)
>>> +                        (("/gnu/store/[^/]*-perl-[^/]*") perl))
>>> +                      #t)))))
>>
>> Why is this needed?  The ‘patch-shebangs’ phase normally takes the
>> inputs, not the native inputs, when changing shebangs.
>
> Because it's not only the shebangs...but
>
> Good question...especially because it teaches me about patch-shebangs!
> Also, I failed to determine exactly what went wrong.  Without m4, perl
> in INPUTS, the shebangs are wrong (obviously).
>
> After adding m4,perl in INPUTS, the shebangs are indeed correct.  I
> hadn't noticed that before, because look:
>
> $ head $(./pre-inst-env guix build --target=i586-pc-gnu 
> autoconf)/bin/autoheader
> #!/gnu/store/ drz7805gcsrqkgr8v43r1f7zydlsxh05-perl-5.30.2/bin/perl
> # -*- Perl -*-
> # Generated from autoheader.in; do not edit by hand.
>
> eval 'case $# in 0) exec 
> /gnu/store/8zvc5mvk0xm3ygrxsgpyy5ilxb5rzjry-perl-5.30.2/bin/perl -S "$0";; *) 
> exec /gnu/store/8zvc5mvk0xm3ygrxsgpyy5ilxb5rzjry-perl-5.30.2/bin/perl -S "$0" 
> "$@";; esac'
>     if 0;
>
> shebangs correct, re-execute EVALs: wrong.

Oh, got it.

> From 3d776e0077d62000f20d23a1b42b32fef718a503 Mon Sep 17 00:00:00 2001
> From: "Jan (janneke) Nieuwenhuizen" <address@hidden>
> Date: Sat, 18 Apr 2020 19:49:54 +0200
> Subject: [PATCH] gnu: autoconf: Support cross-build.
>
> Autoconf cannot be cross-built properly: it lacks the concept of
> <tool>-for-build.  It runs the host `autom4te' (a perl script) during build.
>
> * gnu/packages/autotools.scm (autoconf)[inputs]: When cross-building, add perl
> and m4.
> [native-inputs]: when cross-building, use -for-build names.
> [arguments]: When cross-building, add `fake-cross-build' phase to substitute
> m4 and perl.
> ---
>  gnu/packages/autotools.scm | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/autotools.scm b/gnu/packages/autotools.scm
> index 99ca52730e..fa3ced182f 100644
> --- a/gnu/packages/autotools.scm
> +++ b/gnu/packages/autotools.scm
> @@ -55,12 +55,38 @@
>         (base32
>          "113nlmidxy9kjr45kg9x3ngar4951mvag1js2a3j8nxcz34wxsv4"))))
>      (build-system gnu-build-system)
> +    (inputs
> +     ;; TODO: remove `if' in the next rebuild cycle.
> +     (if (%current-target-system)
> +         `(("perl" ,perl)
> +           ("m4" ,m4))
> +         '()))
>      (native-inputs
>       `(("perl" ,perl)
>         ("m4" ,m4)))
>      ;; XXX: testsuite: 209 and 279 failed. The latter is an impurity. It
>      ;; should use our own "cpp" instead of "/lib/cpp".
> -    (arguments `(#:tests? #f))
> +    (arguments
> +     `(#:tests? #f

I’m really nitpicking, but could you move the XXX comment right after
#:tests? #f?

> +       ,@(if (%current-target-system)
> +             `(#:phases
> +               (modify-phases %standard-phases
> +                 ;; `patch-shebangs' patches shebangs only, and the Perl
> +                 ;; scripts use a re-exec feature that references the build
> +                 ;; hosts' perl.  Also, M4 store references hide in the
> +                 ;; scripts.
> +                 (add-after 'install 'patch-non-shebang-references
> +                   (lambda* (#:key build inputs outputs #:allow-other-keys)
> +                     (let ((m4 (assoc-ref inputs "m4"))
> +                           (perl (assoc-ref inputs "perl"))
> +                           (out  (assoc-ref outputs "out"))
> +                           (store-directory (%store-directory)))
> +                      (substitute* (find-files (string-append out "/bin"))
> +                        (((string-append store-directory "/[^/]*-m4-[^/]*")) 
> m4)
> +                        (((string-append store-directory 
> "/[^/]*-perl-[^/]*"))
> +                         perl))

I’m very much in favor of keeping regexps literal.  What about writing
them as:

  (substitute* …
    (("/[[:graph:]]/bin/m4")
     (string-append m4 "/bin/m4"))
    …)

?

Also please move the comment right below ‘lambda*’.  :-)

And then I think you can push it to ‘core-updates’.

Thank you!

Ludo’.