From: janneke@gnu.org (Jan (janneke) Nieuwenhuizen)
To: Carl Dong <contact@carldong.me>
Cc: 37813@debbugs.gnu.org
Subject: [bug#37813] [PATCH] gnu: mingw-w64: Add -winpthreads variants.
Date: Sat, 19 Oct 2019 09:35:25 +0200 [thread overview]
Message-ID: <87imolmblu.fsf@gnu.org> (raw)
In-Reply-To: <9ex2heUi-a_eFy92HaMuh0B33VewNqHzK6r5aayN566rDR-hlo78vAmX0vhMzY2_hzGQRXZvXRYustyKwDqmT2SK-KNEm2azpTeusxKMiv8=@carldong.me> (Carl Dong's message of "Fri, 18 Oct 2019 17:52:05 +0000")
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
next prev parent reply other threads:[~2019-10-19 7:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-18 17:52 [bug#37813] [PATCH] gnu: mingw-w64: Add -winpthreads variants Carl Dong
2019-10-19 7:35 ` Jan (janneke) Nieuwenhuizen [this message]
2019-10-21 17:49 ` Carl Dong
2019-10-21 18:35 ` Jan Nieuwenhuizen
2019-10-21 20:53 ` Ludovic Courtès
2019-10-21 21:37 ` Carl Dong
2019-10-23 13:49 ` bug#37813: " Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87imolmblu.fsf@gnu.org \
--to=janneke@gnu.org \
--cc=37813@debbugs.gnu.org \
--cc=contact@carldong.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).