[bug#66263] [PATCH 00/23] guix: Add avr as a platform.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi,

Jean-Pierre De Jesus DIAZ <jean@foundationdevices.com> writes:

> * gnu/packages/cross-toolchain.scm (make-cross-gcc-toolchain): New
>   procedure.

You forgot to mention the registration of the new module in the local.mk
file.

[...]

> +++ b/gnu/packages/cross-toolchain.scm
> @@ -0,0 +1,58 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2023 Foundation Devices, Inc. <hello@foundationdevices.com>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (gnu packages cross-toolchain)
> +  #:use-module (gnu packages avr)
> +  #:use-module (gnu packages cross-base)
> +  #:use-module (guix build-system trivial)
> +  #:use-module (guix packages)
> +  #:use-module (srfi srfi-1)
> +  #:export (make-cross-gcc-toolchain))


I'm a bit confused; why do we need this new module; couldn't it live in
(gnu packages cross-base)?  Also, there are extraneous imports: (gnu packages
avr) is not needed for one.  With a recent Guile you can check for such
imports with

--8<---------------cut here---------------start------------->8---
guild compile -W3 your-file.scm
--8<---------------cut here---------------end--------------->8---

> +(define* (make-cross-gcc-toolchain target
> +                                   #:key
> +                                   (libc (cross-libc target))
> +                                   (xgcc (cross-gcc target #:libc libc))
> +                                   (xbinutils (cross-binutils target)))

Please add a doc string, and memoize packages returning procedures like
this one.

> +  (package
> +    (name (string-append (package-name xgcc) "-toolchain"))
> +    (version (package-version xgcc))
> +    (source #f)
> +    (build-system trivial-build-system)
> +    (arguments
> +     '(#:modules ((guix build union))
> +       #:builder (begin
> +         (use-modules (ice-9 match)
> +                      (guix build union))

This will work as long as all the file names are ASCII only, as there's
no locale support in the trivial-build-system.  I guess this is not a
problem here, but just mentioning it, as it bit me in the past.

-- 
Thanks,
Maxim