From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Kost Subject: Re: [PATCH 2/3] gnu: Add ledger. Date: Fri, 13 May 2016 22:16:29 +0300 Message-ID: <87posp7oqa.fsf@gmail.com> References: <1462646320.1130194.601019473.51C9DD36@webmail.messagingengine.com> <1462648816.1139014.601038209.3A886EEC@webmail.messagingengine.com> <20160507222330.GA16592@jasmine> <1462673448.1215366.601202601.6F8B62D7@webmail.messagingengine.com> <20160509033906.GC25977@jasmine> <8737prou9x.fsf@gnu.org> <20160509210103.GB15057@jasmine> <1463023686.3455318.605409553.05451A73@webmail.messagingengine.com> <878tzf1vxu.fsf@gmail.com> <1463070674.3638408.605951137.5C3DB897@webmail.messagingengine.com> 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]:35903) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b1IZh-0006EY-UB for guix-devel@gnu.org; Fri, 13 May 2016 15:16:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b1IZc-0001oI-Kh for guix-devel@gnu.org; Fri, 13 May 2016 15:16:36 -0400 Received: from mail-lb0-x22a.google.com ([2a00:1450:4010:c04::22a]:35393) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b1IZb-0001ny-4I for guix-devel@gnu.org; Fri, 13 May 2016 15:16:32 -0400 Received: by mail-lb0-x22a.google.com with SMTP id ww9so21149244lbc.2 for ; Fri, 13 May 2016 12:16:31 -0700 (PDT) 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: Alex Griffin Cc: guix-devel@gnu.org Alex Griffin (2016-05-12 19:31 +0300) wrote: > On Thu, May 12, 2016, at 04:12 AM, Alex Kost wrote: >> Hello, was this package built successfully for you? I tried it but the >> build phase failed (I'm attaching the last part of the build process [1] >> just in case). I don't know, maybe I just don't have enough memory >> (3GB) to build it (I had such problems with cmake before). > > Yes, it builds fine for me. It looks like the important line in your > build log is "c++: internal compiler error: Killed (program cc1plus)", > which could be from running out of memory. Does it still happen if you > add `#:parallel-build? #f` to the build system arguments? Oh indeed, if parallel-build is disabled, it is built successfully. Thank you (and Leo)! So I don't know, should =E2=80=98#:parallel-build? #f=E2=80=99 be used in t= he final package recipe? I guess not, as it looks like a problem on my side (not enough memory for parallel-build). >> Every phase should return non-false value if it succeeded. So since the >> returned value of 'install-file' is not specified, we add #t to the end >> of such phases. Please add #t to all phases except the following >> (build-doc) because zero? returns #t if succeeded. > > OK, done. > >> Unlike configure-flags where we can use only %build-inputs, in phases, >> it is better to use a functional style using 'inputs' passed to a phase >> as argument: > > Done. > >> It doesn't matter but usually we put #:configure-flags before #:phases. > > Done. Attached is an updated patch. Thank you! Overall the patch looks good to me, I have only a couple of comments more (I'm sorry I didn't notice it before :-)). But no need to resent the patch, I can fix it on my side and commit it. The only thing that is unclear for me is whether this parallel-build should be disabled or not. > From 0090f1d526f35a72144a3d32158cbad987ef4d10 Mon Sep 17 00:00:00 2001 > From: Alex Griffin > Date: Sat, 7 May 2016 12:20:47 -0500 > Subject: [PATCH 2/2] gnu: Add ledger. > > * gnu/packages/finance.scm (ledger): New variable. [...] > +(define-public ledger > + (package > + (name "ledger") > + (version "3.1.1") > + (source (origin > + (method url-fetch) > + (uri (string-append "https://github.com/ledger/ledger/arch= ive/v" > + version > + ".tar.gz")) > + (file-name (string-append name "-" version ".tar.gz")) > + (sha256 > + (base32 > + "12jlv3gsjhrja25q9hrwh73cdacd2l3c2yyn8qnijav9mdhnbw4h"))= )) > + (build-system cmake-build-system) > + (arguments > + `(#:configure-flags > + `("-DBUILD_DOCS:BOOL=3DON" > + "-DBUILD_WEB_DOCS:BOOL=3DON" > + "-DBUILD_EMACSLISP:BOOL=3DON" > + "-DUSE_PYTHON:BOOL=3DON" > + "-DCMAKE_INSTALL_LIBDIR:PATH=3Dlib" > + ,(string-append "-DUTFCPP_INCLUDE_DIR:PATH=3D" > + (assoc-ref %build-inputs "utfcpp") > + "/include")) > + #:phases > + (let* ((out (assoc-ref %outputs "out")) > + (examples (string-append out "/share/doc/ledger/examples")) > + (site-lisp (string-append out "/share/emacs/site-lisp"))) > + (modify-phases %standard-phases I only notice just now that you have 'modify-phases' inside this let. I would rather make local 'let'-s with necessary variables inside phases. I mean 'examples' directory is needed only inside 'install-examples' phase; 'site-lisp' inside 'relocate-elisp' phase. Also it is better to use 'outputs' argument passed to phases instead of the global %outputs. > + (add-before 'configure 'install-examples > + (lambda _ > + (install-file "test/input/sample.dat" examples) > + (install-file "test/input/demo.ledger" examples) > + #t)) > + (add-after 'build 'build-doc > + (lambda _ (zero? (system* "make" "doc")))) > + (add-before 'check 'check-setup > + ;; one test fails if it can't set the timezone > + (lambda* (#:key inputs #:allow-other-keys) > + (setenv "TZDIR" > + (string-append (assoc-ref inputs "tzdata") > + "/share/zoneinfo")) > + #t)) > + (add-after 'install 'relocate-elisp > + (lambda _ > + (mkdir-p (string-append site-lisp "/guix.d")) > + (rename-file (string-append site-lisp "/ledger-mode") > + (string-append site-lisp "/guix.d/ledger-mod= e")) > + #t)))))) It is also good to generate "ledger-autoloads.el" file. This allows a user to have "M-x ledger-mode" available by default without any additional settings. This can be done by calling: (emacs-generate-autoloads "ledger" "") However it is a bit tricky: 'emacs-generate-autoloads' procedure is placed in (guix build emacs-utils) module which is not available for cmake build system by default, so this module should be: 1) imported (via #:imported-modules argument), i.e. available for using 2) used (via #:modules argument). Overall, here is how I would write 'arguments' field of this package: (arguments `(#:modules ((guix build cmake-build-system) (guix build utils) (guix build emacs-utils)) #:imported-modules (,@%cmake-build-system-modules (guix build emacs-utils)) #:configure-flags `("-DBUILD_DOCS:BOOL=3DON" "-DBUILD_WEB_DOCS:BOOL=3DON" "-DBUILD_EMACSLISP:BOOL=3DON" "-DUSE_PYTHON:BOOL=3DON" "-DCMAKE_INSTALL_LIBDIR:PATH=3Dlib" ,(string-append "-DUTFCPP_INCLUDE_DIR:PATH=3D" (assoc-ref %build-inputs "utfcpp") "/include")) #:parallel-build? #f ; needed? #:phases (modify-phases %standard-phases (add-before 'configure 'install-examples (lambda* (#:key outputs #:allow-other-keys) (let ((examples (string-append (assoc-ref outputs "out") "/share/doc/ledger/examples"))) (install-file "test/input/sample.dat" examples) (install-file "test/input/demo.ledger" examples)) #t)) (add-after 'build 'build-doc (lambda _ (zero? (system* "make" "doc")))) (add-before 'check 'check-setup ;; One test fails if it can't set the timezone. (lambda* (#:key inputs #:allow-other-keys) (setenv "TZDIR" (string-append (assoc-ref inputs "tzdata") "/share/zoneinfo")) #t)) (add-after 'install 'relocate-elisp (lambda* (#:key outputs #:allow-other-keys) (let* ((site-dir (string-append (assoc-ref outputs "out") "/share/emacs/site-lisp")) (guix-dir (string-append site-dir "/guix.d")) (orig-dir (string-append site-dir "/ledger-mode")) (dest-dir (string-append guix-dir "/ledger-mode"))) (mkdir-p guix-dir) (rename-file orig-dir dest-dir) (emacs-generate-autoloads ,name dest-dir)) #t))))) The rest looks good to me, so if there will be no other comments, I will commit it (without =E2=80=98#:parallel-build? #f=E2=80=99). > + (inputs `(("boost" ,boost) > + ("gmp" ,gmp) > + ("libedit" ,libedit) > + ("mpfr" ,mpfr) > + ("python" ,python-2) > + ("tzdata" ,tzdata) > + ("utfcpp" ,utfcpp))) > + (native-inputs `(("emacs-no-x" ,emacs-no-x) > + ("groff" ,groff) > + ("texinfo" ,texinfo))) > + (home-page "http://ledger-cli.org/") > + (synopsis "Command-line double-entry accounting program") > + (description > + "Ledger is a powerful, double-entry accounting system that is > +accessed from the UNIX command-line. This may put off some users, since > +there is no flashy UI, but for those who want unparalleled reporting > +access to their data there are few alternatives. > + > +Ledger uses text files for input. It reads the files and generates > +reports; there is no other database or stored state. To use Ledger, > +you create a file of your account names and transactions, run from the > +command line with some options to specify input and requested reports, a= nd > +get output. The output is generally plain text, though you could genera= te > +a graph or html instead. Ledger is simple in concept, surprisingly rich > +in ability, and easy to use.") > + ;; There are some extra licenses in files which do not presently get > + ;; installed when you build this package. Different versions of the= GPL > + ;; are used in the contrib and python subdirectories. The bundled v= ersion > + ;; of utfcpp is under the Boost 1.0 license. Also the file > + ;; `tools/update_copyright_year` has an Expat license. > + (license (list license:bsd-3 > + license:asl2.0 ; src/strptime.cc > + (license:non-copyleft > + "file://src/wcwidth.cc" > + "See src/wcwidth.cc in the distribution.") > + license:gpl2+)))) ; lisp/* --=20 Alex