[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] gnu: Add ledger.
From: |
Alex Kost |
Subject: |
Re: [PATCH 2/3] gnu: Add ledger. |
Date: |
Fri, 13 May 2016 22:16:29 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
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 <address@hidden>
> 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
- Re: [PATCH 2/3] gnu: Add ledger., (continued)
- Re: [PATCH 2/3] gnu: Add ledger., Alex Griffin, 2016/05/07
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/07
- Re: [PATCH 2/3] gnu: Add ledger., Alex Griffin, 2016/05/07
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/08
- Re: [PATCH 2/3] gnu: Add ledger., Ludovic Courtès, 2016/05/09
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/09
- Re: [PATCH 2/3] gnu: Add ledger., Alex Griffin, 2016/05/11
- Re: [PATCH 2/3] gnu: Add ledger., Alex Kost, 2016/05/12
- Re: [PATCH 2/3] gnu: Add ledger., Alex Griffin, 2016/05/12
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/12
- Re: [PATCH 2/3] gnu: Add ledger.,
Alex Kost <=
- Re: [PATCH 2/3] gnu: Add ledger., Alex Griffin, 2016/05/13
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/13
- Re: [PATCH 2/3] gnu: Add ledger., Alex Kost, 2016/05/16
- Re: [PATCH 2/3] gnu: Add ledger., Christopher Allan Webber, 2016/05/16
- Re: [PATCH 2/3] gnu: Add ledger., Efraim Flashner, 2016/05/16
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/16
- Re: [PATCH 2/3] gnu: Add ledger., Leo Famulari, 2016/05/16