From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marius Bakke Subject: Re: 01/01: gnu: Add nsis-x86_64 and nsis-i686. Date: Sat, 12 Oct 2019 13:57:54 +0200 Message-ID: <87h84e2n25.fsf@devup.no> References: <20191011154450.32639.24321@vcs0.savannah.gnu.org> <20191011154452.2EBC720B50@vcs0.savannah.gnu.org> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Return-path: Received: from eggs.gnu.org ([2001:470:142:3::10]:47340) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iJG28-0006cC-93 for guix-devel@gnu.org; Sat, 12 Oct 2019 07:58:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iJG24-00042q-R6 for guix-devel@gnu.org; Sat, 12 Oct 2019 07:58:04 -0400 Received: from wout5-smtp.messagingengine.com ([64.147.123.21]:57463) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iJG24-00041U-Ag for guix-devel@gnu.org; Sat, 12 Oct 2019 07:58:00 -0400 In-Reply-To: <20191011154452.2EBC720B50@vcs0.savannah.gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Carl Dong , guix-devel@gnu.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hello Carl, Some feedback on the patch, since it did not go through the usual channels :-) guix-commits@gnu.org writes: > carl pushed a commit to branch master > in repository guix. > > commit e214a22007450187c3db5d9fdef6ee021be6e953 > Author: Carl Dong > Date: Wed Sep 11 15:43:19 2019 -0400 > > gnu: Add nsis-x86_64 and nsis-i686. >=20=20=20=20=20 > * guix/build-system/scons.scm (scons-build): Add build-targets and > install-targets parameters. > * guix/build/scons-build-system.scm (build, install): Adjust > accordingly. > * doc/guix.texi (Build Systems): Document it. I would prefer if this was a separate commit, since it is unrelated to NSIS, even though it is a prerequisite. If the NSIS patch had to be reverted for some reason, we would still have this. (there is nothing we can do about that now, of course, but its's something to consider in the future) > * gnu/packages/installers.scm: New file, > (make-nsis): New procedure, > (nsis-x86_64, nsis-i686): New variables. > * gnu/packages/patches/nsis-env-passthru.patch: New file. > * gnu/local.mk (dist_patch_DATA, GNU_SYSTEM_MODULES): Adjust > accordingly. [...] > +(define(make-nsis machine target-arch nsis-target-type) > + (let ((triplet (string-append machine "-" "w64-mingw32"))) > + (package > + (name (string-append "nsis-" machine)) > + (version "3.04") > + (source (origin > + (method url-fetch) > + (uri (string-append "http://prdownloads.sourceforge.net/= nsis/nsis-" > + version "-src.tar.bz2")) > + (sha256 > + (base32 > + "1xgllk2mk36ll2509hd31mfq6blgncmdzmwxj3ymrwshdh23d5b0"= )) > + (patches (search-patches "nsis-env-passthru.patch")))) > + (build-system scons-build-system) > + (native-inputs `(("xgcc" ,(cross-gcc triplet #:libc (cross-libc tr= iplet))) > + ("xbinutils" ,(cross-binutils triplet)) > + ("mingw-w64" ,(cross-libc triplet)))) Nit-pick: we usually don't indent like this. > + (inputs `(("zlib" ,zlib))) > + (arguments > + `(#:scons ,scons-python2 > + #:modules ((srfi srfi-1) > + (guix build utils) > + (guix build scons-build-system)) > + #:tests? #f Could you leave a comment explaining why tests are disabled? > + #:scons-flags `("UNICODE=3Dyes" > + "SKIPUTILS=3DMakeLangId,Makensisw,NSIS Menu,Sub= Start,zip2exe" > + "SKIPDOC=3DCOPYING" > + "STRIP_CP=3Dno" > + ,(string-append "PREFIX=3D" %output) > + ,(string-append "TARGET_ARCH=3D" ,target-arch) > + ,(string-append "XGCC_W32_PREFIX=3D" ,triplet "= -") > + ,(string-append "PREFIX_PLUGINAPI_INC=3D" (asso= c-ref %build-inputs "mingw-w64") "/include/") > + ,(string-append "PREFIX_PLUGINAPI_LIB=3D" (asso= c-ref %build-inputs "mingw-w64") "/lib/")) > + #:build-targets '("makensis" > + "stubs" > + "plugins" > + "utils") > + #:install-targets '("install-stubs" > + "install-plugins" > + "install-data" > + "install-utils" > + "install-compiler" > + "install-conf") > + #:phases (modify-phases %standard-phases > + (add-before 'build 'fix-env > + (lambda _ > + (define* (filter-delimited-string delimited-stri= ng predicate #:optional (delimiter #\:)) > + ;; Given a DELIMITED-STRING delimited by DELIM= ITER, > + ;; only keep items that satisfy PREDICATE Please use punctuations in code comments for consistency. > + (string-join > + (filter predicate (string-split delimited-str= ing delimiter)) > + (string delimiter))) > + (define (mingw-path? path) > + (string-prefix? (assoc-ref %build-inputs "ming= w-w64") path)) > + (for-each > + (lambda (env-name) > + (let ((env-val (getenv env-name))) > + ;; Remove all mingw-w64 paths from env vars= meant > + ;; for native toolchain > + (setenv env-name > + (filter-delimited-string env-val (n= egate mingw-path?))) > + ;; Add the removed paths back into > + ;; CROSS_-prefixed version of env vars > + (setenv (string-append "CROSS_" env-name) > + (filter-delimited-string env-val mi= ngw-path?)))) > + '("CPLUS_INCLUDE_PATH" "LIBRARY_PATH" "C_INCLUD= E_PATH")))) > + (add-before 'build 'fix-target-detection > + (lambda _ > + ;; NSIS target detection is screwed up, manually > + ;; change it ourselves > + (substitute* "Source/build.cpp" (("m_target_type= =3DTARGET_X86ANSI") > + (string-append = "m_target_type=3D" ,nsis-target-type)))))))) substitute* and setenv have unspecified return values, so these phases should be modified to end on #t. > + (home-page "http://nsis.sourceforge.net/") > + (synopsis "A professional open source system to create Windows ins= tallers") Everything in Guix is free software, so there is no need to mention "open source" (which is a term we also avoid using, but that's another issue). Synopses also should not start with an article: `guix lint` usually warns about that. Maybe it can be shortened to "Create Windows Installers"? > + (description > + "NSIS (Nullsoft Scriptable Install System) is a professional open > +source system to create Windows installers. It is designed to be as smal= l and > +flexible as possible and is therefore very suitable for internet > +distribution.") "open source" is redundant here too. Also, use two spaces after punctuations to avoid another warning from `guix lint`. > + (license (license:non-copyleft "file://COPYING" > + "See COPYING in the distribution.")= )))) According to COPYING, NSIS is largely the zlib license, with some portions under bzip2 or CPL 1.0. So I think it would be better to have a list of those, with an explaining comment mentioning that COPYING has the gory details. Could you send an updated patch? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl2hv8IACgkQoqBt8qM6 VPr20Af+NWn8ZHquBTTQZfz4i5aLQktlMUmhg9MhEaD67pJs8+rmYGiaTe8AF3NO JggnApV1cjDbRRgDwv6MqVq94mKnAceMkbuJhnKlUik2ePsogecFPoKE2FvHEtFM Btyb04buURQ1oqJ6YedFYWxbYKVqxnDuhPWl/L3HpqF9pdCtUoOznDOTKTdte+WU gMeuLdXr4FI3aQswQlRcFmfUgNL/wEcnT9N1mnjF6Ig0JxgZ0C5xCmIqebAeTWih 41tPkRmUrv75Ouzvcvs0Hlbn05V2974idcXclWCy5IaXToTWvurXec+b76KxuHJj UFYs+x1NYdydnm0Vwnq0K8r1dQe/Uw== =0h34 -----END PGP SIGNATURE----- --=-=-=--