From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Kost Subject: Re: [PATCH] gnu: Add dlib. Date: Mon, 15 Aug 2016 10:43:44 +0300 Message-ID: <87a8gexyrz.fsf@gmail.com> References: <8737m837zk.fsf@ike.i-did-not-set--mail-host-address--so-tickle-me> <20160814172544.GA28736@jasmine> <87twengmbh.fsf@ike.i-did-not-set--mail-host-address--so-tickle-me> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZCYp-0007DN-6m for guix-devel@gnu.org; Mon, 15 Aug 2016 03:43:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZCYk-0005dm-SV for guix-devel@gnu.org; Mon, 15 Aug 2016 03:43:50 -0400 Received: from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]:37347) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZCYk-0005dW-LC for guix-devel@gnu.org; Mon, 15 Aug 2016 03:43:46 -0400 Received: by mail-wm0-x234.google.com with SMTP id i5so87430730wmg.0 for ; Mon, 15 Aug 2016 00:43:46 -0700 (PDT) In-Reply-To: <87twengmbh.fsf@ike.i-did-not-set--mail-host-address--so-tickle-me> (Marius Bakke's message of "Sun, 14 Aug 2016 20:52:34 +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: Marius Bakke Cc: guix-devel Marius Bakke (2016-08-14 22:52 +0300) wrote: Hello and welcome! > From 5e30eff1cf24b236a78cc5abed870992e84f443f Mon Sep 17 00:00:00 2001 > From: Marius Bakke > Date: Sat, 13 Aug 2016 11:26:10 +0100 > Subject: [PATCH] gnu: Add dlib. [...] > +(define-public dlib > + (package > + (name "dlib") > + (version "19.1") > + (source (origin > + (method url-fetch) > + (uri (string-append > + "http://dlib.net/files/dlib-" version ".tar.bz2")) > + (sha256 > + (base32 > + "0p2pvcdalc6jhb6r99ybvjd9x74sclr0ngswdg9j2xl5pj7knbr4")) > + (modules '((guix build utils))) > + (snippet > + '(begin > + ;; Delete ~13MB of bundled dependencies. > + (delete-file-recursively "dlib/external") > + (delete-file-recursively "docs/dlib/external"))))) > + (build-system cmake-build-system) > + (arguments > + `(#:phases > + (let ((test-dir (string-append "../dlib-" ,version "/dlib/test/build"))) I think it's better to move this 'let' inside the phase: ... > + (modify-phases %standard-phases > + (replace 'check > + ;; No test target, so we build and run the unit tests here. > + (lambda _ ... here. > + (mkdir-p test-dir) > + (with-directory-excursion test-dir > + (zero? (system* "cmake" "..")) > + (zero? (system* "cmake" "--build" "." "--config" "Release")) > + (zero? (system* "./dtest" "--runall"))))))))) There is no point in the first 2 'zero?' calls as their returning values will be lost, rather: (and (zero? (system* "cmake" "..")) (zero? (system* "cmake" "--build" "." "--config" "Release")) (zero? (system* "./dtest" "--runall"))) > + (native-inputs > + `(("pkg-config" ,pkg-config))) > + (inputs > + `(("lapack" ,lapack) > + ("libjpeg" ,libjpeg) > + ("libpng" ,libpng) > + ("libx11" ,libx11) > + ("openblas" ,openblas) > + ("zlib" ,zlib))) > + (synopsis "Toolkit for making machine learning and data analysis applications in C++") > + (description > + "Dlib is a modern C++ toolkit containing machine learning algorithms and tools. It > +is used in both industry and academia in a wide range of domains including robotics, > +embedded devices, mobile phones, and large high performance computing environments.") As for me, the above lines (in synopsis and description) are too long, I would stay inside 72-78 columns. > + (home-page "http://dlib.net") > + (license license:boost1.0))) Otherwise, the patch looks good to me, so if Leo or other people don't have comments, I think it can be committed. -- Alex