guix-devel
[Top][All Lists]
Advanced

[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: Thu, 12 May 2016 12:12:45 +0300
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Alex Griffin (2016-05-12 06:28 +0300) wrote:

> Here's an updated patch with the suggested changes. I also re-enabled
> the shared library and python module, and built html docs, and a few
> other things. This turned out to be a much more challenging package than
> I expected! Hopefully it is finally finished or very close to finished.

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).

> From e6fb89828f44765d36a658bd2b497aa8bd8b12d9 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
> +     `(#: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
> +           (add-before 'configure 'install-examples
> +             (lambda _
> +               (install-file "test/input/sample.dat" examples)
> +               (install-file "test/input/demo.ledger" examples)))

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.

> +           (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 _ (setenv "TZDIR"
> +                               (string-append (assoc-ref %build-inputs 
> "tzdata")
> +                                              "/share/zoneinfo"))))

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:

             (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"))))))

(Sorry, the following comment is not related to this patch)

This is another point for us to get rid of "guix.d" part: it is a common
practice for packages to put elisp files in a sub-directory of
"share/emacs/site-lisp/" (at least mu, magit and git-modes do this as
well).

It seems natural for me if our emacs will look in "site-lisp/<package>"
instead of "site-lisp/guix.d/<package>", so there will be no need to
adjust such packages to move elisp files.

So I beg people to answer to
<http://lists.gnu.org/archive/html/guix-devel/2016-05/msg00296.html>.
"guix.d" part is a redundant level in file hierarchy, let's remove it!

> +       #: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"))))

It doesn't matter but usually we put #:configure-flags before #:phases.

Attachment: guix-ledger-build-fail.log
Description: Binary data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]