From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:57424 helo=eggs1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jQPA3-00052h-Bt for guix-patches@gnu.org; Mon, 20 Apr 2020 01:40:03 -0400 Received: from Debian-exim by eggs1p.gnu.org with spam-scanned (Exim 4.90_1) (envelope-from ) id 1jQPA2-0002LI-OY for guix-patches@gnu.org; Mon, 20 Apr 2020 01:40:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:34903) by eggs1p.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jQPA2-0002Jx-CN for guix-patches@gnu.org; Mon, 20 Apr 2020 01:40:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jQPA2-0004Ky-Aj for guix-patches@gnu.org; Mon, 20 Apr 2020 01:40:02 -0400 Subject: bug#40698: [core-updates]: [PATCH v2] gnu: perl: Actually produce a host perl when cross-compiling. Resent-To: guix-patches@gnu.org Resent-Message-ID: From: Jan Nieuwenhuizen References: <87r1wku54w.fsf@gnu.org> <87lfmrua71.fsf@gnu.org> <87imhvh7fh.fsf@devup.no> Date: Mon, 20 Apr 2020 07:38:57 +0200 In-Reply-To: <87imhvh7fh.fsf@devup.no> (Marius Bakke's message of "Sun, 19 Apr 2020 16:33:06 +0200") Message-ID: <87eesisolq.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Marius Bakke Cc: 40698-done@debbugs.gnu.org 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 c= ycle > > 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 "pe= rl-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" "Lin= ux") > > 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/p= od2text" >> + (lambda _ (display ""))) >> + (with-output-to-file "cpan/Pod-Usage/blib/script/p= od2usage" >> + (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 eaff60b35fed75c60d0db76c589e1= 7d1500f60dd 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 --=20 Jan Nieuwenhuizen | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar=C2=AE http://AvatarAcademy.com