[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#37813] [PATCH] gnu: mingw-w64: Add -winpthreads variants.
From: |
Jan (janneke) Nieuwenhuizen |
Subject: |
[bug#37813] [PATCH] gnu: mingw-w64: Add -winpthreads variants. |
Date: |
Sat, 19 Oct 2019 09:35:25 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Carl Dong <address@hidden> 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 <address@hidden> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com