| Message ID | dbe1824a3b222919340538d30ef3948092ba9bc7.1694406359.git.maxim.cournoyer@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Resolve a circular module dependencies in embedded modules | expand |
Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Partially addresses <https://issues.guix.gnu.org/65716>. > > * gnu/packages/avr.scm: Add commentary comment. > (avr-gcc): Turn into this... > (make-avr-gcc): ... procedure. > (avr-libc): Likewise, into... > (make-avr-gcc): ... this. Adjust native-inputs accordingly. > (avr-toolchain): Likewise, into... > (make-avr-toolchain): ... this. > * gnu/packages/avr-xyz.scm (simavr) [propagated-inputs]: replace avr-toolchain > with a call to the 'make-avr-toolchain' procedure. [...] > Fixes <https://issues.guix.gnu.org/65716>. > > Before this change, simply adding the following import: > > modified gnu/packages/firmware.scm > @@ -42,6 +42,7 @@ (define-module (gnu packages firmware) > #:use-module (gnu packages admin) > #:use-module (gnu packages autotools) > #:use-module (gnu packages assembly) > + #:use-module (gnu packages avr) > #:use-module (gnu packages backup) > #:use-module (gnu packages base) > #:use-module (gnu packages bash) > > Would cause byte compilation and/or evaluation to fail due to a circular > module dependency. > > * gnu/packages/embedded.scm: Add commentary. > (gcc-arm-none-eabi-4.9, gcc-arm-none-eabi-6, newlib-arm-none-eabi) > (newlib-nano-arm-none-eabi, gcc-arm-none-eabi-7-2018-q2-update) > (newlib-arm-none-eabi-7-2018-q2-update) > (newlib-nano-arm-none-eabi-7-2018-q2-update) > (arm-none-eabi-toolchain-4.9, arm-none-eabi-nano-toolchain-4.9) > (arm-none-eabi-toolchain-6, arm-none-eabi-nano-toolchain-6) > (arm-none-eabi-toolchain-7-2018-q2-update, gdb-arm-none-eabi) > (propeller-binutils, propeller-gcc-6, propeller-gcc-4) > (propeller-gcc, propeller-toolchain, propeller-development-suite) > (gcc-vc4): Turn into procedures, prefixing the procedure name with 'make-', > and adjust all users. > (make-libstdc++-arm-none-eabi) [arguments]: Avoid an unused warning. > (arm-none-eabi-toolchain): Rename to... > (make-arm-none-eabi-toolchain): ... this. > * gnu/packages/raspberry-pi.scm (raspi-arm-chainloader) [native-inputs]: > Replace gcc-arm-none-eabi-6 with (make-arm-none-eabi-toolchain). > * gnu/packages/axoloti.scm (axoloti-runtime) > [inputs]: Replace arm-none-eabi-nano-toolchain-4.9 > with (make-arm-none-eabi-nano-toolchain-4.9). > (axoloti-patcher): Likewise. > (axoloti-patcher-next) [inputs]: Replace > arm-none-eabi-nano-toolchain-7-2018-q2-update > with (make-arm-none-eabi-nano-toolchain-7-2018-q2-update). People will lose the ability to install those toolchains, for instance with ‘guix install propeller-toolchain’, or to upgrade profiles that contain them (though ‘guix install axoloti-runtime’ is still good, for instance). I’m not sure whether that’s acceptable, but we should check with known users of this, such as Ricardo. I’ve always felt that these toolchains should be provided as part of the “regular” cross-compilation framework in cross-base.scm. Packages that always need to be cross-compiled (to AVR microcontrollers, etc.) would have a hardcoded #:target in their ‘arguments’ field. I forgot why this was rejected. Thanks, Ludo’.
Hi, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> Partially addresses <https://issues.guix.gnu.org/65716>. >> >> * gnu/packages/avr.scm: Add commentary comment. >> (avr-gcc): Turn into this... >> (make-avr-gcc): ... procedure. >> (avr-libc): Likewise, into... >> (make-avr-gcc): ... this. Adjust native-inputs accordingly. >> (avr-toolchain): Likewise, into... >> (make-avr-toolchain): ... this. >> * gnu/packages/avr-xyz.scm (simavr) [propagated-inputs]: replace avr-toolchain >> with a call to the 'make-avr-toolchain' procedure. > > [...] > >> Fixes <https://issues.guix.gnu.org/65716>. >> >> Before this change, simply adding the following import: >> >> modified gnu/packages/firmware.scm >> @@ -42,6 +42,7 @@ (define-module (gnu packages firmware) >> #:use-module (gnu packages admin) >> #:use-module (gnu packages autotools) >> #:use-module (gnu packages assembly) >> + #:use-module (gnu packages avr) >> #:use-module (gnu packages backup) >> #:use-module (gnu packages base) >> #:use-module (gnu packages bash) >> >> Would cause byte compilation and/or evaluation to fail due to a circular >> module dependency. [...] > People will lose the ability to install those toolchains, for instance > with ‘guix install propeller-toolchain’, or to upgrade profiles that > contain them (though ‘guix install axoloti-runtime’ is still good, for > instance). > > I’m not sure whether that’s acceptable, but we should check with known > users of this, such as Ricardo. It's a pity to loose that ability (it's also a pity to not be able to simply 'guix install gcc-cross-some-target', for the same reason) but the statu quo where pulling (gnu packages avr) causes hard to grasp failures is worst, in my opinion. I wasn't able to work on adding packages dependent on (gnu packages avr) for that reason. Debugging was a pain. > I’ve always felt that these toolchains should be provided as part of the > “regular” cross-compilation framework in cross-base.scm. Packages that > always need to be cross-compiled (to AVR microcontrollers, etc.) would > have a hardcoded #:target in their ‘arguments’ field. I forgot why this > was rejected. That'd be an improvement, I think. Right now we have to call a procedure in the input fields everywhere, it's not very elegant. Until then, the change proposed here seems the best we can do. I've been adding new avr-dependent firmware packages in (gnu packages firmware) happily on top of it.
Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Ludovic Courtès <ludo@gnu.org> writes: [...] >> People will lose the ability to install those toolchains, for instance >> with ‘guix install propeller-toolchain’, or to upgrade profiles that >> contain them (though ‘guix install axoloti-runtime’ is still good, for >> instance). >> >> I’m not sure whether that’s acceptable, but we should check with known >> users of this, such as Ricardo. > > It's a pity to loose that ability (it's also a pity to not be able to > simply 'guix install gcc-cross-some-target', for the same reason) but > the statu quo where pulling (gnu packages avr) causes hard to grasp > failures is worst, in my opinion. I wasn't able to work on adding > packages dependent on (gnu packages avr) for that reason. Debugging was > a pain. Oh yes, I’ve been there, so I can tell you I sympathize. :-) I agree that this needs to be addressed. I wondered whether/how hard we should search for a different solution. >> I’ve always felt that these toolchains should be provided as part of the >> “regular” cross-compilation framework in cross-base.scm. Packages that >> always need to be cross-compiled (to AVR microcontrollers, etc.) would >> have a hardcoded #:target in their ‘arguments’ field. I forgot why this >> was rejected. > > That'd be an improvement, I think. Right now we have to call a > procedure in the input fields everywhere, it's not very elegant. BTW, another issue with the package-returning procedures: they return a fresh package object at each call, which defeats caching. I think you should be able to observe it with: GUIX_PROFILING=object-cache guix build axoloti-runtime -d --no-grafts The effect will be more acute on the build farms since they compute derivations for everything that depends on these toolchains. The solution would be to make those procedures memoizing, with ‘mlambda’ or similar. Ludo’.
Hi Ludovic, Ludovic Courtès <ludo@gnu.org> writes: [...] > I agree that this needs to be addressed. I wondered whether/how hard we > should search for a different solution. OK. I think the current situation needs to be resolved ASAP, but longer term I'm definitely willing to revisit our strategy when it comes to cross compiler toolchains. [...] > BTW, another issue with the package-returning procedures: they return a > fresh package object at each call, which defeats caching. I think you > should be able to observe it with: > > GUIX_PROFILING=object-cache guix build axoloti-runtime -d --no-grafts > > The effect will be more acute on the build farms since they compute > derivations for everything that depends on these toolchains. > > The solution would be to make those procedures memoizing, with ‘mlambda’ > or similar. I haven't tried measuring the impact, but I've use mlambda in the v2 just sent; thanks for the suggesting it!
Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Hi Ludovic, > > Ludovic Courtès <ludo@gnu.org> writes: > > [...] > >> I agree that this needs to be addressed. I wondered whether/how hard we >> should search for a different solution. > > OK. I think the current situation needs to be resolved ASAP, but longer > term I'm definitely willing to revisit our strategy when it comes to > cross compiler toolchains. > > [...] > >> BTW, another issue with the package-returning procedures: they return a >> fresh package object at each call, which defeats caching. I think you >> should be able to observe it with: >> >> GUIX_PROFILING=object-cache guix build axoloti-runtime -d --no-grafts >> >> The effect will be more acute on the build farms since they compute >> derivations for everything that depends on these toolchains. >> >> The solution would be to make those procedures memoizing, with ‘mlambda’ >> or similar. > > I haven't tried measuring the impact, but I've use mlambda in the v2 > just sent; thanks for the suggesting it! I've now pushed this series. If we can make AVR a native target/platform in Guix, that may make things simpler/nicer, but otherwise, this is at least resolved.
diff --git a/gnu/packages/avr-xyz.scm b/gnu/packages/avr-xyz.scm index a05157ede7..e8844b8d43 100644 --- a/gnu/packages/avr-xyz.scm +++ b/gnu/packages/avr-xyz.scm @@ -71,7 +71,7 @@ (define-public simavr (string-append "PREFIX=" #$output) (string-append "DESTDIR=" #$output)))) (propagated-inputs - (list avr-toolchain)) + (list (make-avr-toolchain))) (native-inputs (list autoconf which diff --git a/gnu/packages/avr.scm b/gnu/packages/avr.scm index 9c623a9626..abca60eeac 100644 --- a/gnu/packages/avr.scm +++ b/gnu/packages/avr.scm @@ -38,15 +38,27 @@ (define-module (gnu packages avr) #:use-module (gnu packages flashing-tools) #:use-module (gnu packages gcc) #:use-module (gnu packages llvm) - #:use-module (gnu packages vim)) + #:use-module (gnu packages vim) + #:export (make-avr-toolchain)) -(define-public avr-binutils +;;; Commentary: +;;; +;;; This module defines a procedure that can be used to create a complete +;;; avr-toolchain package. The procedure must not be used at the top level, +;;; to avoid cyclic module dependencies caused by the (gnu packages +;;; cross-base) module referring to top level bindings from (gnu packages +;;; gcc). +;;; +;;; It also contains packages for working with or targeting the AVR system. +;;; + +(define (make-avr-binutils) (package (inherit (cross-binutils "avr")) (name "avr-binutils"))) -(define avr-gcc - (let ((xgcc (cross-gcc "avr" #:xbinutils avr-binutils))) +(define (make-avr-gcc) + (let ((xgcc (cross-gcc "avr" #:xbinutils (make-avr-binutils)))) (package (inherit xgcc) (name "avr-gcc") @@ -99,7 +111,7 @@ (define avr-gcc `(("gcc" ,gcc) ,@(package-native-inputs xgcc)))))) -(define avr-libc +(define (make-avr-libc) (package (name "avr-libc") (version "2.0.0") @@ -114,8 +126,8 @@ (define avr-libc (arguments '(#:out-of-source? #t #:configure-flags '("--host=avr"))) - (native-inputs `(("avr-binutils" ,avr-binutils) - ("avr-gcc" ,avr-gcc))) + (native-inputs `(("avr-binutils" ,(make-avr-binutils)) + ("avr-gcc" ,(make-avr-gcc)))) (home-page "https://www.nongnu.org/avr-libc/") (synopsis "The AVR C Library") (description @@ -124,27 +136,31 @@ (define avr-libc (license (license:non-copyleft "http://www.nongnu.org/avr-libc/LICENSE.txt")))) -(define-public avr-toolchain - ;; avr-libc checks the compiler version and passes "--enable-device-lib" for avr-gcc > 5.1.0. - ;; It wouldn't install the library for atmega32u4 etc if we didn't use the corret avr-gcc. - (package - (name "avr-toolchain") - (version (package-version avr-gcc)) - (source #f) - (build-system trivial-build-system) - (arguments '(#:builder (begin (mkdir %output) #t))) - (propagated-inputs - `(("avrdude" ,avrdude) - ("binutils" ,avr-binutils) - ("gcc" ,avr-gcc) - ("libc" ,avr-libc))) - (synopsis "Complete GCC tool chain for AVR microcontroller development") - (description "This package provides a complete GCC tool chain for AVR +(define (make-avr-toolchain) + (let ((avr-binutils (make-avr-binutils)) + (avr-libc (make-avr-libc)) + (avr-gcc (make-avr-gcc))) + ;; avr-libc checks the compiler version and passes "--enable-device-lib" + ;; for avr-gcc > 5.1.0. It wouldn't install the library for atmega32u4 + ;; etc if we didn't use the corret avr-gcc. + (package + (name "avr-toolchain") + (version (package-version avr-gcc)) + (source #f) + (build-system trivial-build-system) + (arguments '(#:builder (begin (mkdir %output) #t))) + (propagated-inputs + `(("avrdude" ,avrdude) + ("binutils" ,avr-binutils) + ("gcc" ,avr-gcc) + ("libc" ,avr-libc))) + (synopsis "Complete GCC tool chain for AVR microcontroller development") + (description "This package provides a complete GCC tool chain for AVR microcontroller development. This includes the GCC AVR cross compiler and avrdude for firmware flashing. The supported programming languages are C and C++.") - (home-page (package-home-page avr-libc)) - (license (package-license avr-gcc)))) + (home-page (package-home-page avr-libc)) + (license (package-license avr-gcc))))) (define-public microscheme (package