[bug#37813] gnu: mingw-w64: Add -winpthreads variants.

This recursive package definition really demonstrates how magical Guix
can be :-)

* gnu/packages/mingw.scm (make-mingw-w64): Add XGCC, XBINUTILS optional
arguments to specify using a non-default cross-compiler/binutils. Add
WITH-WINPTHREADS? optional argument to allow building with winpthreads
support. Adjust accordingly for the new arguments.
(mingw-w64-i686-winpthreads, mingw-w64-x86_64-winpthreads): Add
variables.
* gnu/packages/cross-base.scm (native-libc): Add XGCC, XBINUTILS
key arugments and pass to MAKE-MINGW-W64.
(cross-libc): Pass XGCC and XBINUTILS to NATIVE-LIBC.
---
 gnu/packages/cross-base.scm | 13 +++++++---
 gnu/packages/mingw.scm      | 48 ++++++++++++++++++++++++++++++-------
 2 files changed, 50 insertions(+), 11 deletions(-)

--
2.23.0

Carl Dong <contact@carldong.me> writes:

Hi Carl!

> This recursive package definition really demonstrates how magical Guix
> can be :-)

Beautiful!  The -winpthreads variant depends on a non-winpthreads, and
"just" by adding that dependency, Guix makes it work, right?

Maybe add an example command to the commit message of how to use it,
something like

--8<---------------cut here---------------start------------->8---
Try:

    ./pre-inst-env guix build ... hello/gcc
--8<---------------cut here---------------end--------------->8---

Your patch looks fine, I only have some cosmetic nitpicks.

As a general remark, in GNU we avoid the use the prefix `win' when we
mean Microsoft Windows.  We either use `windows' in full, or `w' (or
w32).  (https://www.gnu.org/prep/standards/html_node/Trademarks.html).

So, what about using `-windows-pthreads' and `with-windows-pthreads',
throughout?

> * gnu/packages/mingw.scm (make-mingw-w64): Add XGCC, XBINUTILS optional
> arguments to specify using a non-default cross-compiler/binutils. Add
                                                                   ^

> WITH-WINPTHREADS? optional argument to allow building with winpthreads
> support. Adjust accordingly for the new arguments.
          ^

> (mingw-w64-i686-winpthreads, mingw-w64-x86_64-winpthreads): Add
> variables.

Please use two spaces after each sentence:

  arguments to specify using a non-default cross-compiler/binutils.  Add
  WITH-WINDOWS-PTHREADS? optional argument to allow building with
  windows-pthreads support.  Adjust accordingly for the new arguments.

>  (define* (native-libc target
>                       #:optional
> -                     (libc glibc))
> +                     (libc glibc)
> +                     #:key
> +                     (xgcc #f)
> +                     (xbinutils #f))

Keyword arguments, if not supplied by the caller, are initialized with
#f, so you do not need to supply a default `#f', just use

                     #:key xgcc xbinutils)

(or each on a new line if you like).  Only if you want other defaults, you
would do something like

  #:key (xgcc a-package) (xbinutils b-package)

>    (if (target-mingw? target)
>        (let ((machine (substring target 0 (string-index target #\-))))
> -        (make-mingw-w64 machine))
> +        (make-mingw-w64 machine
> +                        #:xgcc xgcc
> +                        #:xbinutils xbinutils))
>        libc))
>
>  (define* (cross-newlib? target
> diff --git a/gnu/packages/mingw.scm b/gnu/packages/mingw.scm
> index fe51780fa3..bbfdc0c134 100644
> --- a/gnu/packages/mingw.scm
> +++ b/gnu/packages/mingw.scm
> @@ -30,12 +30,21 @@
>    #:use-module (guix packages)
>    #:use-module (guix download)
>    #:use-module (guix utils)
> -  #:use-module (ice-9 match))
> +  #:use-module (ice-9 match)
> +  #:export (make-mingw-w64))
>
> -(define-public (make-mingw-w64 machine)
> -  (let ((triplet (string-append machine "-" "w64-mingw32")))
> +(define* (make-mingw-w64 machine
> +                         #:key
> +                         (xgcc #f)
> +                         (xbinutils #f)
> +                         (with-winpthreads? #f))

Similarly for #f values here.

> +  "Return a mingw-w64 for targeting MACHINE. If XGCC or XBINUTILS is specified,
> +use that gcc or binutils when cross-compiling. If WITH-WINPTHREADS? is
> +specified, recurse and return a mingw-w64 with support for winpthreads."

Two spaces after senteces.

> +  (let* ((triplet (string-append machine "-" "w64-mingw32")))
>      (package
> -      (name (string-append "mingw-w64" "-" machine))
> +      (name (string-append "mingw-w64" "-" machine
> +                           (if with-winpthreads? "-winpthreads" "")))
>        (version "6.0.0")
>        (source (origin
>                  (method url-fetch)
> @@ -45,8 +54,14 @@
>                  (sha256
>                   (base32 "1w28mynv500y03h92nh87rgw3fnp82qwnjbxrrzqkmr63q812pl0"))
>                  (patches (search-patches "mingw-w64-6.0.0-gcc.patch"))))
> -      (native-inputs `(("xgcc-core" ,(cross-gcc triplet))
> -                       ("xbinutils" ,(cross-binutils triplet))))
> +      (native-inputs `(("xgcc-core" ,(if xgcc xgcc (cross-gcc triplet)))
> +                       ("xbinutils" ,(if xbinutils xbinutils (cross-binutils triplet)))
> +                       ,@(if with-winpthreads?
> +                             `(("xlibc" ,(make-mingw-w64 machine
> +                                                         #:xgcc xgcc
> +                                                         #:xbinutils xbinutils
> +                                                         #:with-winpthreads? #f)))

You do not need to supply the default #:with-winpthreads #f, just use

                             `(("xlibc" ,(make-mingw-w64 machine
                                                         #:xgcc xgcc
                                                         #:xbinutils xbinutils)))

> +                 (when ,with-winpthreads?
> +                     (let ((mingw-itself (assoc-ref inputs "xlibc")))
> +                       (setenv "CROSS_LIBRARY_PATH"
> +                               (string-append
> +                                mingw-itself "/lib" ":"
> +                                mingw-itself "/" ,triplet "/lib"))))))))

This is ok, but I would prefer a more common naming for mingw-ITSELF,
like `libc' or `xlibc'.

For the rest, LGTM; thanks a lot, very nice work!

Greetings,
janneke