From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH] gnu: Add exempi and eog. Date: Wed, 15 Jul 2015 08:46:46 +0200 Message-ID: <878uahhq95.fsf@elephly.net> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:53786) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFGT7-0002Vl-S5 for guix-devel@gnu.org; Wed, 15 Jul 2015 02:47:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFGT2-0007nm-RI for guix-devel@gnu.org; Wed, 15 Jul 2015 02:47:01 -0400 Received: from sender163-mail.zoho.com ([74.201.84.163]:25665) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFGT2-0007nP-IA for guix-devel@gnu.org; Wed, 15 Jul 2015 02:46:56 -0400 In-reply-to: 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-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: David Hashe Cc: guix-devel@gnu.org Hi David, > +(define-public eog > + (package > + (name "eog") > + (version "3.16.2") > + (source (origin > + (method url-fetch) > + (uri (string-append "mirror://gnome/sources/" name "/" > + (version-major+minor version) "/" > + name "-" version ".tar.xz")) > + (sha256 > + (base32 > + "0frw1b5jix9pffznav5s7ajjx91a8rv5lf4sjvjv3fw65mbnhbw0")))) > + (build-system glib-or-gtk-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (add-after > + 'install 'wrap-eog > + (lambda* (#:key inputs outputs #:allow-other-keys) You don’t seem to be using ‘inputs’ anywhere, so you might as well leave it away. > + (let ((out (assoc-ref outputs "out")) > + (gi-typelib-path (getenv "GI_TYPELIB_PATH"))) > + (wrap-program (string-append out "/bin/eog") > + `("GI_TYPELIB_PATH" ":" prefix (,gi-typelib-path)))) > + #t))))) The many spaces after "GI_TYPELIB_PATH" look odd. Other than that I don’t see any problems with this patch. The patch for ‘exempi’, however, has many indentation problems. > +(define-public exempi > + (package > + (name "exempi") > + (version (string-append "2.2.2")) Why ‘string-append’? > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url "http://anongit.freedesktop.org/git/exempi.git") > + (commit version))) > + (sha256 > + (base32 > + "1z25wij89fn86bm38d9ahhzfq8a2sgxaphdc4lrpyq87dgb766q9")) > + (file-name (string-append name "-" version)))) > + (build-system gnu-build-system) > + (arguments > + ;; FIXME: tests depend on boost, but unable to find headers when > + ;; used as an input The comment is not properly aligned. About finding boost, is there a way to specify the location with a variable? > + `(#:configure-flags '("--disable-unittest") > + #:phases (alist-cons-after > + 'unpack 'fix-autogen > + (lambda _ The indentation of the previous two lines is wrong. Have you considered using the ‘modify-phases’ syntax instead? > + (substitute* "autogen.sh" > + ;; autogen.sh tries to run configure before we > + ;; are able to patch it > + (("^.*topsrcdir/configure.*$") ""))) > + (alist-cons-before > + 'configure 'autogen > + (lambda _ > + (zero? (system* "./autogen.sh"))) > + %standard-phases)))) > + (native-inputs > + `(("autoconf" ,(autoconf-wrapper)) I’ve never seen this before. Is it required to use ‘(autoconf-wrapper)’ here? > + ("automake" ,automake) > + ("libtool" ,libtool))) > + (inputs > + `(("expat" ,expat) > + ("zlib" ,zlib))) The indentation here is wrong. > + (home-page "https://wiki.freedesktop.org/libopenraw/Exempi") > + (synopsis "XMP metadata handling") > + (description "Exempi is an implementation of the Extensible Metadata > +Platform (XMP), which enables embedding metadata in PDF and image formats.") > + (license license:bsd-3))) ~~ Ricardo