Re: [PATCH] Add Blast+.

[Mark H Weaver]

Ricardo Wurmus <address@hidden> writes:

>> It would be great to get the tests working, even if we have to disable
>> some of them.  Otherwise we have no way of knowing that we're not
>> distributing broken garbage :)
>
> It's hard to know in this case even with the tests, because it's only
> three tests and they seem hardly representative of the library.

Okay, fair enough.

>> Is everything in here really in the public domain?  I'd guess that in
>> order to make this true, you'd need to remove bzip2 and zlib in a
>> snippet, and even then I'd doubtful :)
>
> I've moved the code to remove the bundled stuff to a snippet.
>
> The NCBI code was released into the public domain.  However, it appears
> that some third-party headers and some build scripts are under a
> different license:
>
>   * Expat:
>     * ncbi-blast-2.2.30+-src/c++/include/util/bitset/
>     * ncbi-blast-2.2.30+-src/c++/src/html/ncbi_menu*.js
>   * Boost license:
>     * 
> ncbi-blast-2.2.30+-src/c++/include/util/impl/floating_point_comparison.hpp
>   * LGPL 2+:
>     * ncbi-blast-2.2.30+-src/c++/include/dbapi/driver/odbc/unix_odbc/
>   * ASL 2.0:
>     * ncbi-blast-2.2.30+-src/c++/src/corelib/teamcity_*
>
> I could not find mention of any other licenses.  Is this correct, then:
>
>     ;; Most of the sources are in the public domain, with the following 
> exceptions:
>     ;; ...(the above list)...
>     (license (list license:public-domain
>                    license:expat
>                    license:boost1.0
>                    license:lgpl2.0+
>                    license:asl2.0))

Looks good, thanks!  See below for more comments on the patch.

> From 8d131f66ba0378738e5b837f78c411edb241d35a Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <address@hidden>
> Date: Tue, 16 Jun 2015 16:24:24 +0200
> Subject: [PATCH] gnu: Add Blast+.
>
> * gnu/packages/bioinformatics.scm (blast+): New variable.
> ---
>  gnu/packages/bioinformatics.scm | 155 
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 155 insertions(+)
>
> diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
> index 3defac8..4603329 100644
> --- a/gnu/packages/bioinformatics.scm
> +++ b/gnu/packages/bioinformatics.scm
> @@ -31,6 +31,7 @@
>    #:use-module (gnu packages base)
>    #:use-module (gnu packages boost)
>    #:use-module (gnu packages compression)
> +  #:use-module (gnu packages cpio)
>    #:use-module (gnu packages file)
>    #:use-module (gnu packages java)
>    #:use-module (gnu packages linux)
> @@ -258,6 +259,160 @@ into separate processes; and more.")
>      (inputs
>       `(("python2-numpy" ,python2-numpy)))))
>  
> +(define-public blast+
> +  (package
> +    (name "blast+")
> +    (version "2.2.30")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "ftp://ftp.ncbi.nlm.nih.gov/blast/executables/blast+/";
> +                    version "/ncbi-blast-" version "+-src.tar.gz"))
> +              (sha256
> +               (base32
> +                "0h0fj5cpx6zpfwixgx5f5xbr4rn3cnai0x3j7grrg50vr18jvxr6"))
> +              (modules '((guix build utils)))
> +              (snippet
> +               '(begin
> +                  ;; Remove bundled bzip2 and zlib
> +                  (delete-file-recursively "c++/src/util/compress/bzip2")
> +                  (delete-file-recursively "c++/src/util/compress/zlib")
> +                  (substitute* "c++/src/util/compress/Makefile.in"
> +                    (("bzip2 zlib api") "api"))
> +                  ;; Remove useless msbuild directory
> +                  (delete-file-recursively
> +                   "c++/src/build-system/project_tree_builder/msbuild")))))

As with phase procedures, the snippet should return a boolean, so please
add #t here.

[...]
> +            ;; Proceed even though the weird build system says that generated
> +            ;; files are out of date
> +            (setenv "NCBICXX_RECONF_POLICY" "warn")
> +
> +            ;; Rewrite hardcoded paths to various tools
> +            (substitute* (append (find-files "scripts/common/check" "\\.sh$")
> +                                 '("scripts/common/impl/if_diff.sh"
> +                                   "scripts/common/impl/run_with_lock.sh"
> +                                   
> "src/build-system/Makefile.configurables.real"
> +                                   "src/build-system/Makefile.in.top"
> +                                   "src/build-system/Makefile.meta.gmake=no"
> +                                   "src/build-system/Makefile.meta.in"
> +                                   "src/build-system/Makefile.meta_l"
> +                                   "src/build-system/Makefile.meta_p"
> +                                   "src/build-system/Makefile.meta_r"
> +                                   "src/build-system/Makefile.mk.in"
> +                                   "src/build-system/Makefile.requirements"
> +                                   
> "src/build-system/Makefile.rules_with_autodep.in"
> +                                   "src/build-system/configure"
> +                                   "src/build-system/configure.ac"))
> +              (("(/usr/bin/|/bin/)([a-z][-_.a-z]*)" all dir cmd)
> +               (or (which* cmd) all)))

It would be better to patch configure.ac first, and then configure, and
then the others.  As you have it here, the modification times will
indicate that 'configure' is older than 'configure.ac' and thus out of
date.  Perhaps this is why you needed to setenv NCBICXX_RECONF_POLICY?

Otherwise, the patch looks great, thanks!

     Mark