diff mbox series

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

Message ID L1wRhWjTmpEwuUIPZmPwoG5YKra-1Icc6dzPMgsS0b5ezV1JF7rdXQ08M6l4DgrAup1zxiOGSExePpjN7ZwrC-Ik4fwNxDQzS7MK5Dpk9ss=@carldong.me
State Accepted
Headers show
Series [bug#37813] gnu: mingw-w64: Add -winpthreads variants. | expand

Commit Message

Carl Dong Oct. 21, 2019, 5:49 p.m. UTC 
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(-)

--
2.23.0
--8<---------------cut here---------------end--------------->8---

Comments

Janneke Nieuwenhuizen Oct. 21, 2019, 6:35 p.m. UTC | #1 
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
Ludovic Courtès Oct. 21, 2019, 8:53 p.m. UTC | #2 
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’.
Carl Dong Oct. 21, 2019, 9:37 p.m. UTC | #3 
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"
Ludovic Courtès Oct. 23, 2019, 1:49 p.m. UTC | #4 
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’.
diff mbox series

Patch

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)