guix-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] gnu: Add minced.


From: Ben Woodcroft
Subject: Re: [PATCH] gnu: Add minced.
Date: Tue, 16 Aug 2016 14:59:59 +1000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0

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

reply via email to

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