guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Add FastTree


From: Alex Kost
Subject: Re: [PATCH] Add FastTree
Date: Fri, 19 Jun 2015 13:13:08 +0300
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Ben Woodcroft (2015-06-19 10:24 +0300) wrote:

> Hi,

Hi, I didn't try your patch, so I don't have real comments, just some
general cosmetic notes, if you don't mind.

[...]
> +   (source (origin
> +             (method url-fetch)
> +             (uri (string-append
> +                   "http://www.microbesonline.org/fasttree/FastTree-";
> +                   version
> +                   ".c"
> +                   ))
Please put it on one line:

                      version ".c"))

> +             (sha256
> +              (base32
> +               "0dzqc9vr9iiiw21y159xfjl2z90vw0y7r4x6456pcaxiy5hd2wmi"))))
> +   (build-system trivial-build-system)
> +   (arguments
> +    '(#:modules ((guix build utils))
> +      #:builder
> +      (begin
> +        (use-modules (guix build utils) (system base compile))
> +        (let ((source (assoc-ref %build-inputs "source"))
> +              (gcc (assoc-ref %build-inputs "gcc"))
> +              (glibc (assoc-ref %build-inputs "glibc"))
> +              (binutils (assoc-ref %build-inputs "binutils"))
> +              (out    (assoc-ref %outputs "out")))          
Please remove trailing spaces                        ^^^^^^^^^^

And it would be more readable to align this 'let':

        (let ((source   (assoc-ref %build-inputs "source"))
              (gcc      (assoc-ref %build-inputs "gcc"))
              (glibc    (assoc-ref %build-inputs "glibc"))
              (binutils (assoc-ref %build-inputs "binutils"))
              (out      (assoc-ref %outputs "out")))


> +          (setenv "PATH" (string-append binutils "/bin:" gcc "/bin"))
> +          (setenv "LIBRARY_PATH" (string-append glibc "/lib"))
> +          (let ((bin (string-append out "/bin")))
> +            (mkdir-p bin)
> +            (system* "gcc"
> +                     "-O3"
> +                     "-finline-functions"
> +                     "-funroll-loops"
> +                     "-Wall"
> +                     "-o"
> +                     (string-append bin "/FastTree")
> +                     source
> +                     "-lm")
> +            (system* "gcc"
> +                     "-DOPENMP"
> +                     "-fopenmp"
> +                     "-O3"
> +                     "-finline-functions"
> +                     "-funroll-loops"
> +                     "-Wall"
> +                     "-o"
> +                     (string-append bin "/FastTreeMP")
> +                     source
> +                     "-lm"))))))
> +   (native-inputs
> +    `(("gcc", gcc-5.1)
> +      ("binutils" ,binutils)
> +      ("glibc" ,glibc)))
> +   (home-page "http://www.microbesonline.org/fasttree";)
> +   (synopsis "FastTree infers approximately-maximum-likelihood
> +phylogenetic trees from alignments of nucleotide or protein sequences")

Synopsis is too long; it should be a one-liner.  You may use "guix lint
fasttree" to perform some common checks.

-- 
Alex



reply via email to

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