From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: Re: 01/01: gnu: Add niftilib. Date: Sun, 19 Mar 2017 16:42:54 -0400 Message-ID: <87d1dd5775.fsf@netris.org> References: <20170318142616.19421.55332@vcs0.savannah.gnu.org> <20170318142617.B58A320DF8@vcs0.savannah.gnu.org> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:49384) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cphfd-0007Cw-VW for guix-devel@gnu.org; Sun, 19 Mar 2017 16:43:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cphfa-0004Gc-Tw for guix-devel@gnu.org; Sun, 19 Mar 2017 16:43:22 -0400 In-Reply-To: <20170318142617.B58A320DF8@vcs0.savannah.gnu.org> (John Darrington's message of "Sat, 18 Mar 2017 10:26:17 -0400 (EDT)") 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: John Darrington Cc: guix-devel@gnu.org john@darrington.wattle.id.au (John Darrington) writes: > jmd pushed a commit to branch master > in repository guix. > > commit 21122bd79e7f9b0b5349ffffe2c146bace7205dc > Author: John Darrington > Date: Tue Mar 7 07:59:21 2017 +0100 > > gnu: Add niftilib. > > * gnu/packages/image.scm (niftilib): New variable. Did you post this for review? Please see below for comments. > +(define-public niftilib > + (package > + (name "niftilib") > + (version "2.0.0") > + (source (origin > + (method url-fetch) > + (uri (list (string-append "mirror://sourceforge/niftilib/" > + "nifticlib/nifticlib_" > + (string-join (string-split version #\.) "_") > + "/nifticlib-" version ".tar.gz"))) Omit the superfluous 'list' call above. > + (sha256 > + (base32 "123z9bwzgin5y8gi5ni8j217k7n683whjsvg0lrpii9flgk8isd3")))) > + (build-system gnu-build-system) > + (arguments > + '(#:tests? #f > + #:parallel-build? #f > + #:phases > + (modify-phases %standard-phases > + (replace 'install Is there a reason not to use the included "make install" target? It looks like it should work, if you pass appropriate settings for INSTALL_{BIN,LIB,INC}_DIR in #:make-flags. > + (lambda _ > + (for-each > + (lambda (dir) > + (let ((directory (assoc-ref %outputs "out"))) If you were going to keep the custom 'install' phase, then instead of using %outputs, please accept the 'outputs' keyword argument and use that. > + (mkdir-p (string-append directory "/" dir)) > + (zero? (system* "cp" "-a" dir directory)))) > + '("bin" "lib" "include")))) We have a 'copy-recursively' procedure that you could use here. If you were going to use "cp", then you should pay attention to its result code so that failures are not silently ignored (by using 'every' instead of 'for-each'). > + (replace 'configure > + (lambda _ > + (substitute* "Makefile" > + (("^SHELL[ \t]*=[ \t]*csh") > + (string-append "SHELL = " > + (assoc-ref %build-inputs "bash") > + "/bin/sh")) > + > + (("^CFLAGS[ \t]*=[ \t]\\$\\(ANSI_FLAGS\\)") > + "CFLAGS = $(ANSI_FLAGS) -fPIC") > + > + (("^ZLIB_INC[ \t]*=[ \t]*-I/usr/include") > + (string-append "ZLIB_INC = -I" > + (assoc-ref %build-inputs "zlib") > + "/include")) > + > + (("^CP[ \t]*=[ \t]*cp") > + (string-append "CP = " > + (assoc-ref %build-inputs "coreutils") > + "/bin/cp"))) Instead of patching the Makefile, it's preferable to simply pass these settings in #:make-flags. Also, within phase procedures, please accept the 'inputs' and 'outputs' keyword arguments instead of using %build-inputs and %outputs. Finally, for purposes of Makefile settings, SHELL can simply be set to "bash" or "sh", since it's in the PATH. I'm not sure why you changed the setting for CP. Mark