diff mbox series

[bug#40698,core-updates] : [PATCH v2] gnu: perl: Actually produce a host perl when cross-compiling.

Message ID 87lfmrua71.fsf@gnu.org
State Accepted
Headers show
Series [bug#40698,core-updates] : [PATCH v2] gnu: perl: Actually produce a host perl when cross-compiling. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job

Commit Message

Janneke Nieuwenhuizen April 19, 2020, 8:54 a.m. UTC 
Jan Nieuwenhuizen writes:

> As discussed on IRC, I tried to apply perl-cross
> (https://github.com/arsv/perl-cross) to make perl actually cross build
> for our Hurd VM.

Find an improved patch below.  It fixes

>     Can't exec "": No such file or directory at /gnu/store/dd5a35aca6411w5l29ask7pl626v8j4r-perl-5.30.2/lib/perl5/5.30.2/i586-/Cwd.pm line 204.
>     Can't exec "": No such file or directory at /gnu/store/mfnmg1m37kyrb65alnj6jn2fzw7zkapw-autoconf-cross-2.69/share/autoconf/Autom4te/FileUtils.pm line 345.

which was due to having

    #define SH_PATH ""

in config.h.  It also fixes "cwd" in perl, that was caused by this

              (substitute* "dist/PathTools/Cwd.pm"
                (("/bin/pwd")
-                (which "pwd")))
+                (which "pwd"))) ;TODO: fix cross-compile next rebuild cycle

and now fixed lateron

+                       ;;TODO: fix this in setup-configure next rebuild cycle
+                       (substitute* "dist/PathTools/Cwd.pm"
+                         (("/gnu/store/[^/]*-coreutils-[^/]*") coreutils))

Using this, I can cross-build perl and which allows me to ceate patches
to cross build, autoconf and automake that work to configure Guix git on
the Hurd.

Greetings,
janneke

Comments

Marius Bakke April 19, 2020, 2:33 p.m. UTC | #1 
Jan Nieuwenhuizen <janneke@gnu.org> writes:

> Jan Nieuwenhuizen writes:
>
>> As discussed on IRC, I tried to apply perl-cross
>> (https://github.com/arsv/perl-cross) to make perl actually cross build
>> for our Hurd VM.
>
> Find an improved patch below.  It fixes
>
>>     Can't exec "": No such file or directory at /gnu/store/dd5a35aca6411w5l29ask7pl626v8j4r-perl-5.30.2/lib/perl5/5.30.2/i586-/Cwd.pm line 204.
>>     Can't exec "": No such file or directory at /gnu/store/mfnmg1m37kyrb65alnj6jn2fzw7zkapw-autoconf-cross-2.69/share/autoconf/Autom4te/FileUtils.pm line 345.
>
> which was due to having
>
>     #define SH_PATH ""
>
> in config.h.  It also fixes "cwd" in perl, that was caused by this
>
>               (substitute* "dist/PathTools/Cwd.pm"
>                 (("/bin/pwd")
> -                (which "pwd")))
> +                (which "pwd"))) ;TODO: fix cross-compile next rebuild cycle
>
> and now fixed lateron
>
> +                       ;;TODO: fix this in setup-configure next rebuild cycle
> +                       (substitute* "dist/PathTools/Cwd.pm"
> +                         (("/gnu/store/[^/]*-coreutils-[^/]*") coreutils))
>
> Using this, I can cross-build perl and which allows me to ceate patches
> to cross build, autoconf and automake that work to configure Guix git on
> the Hurd.

Wooow, awesome work (as usual)!

Some feedback on the patch:

> From b10ca56254cd1b1f8ec4b222ad27cb02ce59316c Mon Sep 17 00:00:00 2001
> From: "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org>
> Date: Sat, 18 Apr 2020 17:05:48 +0200
> Subject: [PATCH] gnu: perl: Actually produce a host perl when cross-compiling.
>
> * gnu/packages/perl.scm (perl)[native-inputs]: When cross-compiling, add
> `perl-cross' input.
> [arguments]: When cross-compiling, use it to produce binary for host.
> ---
>  gnu/packages/perl.scm | 106 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 100 insertions(+), 6 deletions(-)

[...]

> @@ -112,7 +115,7 @@
>               ;; Use the right path for `pwd'.
>               (substitute* "dist/PathTools/Cwd.pm"
>                 (("/bin/pwd")
> -                (which "pwd")))
> +                (which "pwd"))) ;TODO: fix cross-compile next rebuild cycle

It might be clearer to add "TODO: use coreutils from INPUTS instead of
'which'" here, maybe mentioning the related substitution below.

>               ;; Build in GNU89 mode to tolerate C++-style comment in libc's
>               ;; <bits/string3.h>.
> @@ -120,10 +123,84 @@
>                 (("-std=c89")
>                  "-std=gnu89"))
>               #t))
> -         (replace 'configure
> -           (lambda* (#:key configure-flags #:allow-other-keys)
> -             (format #t "Perl configure flags: ~s~%" configure-flags)
> -             (apply invoke "./Configure" configure-flags)))
> +         ,@(if (%current-target-system)
> +               `((add-after 'unpack 'unpack-cross
> +                   (lambda* (#:key inputs #:allow-other-keys)
> +                     (let ((cross-checkout (assoc-ref %build-inputs "perl-cross")))
> +                       (invoke "chmod" "-R" "+w" ".")

Please use 'make-file-writable' instead of chmod.

> +                       (copy-recursively cross-checkout "."))
> +                     (let ((bash (assoc-ref %build-inputs "bash")))

Use the scoped 'inputs' instead of the magical %build-inputs.

> +                       (substitute* '("Makefile.config.SH"
> +                                      "cnf/config.guess"
> +                                      "cnf/config.sub"
> +                                      "cnf/configure"
> +                                      "cnf/configure_misc.sh"
> +                                      "miniperl_top")
> +                         (("! */bin/sh") (string-append "! " bash "/bin/bash"))
> +                         ((" /bin/sh") (string-append bash "/bin/bash")))
> +                       (substitute* '("cnf/configure_tool.sh")
> +                         (( "[\t ]*result \"BSD\".*")
> +                          "\
> +			result \"BSD\"
> +			;;
> +			*-gnu)
> +			result \"GNU\"
> +"))
> +                       (substitute* '("ext/Errno/Errno_pm.PL")
> +                         (( "\\$cpp < errno.c") "gcc -E errno.c")))

Should $cpp not be replaced with 'g++'?

> +                     #t))
> +                 (replace 'configure
> +                   (lambda* (#:key configure-flags outputs inputs #:allow-other-keys)
> +                     (let* ((out (assoc-ref outputs "out"))
> +                            (configure-flags
> +                             (cons*
> +                              ;; `perl-cross' confuses target and host
> +                              (string-append "--target=" ,(%current-target-system))
> +                              (string-append "--prefix=" out)
> +                              "-Dbyteorder=1234"
> +                              (filter (negate
> +                                       (lambda (x) (or (string-prefix? "-d" x)
> +                                                       (string-prefix? "-Dcc=" x))))
> +                                      configure-flags)))
> +                            (bash (assoc-ref inputs "bash"))
> +                            (coreutils (assoc-ref inputs "coreutils")))
> +                       (format (current-error-port)
> +                               "running ./configure ~a\n" (string-join configure-flags))
> +                       (apply invoke (cons "./configure" configure-flags))
> +                       (substitute* "config.sh"
> +                         (("^libs=.*") "libs='-ldl -lpthread -lm'\n")
> +                         (("^perllibs=.*") "perllibs='-ldl -lpthread -lm'\n")
> +                         (("/gnu/store/[^/]*-bash-[^/]*") bash))

This phase should add a let binding for (%store-directory) and refer to
that instead of the literal /gnu/store strings (see e.g. 'git').

> +                       (substitute* '("config.h")
> +                         (("^#define OSNAME .*")
> +                          (string-append "#define OSNAME \""
> +                                         ,(if (hurd-target?) "GNU" "Linux")

Would it make sense to upstream this?

> +                                         "\"\n"))
> +                         (("^# HAS_NANOSLEEP") "/* #undef HAS_NANOSLEEP */")

Is this substitution required on all cross-compilation targets?

> +                         (("^#define SH_PATH .*")
> +                          (string-append  "#define SH_PATH \"" bash "/bin/bash\"\n")))
> +                       ;;TODO: fix this in setup-configure next rebuild cycle
> +                       (substitute* "dist/PathTools/Cwd.pm"
> +                         (("/gnu/store/[^/]*-coreutils-[^/]*") coreutils))
> +                       #t)))
> +                 (add-after 'build 'touch-non-built-files-for-install
> +                   (lambda _
> +                     ;; `make install' wants to install this; it wasn't built...
> +                     (mkdir-p "cpan/Pod-Usage/blib/script")
> +                     (with-output-to-file "cpan/Pod-Usage/blib/script/pod2text"
> +                       (lambda _ (display "")))
> +                     (with-output-to-file "cpan/Pod-Usage/blib/script/pod2usage"
> +                       (lambda _ (display "")))
> +                     (with-output-to-file "cpan/Pod-Checker/blib/script/podchecker"
> +                       (lambda _ (display "")))
> +                     (mkdir-p "cpan/Pod-Parser/blib/script")
> +                     (with-output-to-file "cpan/Pod-Parser/blib/script/podselect"
> +                       (lambda _ (display "")))

Using '(call-with-output-file "foo" (const #t))' is clearer IMO.  Also
consider using 'for-each' here.

Phew!  Thanks a lot for this, LGTM!

> +                     #t)))
> +               `((replace 'configure
> +                   (lambda* (#:key configure-flags #:allow-other-keys)
> +                     (format #t "Perl configure flags: ~s~%" configure-flags)
> +                     (apply invoke "./Configure" configure-flags)))))
>           (add-after 'install 'remove-extra-references
>             (lambda* (#:key inputs outputs #:allow-other-keys)
>               (let* ((out     (assoc-ref outputs "out"))
> @@ -152,6 +229,23 @@
>                                               "/lib',\n"))))
>                           config2)
>                 #t))))))
> +    (inputs
> +     (if (%current-target-system)
> +         `(("bash" ,bash-minimal)
> +           ("coreutils" ,coreutils))
> +         '()))
> +    (native-inputs
> +     (if (%current-target-system)
> +         `(("perl-cross"
> +            ,(origin
> +               (method git-fetch)
> +               (uri (git-reference
> +                     (url "https://github.com/arsv/perl-cross")
> +                     (commit "1.3.3")))
> +               (file-name (git-file-name "perl-cross" "1.3.3"))
> +               (sha256
> +                (base32 "065qbl1x44maykaj8p8za0b6qxj74bz7fi2zsrlydir1mqb1js3d")))))
> +         '()))
>      (native-search-paths (list (search-path-specification
>                                  (variable "PERL5LIB")
>                                  (files '("lib/perl5/site_perl")))))
> -- 
> 2.26.0
>
>
> -- 
> Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
> Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com
Janneke Nieuwenhuizen April 20, 2020, 5:38 a.m. UTC | #2 
Marius Bakke writes:

Hello Marius,

>> Using this, I can cross-build perl and which allows me to ceate patches
>> to cross build, autoconf and automake that work to configure Guix git on
>> the Hurd.
>
> Wooow, awesome work (as usual)!
>
> Some feedback on the patch:

Wow, thanks!

>> -                (which "pwd")))
>> +                (which "pwd"))) ;TODO: fix cross-compile next rebuild cycle
>
> It might be clearer to add "TODO: use coreutils from INPUTS instead of
> 'which'" here, maybe mentioning the related substitution below.

Yes, changed now to

             ;; Use the right path for `pwd'.
             ;; TODO: use coreutils from INPUTS instead of 'which'
             ;; in next rebuild cycle, see fixup below.
             (substitute* "dist/PathTools/Cwd.pm"
               (("/bin/pwd")
                (which "pwd")))


>> +                     (let ((cross-checkout (assoc-ref %build-inputs "perl-cross")))
>> +                       (invoke "chmod" "-R" "+w" ".")
>
> Please use 'make-file-writable' instead of chmod.

Finally changed it to

                       (rename-file "Artistic" "Artistic.perl")
                       (rename-file "Copying" "Copying.perl")

>> +                       (copy-recursively cross-checkout "."))
>> +                     (let ((bash (assoc-ref %build-inputs "bash")))
>
> Use the scoped 'inputs' instead of the magical %build-inputs.

Ah, right!  There is the scoped `native-inputs' too.  I have always
missed that and been using %build-inputs instead.  Hmm.

>> +                       (substitute* '("ext/Errno/Errno_pm.PL")
>> +                         (( "\\$cpp < errno.c") "gcc -E errno.c")))
>
> Should $cpp not be replaced with 'g++'?

No, I don't think so.  The non-replaced value of $cpp is "gcc -E -P -", and
that breaks terribly; this substitution is only to remove -the `-P' and
input redirection.  I did change this to the somewhat nicer

                       (substitute* '("ext/Errno/Errno_pm.PL")
                         (("\\$cpp < errno.c") "$Config{cc} -E errno.c")))

and mentioned this with my patch sent to `perl-cross'.

>> +                     #t))
>> +                 (replace 'configure
...
>> +                         (("/gnu/store/[^/]*-bash-[^/]*") bash))
>
> This phase should add a let binding for (%store-directory) and refer to
> that instead of the literal /gnu/store strings (see e.g. 'git').

Ah...nice!  Done.

>> +                       (substitute* '("config.h")
>> +                         (("^#define OSNAME .*")
>> +                          (string-append "#define OSNAME \""
>> +                                         ,(if (hurd-target?) "GNU" "Linux")
>
> Would it make sense to upstream this?

Yes, I think so.  I created a patch, now added as `perl-cross.patch' too
and sent it upstream.

>> +                                         "\"\n"))
>> +                         (("^# HAS_NANOSLEEP") "/* #undef HAS_NANOSLEEP */")
>
> Is this substitution required on all cross-compilation targets?

Good question.  This is no longer necessary now that configure actually
detects the Hurd using gcc; togeether with my patch.

>> +                     ;; `make install' wants to install this; it wasn't built...
>> +                     (mkdir-p "cpan/Pod-Usage/blib/script")
>> +                     (with-output-to-file "cpan/Pod-Usage/blib/script/pod2text"
>> +                       (lambda _ (display "")))
>> +                     (with-output-to-file "cpan/Pod-Usage/blib/script/pod2usage"
>> +                       (lambda _ (display "")))
>> +                     (with-output-to-file "cpan/Pod-Checker/blib/script/podchecker"
>> +                       (lambda _ (display "")))
>> +                     (mkdir-p "cpan/Pod-Parser/blib/script")
>> +                     (with-output-to-file "cpan/Pod-Parser/blib/script/podselect"
>> +                       (lambda _ (display "")))
>
> Using '(call-with-output-file "foo" (const #t))' is clearer IMO.  Also
> consider using 'for-each' here.

Changed to

                     (with-directory-excursion "cpan"
                       (mkdir-p "Pod-Usage/blib/script")
                       (mkdir-p "Pod-Parser/blib/script")
                       (for-each (lambda (file)
                                   (call-with-output-file file
                                     (lambda (port) (display "" port))))
                                 '("Pod-Usage/blib/script/pod2text"
                                   "Pod-Usage/blib/script/pod2usage"
                                   "Pod-Checker/blib/script/podchecker"
                                   "Pod-Parser/blib/script/podselect")))

> Phew!  Thanks a lot for this, LGTM!

With these changes, pushed to core-updates as eaff60b35fed75c60d0db76c589e17d1500f60dd

Thanks a lot for your review, I was pretty certain that I missed some
things; but there were more things that I learned than I expected.
Also, a good question here and there is really helpful for me to improve
things or to do the right thing.

Greetings,
janneke
diff mbox series

Patch

From b10ca56254cd1b1f8ec4b222ad27cb02ce59316c Mon Sep 17 00:00:00 2001
From: "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org>
Date: Sat, 18 Apr 2020 17:05:48 +0200
Subject: [PATCH] gnu: perl: Actually produce a host perl when cross-compiling.

* gnu/packages/perl.scm (perl)[native-inputs]: When cross-compiling, add
`perl-cross' input.
[arguments]: When cross-compiling, use it to produce binary for host.
---
 gnu/packages/perl.scm | 106 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 100 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index d6a75506ec..c8332ecead 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -11,7 +11,7 @@ 
 ;;; Copyright © 2016 Alex Sassmannshausen <alex@pompo.co>
 ;;; Copyright © 2016, 2018, 2020 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
-;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2016, 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2017 Raoul J.P. Bonnal <ilpuccio.febo@gmail.com>
 ;;; Copyright © 2017, 2018 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2017 Adriano Peluso <catonano@gmail.com>
@@ -48,14 +48,17 @@ 
   #:use-module (gnu packages)
   #:use-module (guix packages)
   #:use-module (guix download)
+  #:use-module (guix git-download)
   #:use-module (guix utils)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system perl)
   #:use-module (gnu packages base)
+  #:use-module (gnu packages bash)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages databases)
   #:use-module (gnu packages freedesktop)
   #:use-module (gnu packages gd)
+  #:use-module (gnu packages hurd)
   #:use-module (gnu packages less)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages perl-check)
@@ -112,7 +115,7 @@ 
              ;; Use the right path for `pwd'.
              (substitute* "dist/PathTools/Cwd.pm"
                (("/bin/pwd")
-                (which "pwd")))
+                (which "pwd"))) ;TODO: fix cross-compile next rebuild cycle
 
              ;; Build in GNU89 mode to tolerate C++-style comment in libc's
              ;; <bits/string3.h>.
@@ -120,10 +123,84 @@ 
                (("-std=c89")
                 "-std=gnu89"))
              #t))
-         (replace 'configure
-           (lambda* (#:key configure-flags #:allow-other-keys)
-             (format #t "Perl configure flags: ~s~%" configure-flags)
-             (apply invoke "./Configure" configure-flags)))
+         ,@(if (%current-target-system)
+               `((add-after 'unpack 'unpack-cross
+                   (lambda* (#:key inputs #:allow-other-keys)
+                     (let ((cross-checkout (assoc-ref %build-inputs "perl-cross")))
+                       (invoke "chmod" "-R" "+w" ".")
+                       (copy-recursively cross-checkout "."))
+                     (let ((bash (assoc-ref %build-inputs "bash")))
+                       (substitute* '("Makefile.config.SH"
+                                      "cnf/config.guess"
+                                      "cnf/config.sub"
+                                      "cnf/configure"
+                                      "cnf/configure_misc.sh"
+                                      "miniperl_top")
+                         (("! */bin/sh") (string-append "! " bash "/bin/bash"))
+                         ((" /bin/sh") (string-append bash "/bin/bash")))
+                       (substitute* '("cnf/configure_tool.sh")
+                         (( "[\t ]*result \"BSD\".*")
+                          "\
+			result \"BSD\"
+			;;
+			*-gnu)
+			result \"GNU\"
+"))
+                       (substitute* '("ext/Errno/Errno_pm.PL")
+                         (( "\\$cpp < errno.c") "gcc -E errno.c")))
+                     #t))
+                 (replace 'configure
+                   (lambda* (#:key configure-flags outputs inputs #:allow-other-keys)
+                     (let* ((out (assoc-ref outputs "out"))
+                            (configure-flags
+                             (cons*
+                              ;; `perl-cross' confuses target and host
+                              (string-append "--target=" ,(%current-target-system))
+                              (string-append "--prefix=" out)
+                              "-Dbyteorder=1234"
+                              (filter (negate
+                                       (lambda (x) (or (string-prefix? "-d" x)
+                                                       (string-prefix? "-Dcc=" x))))
+                                      configure-flags)))
+                            (bash (assoc-ref inputs "bash"))
+                            (coreutils (assoc-ref inputs "coreutils")))
+                       (format (current-error-port)
+                               "running ./configure ~a\n" (string-join configure-flags))
+                       (apply invoke (cons "./configure" configure-flags))
+                       (substitute* "config.sh"
+                         (("^libs=.*") "libs='-ldl -lpthread -lm'\n")
+                         (("^perllibs=.*") "perllibs='-ldl -lpthread -lm'\n")
+                         (("/gnu/store/[^/]*-bash-[^/]*") bash))
+                       (substitute* '("config.h")
+                         (("^#define OSNAME .*")
+                          (string-append "#define OSNAME \""
+                                         ,(if (hurd-target?) "GNU" "Linux")
+                                         "\"\n"))
+                         (("^# HAS_NANOSLEEP") "/* #undef HAS_NANOSLEEP */")
+                         (("^#define SH_PATH .*")
+                          (string-append  "#define SH_PATH \"" bash "/bin/bash\"\n")))
+                       ;;TODO: fix this in setup-configure next rebuild cycle
+                       (substitute* "dist/PathTools/Cwd.pm"
+                         (("/gnu/store/[^/]*-coreutils-[^/]*") coreutils))
+                       #t)))
+                 (add-after 'build 'touch-non-built-files-for-install
+                   (lambda _
+                     ;; `make install' wants to install this; it wasn't built...
+                     (mkdir-p "cpan/Pod-Usage/blib/script")
+                     (with-output-to-file "cpan/Pod-Usage/blib/script/pod2text"
+                       (lambda _ (display "")))
+                     (with-output-to-file "cpan/Pod-Usage/blib/script/pod2usage"
+                       (lambda _ (display "")))
+                     (with-output-to-file "cpan/Pod-Checker/blib/script/podchecker"
+                       (lambda _ (display "")))
+                     (mkdir-p "cpan/Pod-Parser/blib/script")
+                     (with-output-to-file "cpan/Pod-Parser/blib/script/podselect"
+                       (lambda _ (display "")))
+                     #t)))
+               `((replace 'configure
+                   (lambda* (#:key configure-flags #:allow-other-keys)
+                     (format #t "Perl configure flags: ~s~%" configure-flags)
+                     (apply invoke "./Configure" configure-flags)))))
          (add-after 'install 'remove-extra-references
            (lambda* (#:key inputs outputs #:allow-other-keys)
              (let* ((out     (assoc-ref outputs "out"))
@@ -152,6 +229,23 @@ 
                                              "/lib',\n"))))
                          config2)
                #t))))))
+    (inputs
+     (if (%current-target-system)
+         `(("bash" ,bash-minimal)
+           ("coreutils" ,coreutils))
+         '()))
+    (native-inputs
+     (if (%current-target-system)
+         `(("perl-cross"
+            ,(origin
+               (method git-fetch)
+               (uri (git-reference
+                     (url "https://github.com/arsv/perl-cross")
+                     (commit "1.3.3")))
+               (file-name (git-file-name "perl-cross" "1.3.3"))
+               (sha256
+                (base32 "065qbl1x44maykaj8p8za0b6qxj74bz7fi2zsrlydir1mqb1js3d")))))
+         '()))
     (native-search-paths (list (search-path-specification
                                 (variable "PERL5LIB")
                                 (files '("lib/perl5/site_perl")))))
-- 
2.26.0