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

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

[Carl Dong]

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(-)

diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
index 76d15f4c59..c051c77ad0 100644
--- a/gnu/packages/cross-base.scm
+++ b/gnu/packages/cross-base.scm
@@ -454,7 +454,9 @@ target that libc."
   "Return LIBC cross-built for TARGET, a GNU triplet. Use XGCC and XBINUTILS
 and the cross tool chain."
   (if (cross-newlib? target libc)
-      (native-libc target libc)
+      (native-libc target libc
+                   #:xgcc xgcc
+                   #:xbinutils xbinutils)
       (let ((libc libc))
         (package (inherit libc)
           (name (string-append "glibc-cross-" target))
@@ -511,10 +513,15 @@ and the cross tool chain."

 (define* (native-libc target
                      #:optional
-                     (libc glibc))
+                     (libc glibc)
+                     #:key
+                     (xgcc #f)
+                     (xbinutils #f))
   (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))
+  "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."
+  (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)))
+                             '())))
       (build-system gnu-build-system)
       (search-paths
        (list (search-path-specification
@@ -59,7 +74,10 @@
                  ,(string-append triplet "/lib")
                  ,(string-append triplet "/lib64"))))))
       (arguments
-       `(#:configure-flags '(,(string-append "--host=" triplet))
+       `(#:configure-flags '(,(string-append "--host=" triplet)
+                             ,@(if with-winpthreads?
+                                   '("--with-libraries=winpthreads")
+                                   '()))
          #:phases
          (modify-phases %standard-phases
            (add-before 'configure 'setenv
@@ -74,7 +92,13 @@
                           ":" mingw-headers "/include"
                           ":" mingw-headers "/crt"
                           ":" mingw-headers "/defaults/include"
-                          ":" mingw-headers "/direct-x/include"))))))
+                          ":" mingw-headers "/direct-x/include"))
+                 (when ,with-winpthreads?
+                     (let ((mingw-itself (assoc-ref inputs "xlibc")))
+                       (setenv "CROSS_LIBRARY_PATH"
+                               (string-append
+                                mingw-itself "/lib" ":"
+                                mingw-itself "/" ,triplet "/lib"))))))))
          #:make-flags (list "DEFS=-DHAVE_CONFIG_H -D__MINGW_HAS_DXSDK=1")
          #:tests? #f ; compiles and includes glibc headers
          #:strip-binaries? #f))
@@ -98,4 +122,12 @@ several new APIs such as DirectX and DDK, and 64-bit support.")
 (define-public mingw-w64-x86_64
   (make-mingw-w64 "x86_64"))

+(define-public mingw-w64-i686-winpthreads
+  (make-mingw-w64 "i686"
+                  #:with-winpthreads? #t))
+
+(define-public mingw-w64-x86_64-winpthreads
+  (make-mingw-w64 "x86_64"
+                  #:with-winpthreads? #t))
+
 (define-public mingw-w64 mingw-w64-i686)
--
2.23.0

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

[Jan (janneke) Nieuwenhuizen]

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

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

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

[Carl Dong]

Hi janneke!

Thank you for your thorough review, definitely a lot to learn for me still :-)

> 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?

Actually, the library itself is called `winpthreads` according to
http://mingw-w64.org/ and as seen in the codebase here:
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/,
is that okay?

I believe I've addressed the rest of the concerns in the patch below, thanks
again!

Cheers,
Carl Dong

--8<---------------cut here---------------start------------->8---
This recursive package definition really demonstrates how magical Guix
can be :-)

Try invoking:
    ./pre-inst-env guix build mingw-w64-{x86_64,i686}{,-winpthreads}

* 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      | 47 ++++++++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
index 76d15f4c59..13237fb8a8 100644
--- a/gnu/packages/cross-base.scm
+++ b/gnu/packages/cross-base.scm
@@ -454,7 +454,9 @@ target that libc."
   "Return LIBC cross-built for TARGET, a GNU triplet. Use XGCC and XBINUTILS
 and the cross tool chain."
   (if (cross-newlib? target libc)
-      (native-libc target libc)
+      (native-libc target libc
+                   #:xgcc xgcc
+                   #:xbinutils xbinutils)
       (let ((libc libc))
         (package (inherit libc)
           (name (string-append "glibc-cross-" target))
@@ -511,10 +513,15 @@ and the cross tool chain."

 (define* (native-libc target
                      #:optional
-                     (libc glibc))
+                     (libc glibc)
+                     #:key
+                     xgcc
+                     xbinutils)
   (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..88c8d41ef8 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
+                         xbinutils
+                         with-winpthreads?)
+  "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."
+  (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,13 @@
                 (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)))
+                             '())))
       (build-system gnu-build-system)
       (search-paths
        (list (search-path-specification
@@ -59,7 +73,10 @@
                  ,(string-append triplet "/lib")
                  ,(string-append triplet "/lib64"))))))
       (arguments
-       `(#:configure-flags '(,(string-append "--host=" triplet))
+       `(#:configure-flags '(,(string-append "--host=" triplet)
+                             ,@(if with-winpthreads?
+                                   '("--with-libraries=winpthreads")
+                                   '()))
          #:phases
          (modify-phases %standard-phases
            (add-before 'configure 'setenv
@@ -74,7 +91,13 @@
                           ":" mingw-headers "/include"
                           ":" mingw-headers "/crt"
                           ":" mingw-headers "/defaults/include"
-                          ":" mingw-headers "/direct-x/include"))))))
+                          ":" mingw-headers "/direct-x/include"))
+                 (when ,with-winpthreads?
+                     (let ((xlibc (assoc-ref inputs "xlibc")))
+                       (setenv "CROSS_LIBRARY_PATH"
+                               (string-append
+                                xlibc "/lib" ":"
+                                xlibc "/" ,triplet "/lib"))))))))
          #:make-flags (list "DEFS=-DHAVE_CONFIG_H -D__MINGW_HAS_DXSDK=1")
          #:tests? #f ; compiles and includes glibc headers
          #:strip-binaries? #f))
@@ -98,4 +121,12 @@ several new APIs such as DirectX and DDK, and 64-bit support.")
 (define-public mingw-w64-x86_64
   (make-mingw-w64 "x86_64"))

+(define-public mingw-w64-i686-winpthreads
+  (make-mingw-w64 "i686"
+                  #:with-winpthreads? #t))
+
+(define-public mingw-w64-x86_64-winpthreads
+  (make-mingw-w64 "x86_64"
+                  #:with-winpthreads? #t))
+
 (define-public mingw-w64 mingw-w64-i686)
--
2.23.0
--8<---------------cut here---------------end--------------->8---

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

[Jan Nieuwenhuizen]

Carl Dong writes:

Hi Carl,

> Thank you for your thorough review, definitely a lot to learn for me
> still :-)

You're welcome -- however my patches are no better than yours, I was
very happy with its state.

>> 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?
>
> Actually, the library itself is called `winpthreads` according to
> http://mingw-w64.org/ and as seen in the codebase here:
> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/,
> is that okay?

Ah, I see.  That is unfortunate.  We would need a maintainer to decide
then, I'm CC'ing Ludo'.  Personally I would still prefer
`windows-pthreads' or `wpthreads'.

> I believe I've addressed the rest of the concerns in the patch below, thanks
> again!

Certainly, other than that, LGTM!

Thanks a lot for your patch.

Greetings,
janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

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

[Ludovic Courtès]

Hello!

Jan Nieuwenhuizen <janneke@gnu.org> skribis:

>> Thank you for your thorough review, definitely a lot to learn for me
>> still :-)
>
> You're welcome -- however my patches are no better than yours, I was
> very happy with its state.

Yes, the patch LGTM as well!

>>> 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?
>>
>> Actually, the library itself is called `winpthreads` according to
>> http://mingw-w64.org/ and as seen in the codebase here:
>> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/,
>> is that okay?
>
> Ah, I see.  That is unfortunate.  We would need a maintainer to decide
> then, I'm CC'ing Ludo'.  Personally I would still prefer
> `windows-pthreads' or `wpthreads'.

I’d lean towards keeping the upstream name, so “winpthreads”, as per the
current package naming policy (info "(guix) Package Naming").

Thanks,
Ludo’.

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

[Carl Dong]

Ludovic,

Thanks for your response! I'll push the existing patch up to master then :-)

Cheers,
Carl Dong
contact@carldong.me
"I fight for the users"

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

[Ludovic Courtès]

Hi Carl,

Carl Dong <contact@carldong.me> skribis:

> Thanks for your response! I'll push the existing patch up to master then :-)

Great, closing this issue!

Ludo’.