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 ‘#:parallel-build? #f’ be used in the 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 <a...@ajgrf.com> > 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/archive/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=ON" > + "-DBUILD_WEB_DOCS:BOOL=ON" > + "-DBUILD_EMACSLISP:BOOL=ON" > + "-DUSE_PYTHON:BOOL=ON" > + "-DCMAKE_INSTALL_LIBDIR:PATH=lib" > + ,(string-append "-DUTFCPP_INCLUDE_DIR:PATH=" > + (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-mode")) > + #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" "<elisp-dir>") 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=ON" "-DBUILD_WEB_DOCS:BOOL=ON" "-DBUILD_EMACSLISP:BOOL=ON" "-DUSE_PYTHON:BOOL=ON" "-DCMAKE_INSTALL_LIBDIR:PATH=lib" ,(string-append "-DUTFCPP_INCLUDE_DIR:PATH=" (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 ‘#:parallel-build? #f’). > + (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, and > +get output. The output is generally plain text, though you could generate > +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 > version > + ;; 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/* -- Alex
