From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] Add php Date: Wed, 09 Nov 2016 16:44:41 +0100 Message-ID: <87eg2k8xp2.fsf@gnu.org> References: <20161030130828.3797d37d@polymos.lepiller.eu> <20161030175105.1f6eeff2@polymos.lepiller.eu> <87ins9s9y1.fsf@duckhunt.i-did-not-set--mail-host-address--so-tickle-me> <20161102224052.7ec98d2d@lepiller.eu> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:56150) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4V3P-0008Dc-Th for guix-devel@gnu.org; Wed, 09 Nov 2016 10:44:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4V3M-0004PF-RD for guix-devel@gnu.org; Wed, 09 Nov 2016 10:44:47 -0500 In-Reply-To: <20161102224052.7ec98d2d@lepiller.eu> (Julien Lepiller's message of "Wed, 2 Nov 2016 22:40:52 +0100") 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: Julien Lepiller Cc: guix-devel@gnu.org Hello Julien, Seems like the original reviewer (hi Marius! ;-)) missed this revision, so here are a few comments. Julien Lepiller skribis: > From 94c512aa3c9710b65b6fce0cd108744a7c308c63 Mon Sep 17 00:00:00 2001 > From: Julien Lepiller > Date: Sun, 30 Oct 2016 15:05:51 +0100 > Subject: [PATCH] gnu: Add php > > * gnu/packages/php.scm: New file. > * gnu/packages/php.scm (php): New variable. Only the first line is needed. > + (snippet > + '(with-directory-excursion "ext" > + (for-each delete-file-recursively > + `("pcre/pcrelib" > + "sqlite3/libsqlite" > + "gd/libgd" > + "mbstring/oniguruma" > + "xmlrpc/libxmlrpc" > + "zip/lib")))))) > + ;; couldn't unbundle these libraries: > + ;"bcmath/libbcmath" ;; this is bc. > + ;"fileinfo/libmagic" > + ;"mbstring/libmbfl" > + ;"date/lib" Is it hard to unbundle =E2=80=98bc=E2=80=99 and =E2=80=98file=E2=80=99 (lib= magic)? > + (build-system gnu-build-system) > + (arguments > + '( > + #:configure-flags Please adjust the indentation (check what other files do.) > + (list (string-append "--with-libxml-dir=3D" > + (assoc-ref %build-inputs "libxml2")) I suggest this trick to make it a bit more concise: #:configure-flags (let-syntax ((with (syntax-rules () ((_ option input) (string-append option "=3D" (assoc-ref %build-inputs input)))))) (list (with "--with-libxml-dir" "libxml2") (with "--with-readline" "readline") =E2=80=A6)) > + ; A lot of tests fail and failure is not considered fatal. > + #:tests? #f)) In what sense is it not considered fatal? :-) It would be nice to have some understanding of why the test fails. If it turns out to be difficult to fix, we can at least document the problem in the comment, with enough detail. Could you try to investigate a bit? > + (license license:gpl2)));(list > + ; (license:non-copyleft "file://LICENSE"); the php license > + ; license:lgpl2.1;bcmath and libmbfl > + ; license:bsd-2;libmagic > + ; license:expat)));date/lib So PHP itself is GPLv2-only? Apart from that, I think we=E2=80=99re almost done. Could you send an updated patch to address those issues? Then we can happily apply it. Thank you for working on it! Ludo=E2=80=99.