Re: [PATCH] Add vcflib.

[Ricardo Wurmus]

Roel Janssen <address@hidden> writes:
> From 07f041559f7f023c56c23f81ac7e90441c44f91b Mon Sep 17 00:00:00 2001
> From: Roel Janssen <address@hidden>
> Date: Tue, 22 Mar 2016 16:06:45 +0100
> Subject: [PATCH 8/8] gnu: Add vcflib.

> * gnu/packages/bioinformatics.scm (tabixpp-vcflib): New variable.
> * gnu/packages/bioinformatics.scm (vcflib): New variable.

These should be two separate patches.
 
> +;; This version works with FreeBayes while the released version doesn't. The
> +;; release creates a variable with the name "vcf" somewhere, which is also 
> the
> +;; name of a namespace in vcflib.

Is there an upstream bugreport about this?  If so it would be good to
link to it.

> +(define-public tabixpp-vcflib
> +  (let ((commit "bbc63a49acc52212199f92e9e3b8fba0a593e3f7"))
> +    (package (inherit tabixpp)
> +      (name "tabixpp-vcflib")
> +      (version (string-append "0-1." (string-take commit 7)))
> +      (source (origin
> +        (method url-fetch)
> +        (uri (string-append "https://github.com/ekg/tabixpp/archive/";
> +                            commit ".tar.gz"))
> +        (file-name (string-append name "-" version
> "-checkout.tar.gz"))

No need to append “-checkout”.

> +        (sha256
> +         (base32 
> "1s06wmpgj4my4pik5kp2lc42hzzazbp5ism2y4i2ajp2y1c68g77")))))))
> +
>  (define-public smithwaterman
>    (let ((commit "203218b47d45ac56ef234716f1bd4c741b289be1"))
>      (package
> @@ -5017,4 +5033,82 @@ automatically handles index file generation and use.")
>  tree using C++ templates, allowing the insertion of arbitrary types into the
>  tree.")
>        (license license:expat))))
> +
> +(define-public vcflib
> +  (let ((commit "5ac091365fdc716cc47cc5410bb97ee5dc2a2c92"))
> +    (package
> +      (name "vcflib")
> +      (version (string-append "0-1." (string-take commit 7)))
> +      (source (origin
> +         (method url-fetch)
> +         (uri (string-append "https://github.com/vcflib/vcflib/archive/";
> +                commit ".tar.gz"))

The indentation is off here.  “(method” is way too far to the left.
Also “commit” is misaligned.

> +         (file-name (string-append name "-" version ".tar.gz"))
> +         (sha256
> +          (base32 "0ywshwpif059z5h0g7zzrdfzzdj2gr8xvwlwcsdxrms3p9iy35h8"))))
> +      (build-system gnu-build-system)
> +      (inputs
> +       `(("intervaltree" ,intervaltree)
> +         ("htslib" ,htslib)
> +         ("zlib" ,zlib)))
> +      (native-inputs
> +       `(("python" ,python-2)
> +         ("perl" ,perl)
> +         ("r" ,r)
> +         ("node" ,node)
> +         ("tabixpp-src" ,(package-source tabixpp-vcflib))
> +         ("smithwaterman-src" ,(package-source smithwaterman))
> +         ("multichoose-src" ,(package-source multichoose))
> +         ("fsom-src" ,(package-source fsom))
> +         ("filevercmp-src" ,(package-source filevercmp))
> +         ("fastahack-src" ,(package-source fastahack))))

Not a fan of this, but I cannot think of a better way to do this.

> +      (arguments
> +       `(#:tests? #f ; There are no tests to run.
> +         #:phases
> +         (modify-phases %standard-phases
> +           (delete 'configure)
> +           (add-after 'unpack 'unpack-submodule-sources
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               (let ((unpack (lambda (source target)
> +                               (with-directory-excursion target
> +                                 (zero? (system* "tar" "xvf"
> +                                        (assoc-ref inputs source)
> +                                        "--strip-components=1"))))))
> +                 (and
> +                  (unpack "fastahack-src" "fastahack")
> +                  (unpack "filevercmp-src" "filevercmp")
> +                  (unpack "fsom-src" "fsom")
> +                  (unpack "multichoose-src" "multichoose")
> +                  (unpack "smithwaterman-src" "smithwaterman")
> +                  (unpack "tabixpp-src" "tabixpp")))))
> +           (add-after 'unpack-submodule-sources 'fix-makefile
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               (substitute* '("Makefile")
> +                 (("^GIT_VERSION.*") "GIT_VERSION = v1.0.0"))))

What is this for?  Where does this version number come from?  Should it
match this package’s version?

> +           (replace 'build
> +            (lambda* (#:key inputs make-flags #:allow-other-keys)
> +              (with-directory-excursion "tabixpp"
> +                (zero? (system* "make")))
> +              (zero? (system* "make" "CC=gcc"
> +                (string-append
> +                  "CFLAGS=\"" "-Itabixpp "
> +                  "-I" (assoc-ref inputs "htslib") "/include "
> +                  "-I" (assoc-ref inputs "intervaltree") "/include "
> +                  "\"") "all"))))

Instead of replacing the build phase try to use make-flags.

> +           (replace 'install
> +            (lambda* (#:key outputs #:allow-other-keys)
> +              (let* ((out (assoc-ref outputs "out"))
> +                     (bin (string-append out "/bin"))
> +                     (lib (string-append out "/lib")))
> +                (for-each (lambda (file)
> +                            (install-file file bin))
> +                          (find-files "bin" ".*"))
> +                (install-file "libvcflib.a" lib)))))))

Remember to return “#t”.

> +      (home-page "https://github.com/vcflib/vcflib/";)
> +      (synopsis "Library for parsing and manipulating VCF files")
> +      (description "Vcflib provides methods to manipulate and interpret
> +sequence variation as it can be described by VCF. It is both an API for 
> parsing
> +and operating on records of genomic variation as it can be described by the 
> VCF
> +format, and a collection of command-line utilities for executing complex
> +manipulations on VCF files.")

Please use double-spacing between sentences.  I wonder what it means
that “Vcflib ... is ... an API”.  I think this should say “It offers an
API for parsing” or “It is a library for parsing”.

> +      (license license:expat))))

There are no license headers in most (all?) of the files.  There’s a bug
report for that:

   https://github.com/vcflib/vcflib/issues/16

I think this is not a problem, though, as the README file says that the
“MIT” license applies to the project.

~~ Ricardo