Message ID | 20220330092313.23584-1-arunisaac@systemreboot.net |
---|---|
State | Accepted |
Headers | show |
Series | Add wfmash | expand |
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.
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.
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.
> 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.
>> > ,@(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---
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.
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 --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