From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:35080) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iLjHP-0001yb-CF for guix-patches@gnu.org; Sat, 19 Oct 2019 03:36:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iLjHN-0003zT-Rj for guix-patches@gnu.org; Sat, 19 Oct 2019 03:36:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:43042) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iLjHN-0003zP-OI for guix-patches@gnu.org; Sat, 19 Oct 2019 03:36:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1iLjHN-0007sQ-JP for guix-patches@gnu.org; Sat, 19 Oct 2019 03:36:01 -0400 Subject: [bug#37813] [PATCH] gnu: mingw-w64: Add -winpthreads variants. Resent-Message-ID: From: janneke@gnu.org (Jan (janneke) Nieuwenhuizen) References: <9ex2heUi-a_eFy92HaMuh0B33VewNqHzK6r5aayN566rDR-hlo78vAmX0vhMzY2_hzGQRXZvXRYustyKwDqmT2SK-KNEm2azpTeusxKMiv8=@carldong.me> Date: Sat, 19 Oct 2019 09:35:25 +0200 In-Reply-To: <9ex2heUi-a_eFy92HaMuh0B33VewNqHzK6r5aayN566rDR-hlo78vAmX0vhMzY2_hzGQRXZvXRYustyKwDqmT2SK-KNEm2azpTeusxKMiv8=@carldong.me> (Carl Dong's message of "Fri, 18 Oct 2019 17:52:05 +0000") Message-ID: <87imolmblu.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Carl Dong Cc: 37813@debbugs.gnu.org Carl Dong 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 spe= cified, > +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 "1w28mynv500y03h92nh87rgw3fnp82qwnjbxrrzqkmr63q= 812pl0")) > (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-binu= tils triplet))) > + ,@(if with-winpthreads? > + `(("xlibc" ,(make-mingw-w64 machine > + #:xgcc xgcc > + #:xbinutils xbi= nutils > + #:with-winpthre= ads? #f))) You do not need to supply the default #:with-winpthreads #f, just use `(("xlibc" ,(make-mingw-w64 machine #:xgcc xgcc #:xbinutils xbinut= ils))) > + (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 --=20 Jan Nieuwenhuizen | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar=C2=AE http://AvatarAcademy.com