Re: [PATCH] gnu: Add minced.

[Ben Woodcroft <b.woodcroft@uq.edu.au>]

[-- Attachment #1: Type: text/plain, Size: 2986 bytes --]

Hi Marius,

Excellent to see others interested in packaging microbial bioinformatics 
tools.


I tried this in a container and it seems that it calls out to a few 
programs preventing it from working:

[env]# minced -h
/gnu/store/8asw2i54x53rrwdr8qw4j2rpbkc9fqzz-profile/bin/minced: line 7: 
dirname: command not found
/gnu/store/8asw2i54x53rrwdr8qw4j2rpbkc9fqzz-profile/bin/minced: line 9: 
dirname: command not found
/gnu/store/8asw2i54x53rrwdr8qw4j2rpbkc9fqzz-profile/bin/minced: line 11: 
basename: command not found

So I think that (in order of preference), the source files themselves 
should be patched with the absolute paths to these tools, the binary 
should be wrapped, or coreutils should be propagated. For the first two 
options, coreutils should be an input.

Now some small comments on the patch itself.

> +    (arguments
> +     `(#:test-target "test"
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (add-before 'check 'fix-test
> +           (lambda _
> +             ;; Fix test for latest version.
> +             (substitute* "t/Aquifex_aeolicus_VF5.expected"
> +               (("minced:0.1.6") "minced:0.2.0"))))
It might be more future-proof to use '(string-append "minced:" 
,version)' instead of hard-coding 0.2.0.

Also, this phase (and the next two) should end in #t since the return 
value of substitute* is undefined.


> +         (add-before 'install 'qualify-java-path
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (substitute* "minced"
> +               ;; Set full path to java binary in wrapper script.
> +               (("^java") (string-append (assoc-ref inputs "jre")
> +                                         "/bin/java")))))
> +         (replace 'install
> +           ;; No install target.
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (bin (string-append out "/bin")))
> +               (for-each (lambda (file)
> +                           (install-file file bin))
> +                         (list "minced" "minced.jar"))))))))
> +    (native-inputs
> +     `(("jdk", icedtea "jdk")))
> +    (inputs
> +     `(("jre", icedtea)))
The commas should go after the space.


> +    (home-page"https://github.com/ctSkennerton/minced")
> +    (synopsis "Mining CRISPRs in Environmental Datasets")
> +    (description
> +     "MinCED is a program to find Clustered Regularly Interspaced Short
> +Palindromic Repeats (CRISPRs) in full genomes or environmental datasets such
> +as metagenomes, in which sequence size can be anywhere from 100 to 800 bp.")
That description which you took from the README is a little dated at the 
end. How about this?

"MinCED is a program to find Clustered Regularly Interspaced Short
Palindromic Repeats (CRISPRs) in both full genomes and shorter metagenomic sequences."

The rest LGTM. Thanks. Can you send an updated patch please?

ben

[-- Attachment #2: Type: text/html, Size: 3692 bytes --]