diff mbox series

[bug#54635,1/5] gnu: gsl: Force bootstrap when cross-compiling to riscv64-linux.

Message ID 20220330092313.23584-1-arunisaac@systemreboot.net
State Accepted
Headers show
Series Add wfmash | expand

Commit Message

Arun Isaac March 30, 2022, 9:23 a.m. UTC
* gnu/packages/maths.scm (gsl)[arguments]: Force autotools bootstrap when
cross-compiling to riscv64-linux.
[native-inputs]: Add autoconf, automake and libtool when cross-compiling to
riscv64-linux.
---
 gnu/packages/maths.scm | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

M March 30, 2022, 11:33 a.m. UTC | #1
Arun Isaac schreef op wo 30-03-2022 om 14:53 [+0530]:
> +    (native-inputs
> +     (if (target-riscv64?)
> +         `(("autoconf" ,autoconf)
> +           ("automake" ,automake)
> +           ("libtool" ,libtool))
> +         '()))

Nowadays input labels are not required anymore here, you can do

 (native-inputs (if (targer-riscv64?) (list autoconf automake libtool)
'()))

Greetings,
Maxime.
M March 30, 2022, 11:36 a.m. UTC | #2
Arun Isaac schreef op wo 30-03-2022 om 14:53 [+0530]:
>             ,@(cond
> +              ((target-riscv64?)
> +               '((add-after 'unpack 'force-bootstrap
> +                   (lambda _
> +                     ;; gsl ships with an old configure script that does not
> +                     ;; support riscv64. Regenerate it.
> +                     (delete-file "configure")))))
> +

WDYT of making this unconditional?  Two benefits:

  * if Guix is ported to another new architecture,
    then no changes are necessary to the package definition.

  * 'configure' and 'Makefile.in' are not source code,
    and more difficult to audit for things like malware than
    'configure.ac' and 'Makefile.am'.

Greetings,
Maxime.
Efraim Flashner March 30, 2022, 11:39 a.m. UTC | #3
On Wed, Mar 30, 2022 at 01:36:34PM +0200, Maxime Devos wrote:
> Arun Isaac schreef op wo 30-03-2022 om 14:53 [+0530]:
> >             ,@(cond
> > +              ((target-riscv64?)
> > +               '((add-after 'unpack 'force-bootstrap
> > +                   (lambda _
> > +                     ;; gsl ships with an old configure script that does not
> > +                     ;; support riscv64. Regenerate it.
> > +                     (delete-file "configure")))))
> > +
> 
> WDYT of making this unconditional?  Two benefits:
> 
>   * if Guix is ported to another new architecture,
>     then no changes are necessary to the package definition.
> 
>   * 'configure' and 'Makefile.in' are not source code,
>     and more difficult to audit for things like malware than
>     'configure.ac' and 'Makefile.am'.

This can be with a TODO for core-updates. gsl itself has about 2000
dependant packages.

That said, I'm not convinced about unilaterally removing configure
unless we make it a policy to remove it. Also, I haven't had trouble
with building gsl on riscv64-linux without this patch.
Arun Isaac March 31, 2022, 6:14 a.m. UTC | #4
> Nowadays input labels are not required anymore here, you can do
>
>  (native-inputs (if (targer-riscv64?) (list autoconf automake libtool)
> '()))

Yes, indeed! Will remove the input labels in the next version of the
patchset.
Arun Isaac March 31, 2022, 6:33 a.m. UTC | #5
>> >             ,@(cond
>> > +              ((target-riscv64?)
>> > +               '((add-after 'unpack 'force-bootstrap
>> > +                   (lambda _
>> > +                     ;; gsl ships with an old configure script that does not
>> > +                     ;; support riscv64. Regenerate it.
>> > +                     (delete-file "configure")))))
>> > +
>> 
>> WDYT of making this unconditional?  Two benefits:
>> 
>>   * if Guix is ported to another new architecture,
>>     then no changes are necessary to the package definition.
>> 
>>   * 'configure' and 'Makefile.in' are not source code,
>>     and more difficult to audit for things like malware than
>>     'configure.ac' and 'Makefile.am'.
>
> This can be with a TODO for core-updates. gsl itself has about 2000
> dependant packages.

I agree. That was my reasoning as well. If we agree that making it
unconditional is the way forward, I can send another patch for
core-updates after this patchset is pushed to master.

> That said, I'm not convinced about unilaterally removing configure
> unless we make it a policy to remove it. Also, I haven't had trouble
> with building gsl on riscv64-linux without this patch.

Without the force-boostrap phase, the configure phase fails during
cross-compilation.

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix build --target=riscv64-linux-gnu gsl
[...]
starting phase `configure'
source directory: "/tmp/guix-build-gsl-2.7.drv-0/gsl-2.7" (relative from build: ".")
build directory: "/tmp/guix-build-gsl-2.7.drv-0/gsl-2.7"
configure flags: ("CC_FOR_BUILD=gcc" "CONFIG_SHELL=/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "SHELL=/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "--prefix=/gnu/store/x7ag3i38ykn2l3f6sfn06bn9356kdk0x-gsl-2.7" "--enable-fast-install" "--build=x86_64-unknown-linux-gnu" "--host=riscv64-linux-gnu" "--disable-static")
checking for a BSD-compatible install... /gnu/store/d251rfgc9nm2clzffzhgiipdvfvzkvwi-coreutils-8.32/bin/install -c
checking whether build environment is sane... yes
checking for riscv64-linux-gnu-strip... riscv64-linux-gnu-strip
checking for a race-free mkdir -p... /gnu/store/d251rfgc9nm2clzffzhgiipdvfvzkvwi-coreutils-8.32/bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... no
checking whether make supports nested variables... yes
checking whether to enable maintainer-specific portions of Makefiles... no
checking for a sed that does not truncate output... /gnu/store/wxgv6i8g0p24q5gcyzd0yr07s8kn9680-sed-4.8/bin/sed
checking whether make sets $(MAKE)... (cached) no
checking build system type... x86_64-unknown-linux-gnu
checking host system type... Invalid configuration `riscv64-linux-gnu': machine `riscv64' not recognized
configure: error: /gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash ./config.sub riscv64-linux-gnu failed
error: in phase 'configure': uncaught exception:
%exception #<&invoke-error program: "/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" arguments: ("./configure" "CC_FOR_BUILD=gcc" "CONFIG_SHELL=/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "SHELL=/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "--prefix=/gnu/store/x7ag3i38ykn2l3f6sfn06bn9356kdk0x-gsl-2.7" "--enable-fast-install" "--build=x86_64-unknown-linux-gnu" "--host=riscv64-linux-gnu" "--disable-static") exit-status: 1 term-signal: #f stop-signal: #f> 
phase `configure' failed after 0.3 seconds
command "/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "./configure" "CC_FOR_BUILD=gcc" "CONFIG_SHELL=/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "SHELL=/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash" "--prefix=/gnu/store/x7ag3i38ykn2l3f6sfn06bn9356kdk0x-gsl-2.7" "--enable-fast-install" "--build=x86_64-unknown-linux-gnu" "--host=riscv64-linux-gnu" "--disable-static" failed with status 1
builder for `/gnu/store/fwjvvklmswc9midrcdg6qir2knvmraif-gsl-2.7.drv' failed with exit code 1
build of /gnu/store/fwjvvklmswc9midrcdg6qir2knvmraif-gsl-2.7.drv failed
View build log at '/var/log/guix/drvs/fw/jvvklmswc9midrcdg6qir2knvmraif-gsl-2.7.drv.gz'.
guix build: error: build of `/gnu/store/fwjvvklmswc9midrcdg6qir2knvmraif-gsl-2.7.drv' failed
--8<---------------cut here---------------end--------------->8---
M March 31, 2022, 11:29 a.m. UTC | #6
Arun Isaac schreef op do 31-03-2022 om 12:03 [+0530]:
> > > WDYT of making this unconditional?  Two benefits:
> > > 
> > >    * if Guix is ported to another new architecture,
> > >      then no changes are necessary to the package definition.
> > > 
> > >    * 'configure' and 'Makefile.in' are not source code,
> > >      and more difficult to audit for things like malware than
> > >      'configure.ac' and 'Makefile.am'.
> > 
> > This can be with a TODO for core-updates. gsl itself has about 2000
> > dependant packages.
> 
> I agree. That was my reasoning as well. If we agree that making it
> unconditional is the way forward, I can send another patch for
> core-updates after this patchset is pushed to master.

I started an e-mail thread about this at
<https://lists.gnu.org/archive/html/guix-devel/2022-03/msg00226.html>.

Greetings,
Maxime.
Efraim Flashner March 31, 2022, 12:35 p.m. UTC | #7
On Thu, Mar 31, 2022 at 12:03:39PM +0530, Arun Isaac wrote:
> 
> >> >             ,@(cond
> >> > +              ((target-riscv64?)
> >> > +               '((add-after 'unpack 'force-bootstrap
> >> > +                   (lambda _
> >> > +                     ;; gsl ships with an old configure script that does not
> >> > +                     ;; support riscv64. Regenerate it.
> >> > +                     (delete-file "configure")))))
> >> > +
> >> 
> >> WDYT of making this unconditional?  Two benefits:
> >> 
> >>   * if Guix is ported to another new architecture,
> >>     then no changes are necessary to the package definition.
> >> 
> >>   * 'configure' and 'Makefile.in' are not source code,
> >>     and more difficult to audit for things like malware than
> >>     'configure.ac' and 'Makefile.am'.
> >
> > This can be with a TODO for core-updates. gsl itself has about 2000
> > dependant packages.
> 
> I agree. That was my reasoning as well. If we agree that making it
> unconditional is the way forward, I can send another patch for
> core-updates after this patchset is pushed to master.
> 
> > That said, I'm not convinced about unilaterally removing configure
> > unless we make it a policy to remove it. Also, I haven't had trouble
> > with building gsl on riscv64-linux without this patch.
> 
> Without the force-boostrap phase, the configure phase fails during
> cross-compilation.
> 
> --8<---------------cut here---------------start------------->8---
..snip..
> --8<---------------cut here---------------end--------------->8---

I somehow missed that when I was testing it before. It builds fine
natively on riscv64-linux and I haven't tested cross-building from
riscv64-linux to another architecture. I think for now we can tag it as
(target-riscv64?) and (%current-target-system) so it only takes effect
when needed.
diff mbox series

Patch

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index ecb85642ec..f01bf51580 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -19,7 +19,7 @@ 
 ;;; Copyright © 2017 Nikita <nikita@n0.is>
 ;;; Copyright © 2017 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2017 Theodoros Foradis <theodoros@foradis.org>
-;;; Copyright © 2017, 2019 Arun Isaac <arunisaac@systemreboot.net>
+;;; Copyright © 2017, 2019, 2022 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2017–2021 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2017 Dave Love <me@fx@gnu.org>
 ;;; Copyright © 2018, 2019, 2020, 2021 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
@@ -533,6 +533,13 @@  (define-public gsl
          #:phases
          (modify-phases %standard-phases
            ,@(cond
+              ((target-riscv64?)
+               '((add-after 'unpack 'force-bootstrap
+                   (lambda _
+                     ;; gsl ships with an old configure script that does not
+                     ;; support riscv64. Regenerate it.
+                     (delete-file "configure")))))
+
               ((or (string-prefix? "aarch64" system)
                    (string-prefix? "powerpc" system))
                ;; Some sparse matrix tests are failing on AArch64 and PowerPC:
@@ -568,6 +575,12 @@  (define-public gsl
                         (string-append "exit (77);\n" all)))))))
 
               (else '()))))))
+    (native-inputs
+     (if (target-riscv64?)
+         `(("autoconf" ,autoconf)
+           ("automake" ,automake)
+           ("libtool" ,libtool))
+         '()))
     (home-page "https://www.gnu.org/software/gsl/")
     (synopsis "Numerical library for C and C++")
     (description