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

[Jan Nieuwenhuizen]

Ludovic Courtès writes:

Hello,

>>     gnu: autoconf: Support cross-build.

> Thanks for fixing it!  Some comments:

>> +    (inputs
>> +     (if (%current-target-system)
>> +         `(("perl" ,perl)
>> +           ("m4" ,m4))
>> +         '()))
>>      (native-inputs
>> -     `(("perl" ,perl)
>> -       ("m4" ,m4)))
>> +     (if (%current-target-system)
>> +         `(("perl-for-build" ,perl)
>> +           ("m4-for-build" ,m4))
>> +         `(("perl" ,perl)
>> +           ("m4" ,m4))))
>
> You can remove the ‘if’ in both cases: we always need Perl/M4 both as a
> native input and as an input in both cases.

...yes

> However, that might trigger a rebuild, so perhaps you’ll have to leave
> the ifs, but with a TODO telling to remove it on the next rebuild or
> Marius will be mad at us.

Indeed; so changed to:

    (inputs
     ;; TODO: remove `if' in the next rebuild cycle.
     (if (%current-target-system)
         `(("perl" ,perl)
           ("m4" ,m4))
         '()))
    (native-inputs
     `(("perl" ,perl)
       ("m4" ,m4)))


>> +    (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 :-)

>> +       ,@(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:

--8<---------------cut here---------------start------------->8---
$ 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;
--8<---------------cut here---------------end--------------->8---

shebangs correct, re-execute EVALs: wrong.

I have now changed this bit to:

       ,@(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))
                      #t)))))

> (You previously found that something’s wrong there, but I forgot what…)

(yes, and I did not have the whole story)

Attaching the updated patch in full.

(automake is pretty similar, i'll send an updated patch for that right
after this review is finished).

Greetings,
janneke

>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
+       ,@(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))
+                      #t)))))
+             '())))
     (home-page "https://www.gnu.org/software/autoconf/";)
     (synopsis "Create source code configuration scripts")
     (description
-- 
2.26.0

-- 
Jan Nieuwenhuizen <address@hidden> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com