[PATCH] Add MAFFT.

[PATCH] Add MAFFT.

[Ben Woodcroft]

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

Note that the "extensions" listed on the website are research/non-profit 
only, and so not fit for inclusion.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-mafft.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-mafft.patch", Size: 3797 bytes --]

From b966b0c8ab9b194bbb7089cc4b4e3a9727862691 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Thu, 23 Jul 2015 22:42:17 +1000
Subject: [PATCH] gnu: Add mafft.

* gnu/packages/bioinformatics.scm (mafft): New variable
---
 gnu/packages/bioinformatics.scm | 57 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index 78cd3ce..a09b89e 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -1394,6 +1394,63 @@ resolution of binding sites through combining the information of both
 sequencing tag position and orientation.")
     (license license:bsd-3)))
 
+(define-public mafft
+  (package
+    (name "mafft")
+    (version "7.221")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "http://mafft.cbrc.jp/alignment/software/mafft-" version
+                    "-without-extensions-src.tgz"))
+              (file-name (string-append name "-" version ".tgz"))
+              (sha256
+               (base32
+                "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; no automated tests, though there are tests in the readme
+       #:make-flags (list (string-append "PREFIX=" (assoc-ref %outputs "out"))
+                          (string-append "BINDIR=" (string-append
+                                                    (assoc-ref %outputs "out")
+                                                    "/bin")))
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'enter-dir
+                    (lambda _ (chdir "core") #t))
+         ;; on advice from the MAFFT authors, do not distribute mafft-profile,
+         ;; mafft-distance mafft-homologs.rb
+         (add-after 'enter-dir 'patch-makefile
+                    (lambda _
+                      (substitute* "Makefile"
+                        (("^SCRIPTS = mafft mafft-homologs.rb")
+                         "SCRIPTS = mafft")
+                        (((string-append "^PROGS = dvtditr dndfast7 dndblast"
+                                         " sextet5 mafft-distance"))
+                         "PROGS = dvtditr dndfast7 dndblast sextet5")
+                        (((string-append "splittbfast disttbfast tbfast "
+                                         "mafft-profile f2cl mccaskillwrap"))
+                         "splittbfast disttbfast tbfast f2cl mccaskillwrap")
+                        (("^rm -f mafft-profile mafft-profile.exe") "#")
+                        (("^rm -f mafft-distance mafft-distance.exe") ")#")
+                        ;; do not install MAN pages in libexec folder
+                        (((string-append "^\t\\$\\(INSTALL\\) -m 644 \\$\\("
+                                         "MANPAGES\\) \\$\\(DESTDIR\\)\\$"
+                                         "\\(LIBDIR\\)")) "#"))
+                      #t))
+         (delete 'configure))))
+    (inputs
+     `(("perl" ,perl)))
+    (home-page "http://mafft.cbrc.jp/alignment/software/")
+    (synopsis
+     "Multiple sequence alignment program for unix-like operating systems")
+    (description
+     "MAFFT offers a range of multiple alignment methods for nucleotide and
+protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
+of <∼200 sequences) and FFT-NS-2 (fast; for alignment of <∼30,000
+sequences).")
+   (license (license:non-copyleft "file://license"
+                                  "See license in the distribution."))))
 
 (define-public metabat
   (package
-- 
2.4.3

Re: [PATCH] Add MAFFT.

[Ricardo Wurmus]

Hi Ben,

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'enter-dir
> +                    (lambda _ (chdir "core") #t))
> +         ;; on advice from the MAFFT authors, do not distribute mafft-profile,
> +         ;; mafft-distance mafft-homologs.rb

What does this mean?  Are they non-free software?  If so, they should be
removed in a snippet.

> +         (add-after 'enter-dir 'patch-makefile
> +                    (lambda _
> +                      (substitute* "Makefile"
> +                        (("^SCRIPTS = mafft mafft-homologs.rb")
> +                         "SCRIPTS = mafft")
> +                        (((string-append "^PROGS = dvtditr dndfast7 dndblast"
> +                                         " sextet5 mafft-distance"))
> +                         "PROGS = dvtditr dndfast7 dndblast sextet5")
> +                        (((string-append "splittbfast disttbfast tbfast "
> +                                         "mafft-profile f2cl mccaskillwrap"))
> +                         "splittbfast disttbfast tbfast f2cl mccaskillwrap")

This line looks particularly unclear.  It took me a little too long to
find the difference in the replacement.  Is there a nicer regular
expression that you could use to indicate the removal of
“mafft-profile”?

> +                        (("^rm -f mafft-profile mafft-profile.exe") "#")
> +                        (("^rm -f mafft-distance mafft-distance.exe") ")#")
> +                        ;; do not install MAN pages in libexec folder
> +                        (((string-append "^\t\\$\\(INSTALL\\) -m 644 \\$\\("
> +                                         "MANPAGES\\) \\$\\(DESTDIR\\)\\$"
> +                                         "\\(LIBDIR\\)")) "#"))
> +                      #t))
> +         (delete 'configure))))
> +    (inputs
> +     `(("perl" ,perl)))
> +    (home-page "http://mafft.cbrc.jp/alignment/software/")
> +    (synopsis
> +     "Multiple sequence alignment program for unix-like operating systems")

Please remove “for unix-like operating systems”.

> +    (description
> +     "MAFFT offers a range of multiple alignment methods for nucleotide and
> +protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
> +of <∼200 sequences) and FFT-NS-2 (fast; for alignment of <∼30,000
> +sequences).")
> +   (license (license:non-copyleft "file://license"
> +                                  "See license in the distribution."))))

~~ Ricardo

Re: [PATCH] Add MAFFT.

[Ben Woodcroft]

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

On 26/07/15 05:26, Ricardo Wurmus wrote:
> Hi Ben,
>
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'unpack 'enter-dir
>> +                    (lambda _ (chdir "core") #t))
>> +         ;; on advice from the MAFFT authors, do not distribute mafft-profile,
>> +         ;; mafft-distance mafft-homologs.rb
> What does this mean?  Are they non-free software?  If so, they should be
> removed in a snippet.

Sorry for the confusion. MAFFT itself is all free software, but on the 
website it allows you to download a version "with-extensions" which 
includes non-free software.

I contacted the MAFFT authors and they suggested that there was no need 
to distribute these three programs in a mafft package. I've updated the 
comment to clarify.

I'm actually a bit confused as to the difference between a snippet vs. a 
patch stage as above - when is each appropriate? Snippets are only for 
removing non-free software?

>
>> +         (add-after 'enter-dir 'patch-makefile
>> +                    (lambda _
>> +                      (substitute* "Makefile"
>> +                        (("^SCRIPTS = mafft mafft-homologs.rb")
>> +                         "SCRIPTS = mafft")
>> +                        (((string-append "^PROGS = dvtditr dndfast7 dndblast"
>> +                                         " sextet5 mafft-distance"))
>> +                         "PROGS = dvtditr dndfast7 dndblast sextet5")
>> +                        (((string-append "splittbfast disttbfast tbfast "
>> +                                         "mafft-profile f2cl mccaskillwrap"))
>> +                         "splittbfast disttbfast tbfast f2cl mccaskillwrap")
> This line looks particularly unclear.  It took me a little too long to
> find the difference in the replacement.  Is there a nicer regular
> expression that you could use to indicate the removal of
> “mafft-profile”?

I've added comments and alignment.

ben

[-- Attachment #2: 0001-gnu-Add-mafft.patch --]
[-- Type: text/x-patch, Size: 5015 bytes --]

From ce50d7382c8f7f092faa56ff5316134417a9eca1 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Sun, 26 Jul 2015 13:01:01 +1000
Subject: [PATCH] gnu: Add mafft.

* gnu/packages/bioinformatics.scm (mafft): New variable
---
 gnu/packages/bioinformatics.scm | 59 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 54 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index 38f01a8..11ef454 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -54,6 +54,54 @@
   #:use-module (gnu packages xml)
   #:use-module (gnu packages zip))
 
+(define-public aragorn
+  (package
+    (name "aragorn")
+    (version "1.2.36")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "http://mbio-serv2.mbioekol.lu.se/ARAGORN/Downloads/aragorn"
+                    version ".tgz"))
+              (sha256
+               (base32
+                "1dg7jlz1qpqy88igjxd6ncs11ccsirb36qv1z01a0np4i4jh61mb"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; there are no tests
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (replace 'build
+                  (lambda _
+                    (zero? (system* "gcc"
+                                    "-O3"
+                                    "-ffast-math"
+                                    "-finline-functions"
+                                    "-o"
+                                    "aragorn"
+                                    (string-append "aragorn" ,version ".c")))))
+         (replace 'install
+                  (lambda* (#:key outputs #:allow-other-keys)
+                    (let* ((out (assoc-ref outputs "out"))
+                           (bin (string-append out "/bin"))
+                           (man (string-append out "/share/man/man1")))
+                      (mkdir-p bin)
+                      (copy-file "aragorn"
+                                 (string-append bin "/aragorn"))
+                      (mkdir-p man)
+                      (copy-file "aragorn.1"
+                                 (string-append man "/aragorn.1")))
+                    #t)))))
+    (home-page "http://mbio-serv2.mbioekol.lu.se/ARAGORN")
+    (synopsis "Detect tRNA, mtRNA and tmRNA genes in nucleotide sequences")
+    (description
+     "Aragorn identifies transfer RNA, mitochondrial RNA and
+transfer-messenger RNA from nucleotide sequences, based on homology to known
+tRNA consensus sequences and RNA structure.  It also outputs the secondary
+structure of the predicted RNA.")
+    (license license:gpl2)))
+
 (define-public bamtools
   (package
     (name "bamtools")
@@ -148,7 +196,7 @@ computational cluster.")
 (define-public bedtools
   (package
     (name "bedtools")
-    (version "2.22.0")
+    (version "2.24.0")
     (source (origin
               (method url-fetch)
               (uri (string-append "https://github.com/arq5x/bedtools2/archive/v"
@@ -156,7 +204,7 @@ computational cluster.")
               (file-name (string-append name "-" version ".tar.gz"))
               (sha256
                (base32
-                "16aq0w3dmbd0853j32xk9jin4vb6v6fgakfyvrsmsjizzbn3fpfl"))))
+                "0lnxrjvs3nnmb4bmskag1wg3h2hd80przz5q3xd0bvs7vyxrvpbl"))))
     (build-system gnu-build-system)
     (native-inputs `(("python" ,python-2)))
     (inputs `(("samtools" ,samtools)
@@ -1409,7 +1457,7 @@ sequencing tag position and orientation.")
                 "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
     (build-system gnu-build-system)
     (arguments
-     `(#:tests? #f ; no automated tests, though there are tests in the readme
+     `(#:tests? #f ; no automated tests, though there are tests in the read me
        #:make-flags (list (string-append "PREFIX=" (assoc-ref %outputs "out"))
                           (string-append "BINDIR=" (string-append
                                                     (assoc-ref %outputs "out")
@@ -1424,10 +1472,11 @@ sequencing tag position and orientation.")
                       ;; distribute mafft-profile, mafft-distance, or
                       ;; mafft-homologs.rb as they are too "specialised".
                       (substitute* "Makefile"
+                        ;; remove mafft-homologs.rb from SCRIPTS
                         (("^SCRIPTS = mafft mafft-homologs.rb")
                          "SCRIPTS = mafft")
-                        ;; remove mafft-distance from PROGS 
-                       (((string-append "^PROGS = dvtditr dndfast7 dndblast"
+                        ;; remove mafft-distance from PROGS
+                        (((string-append "^PROGS = dvtditr dndfast7 dndblast"
                                          " sextet5 mafft-distance"))
                          "PROGS = dvtditr dndfast7 dndblast sextet5")
                         ;; remove mafft-profile from PROGS
-- 
2.4.3

Re: [PATCH] Add MAFFT.

[Andreas Enge]

On Sun, Jul 26, 2015 at 01:10:14PM +1000, Ben Woodcroft wrote:
> I'm actually a bit confused as to the difference between a snippet vs. a
> patch stage as above - when is each appropriate? Snippets are only for
> removing non-free software?

I would use a snippet for something internal to Guix, or specific to the
way we handle things, while a patch is intended to be passed upstream (or
sometimes taken from upstream already) to correct a bug in the software.

Andreas

Re: [PATCH] Add MAFFT.

[Ricardo Wurmus]

Andreas Enge <andreas@enge.fr> writes:

> On Sun, Jul 26, 2015 at 01:10:14PM +1000, Ben Woodcroft wrote:
>> I'm actually a bit confused as to the difference between a snippet vs. a
>> patch stage as above - when is each appropriate? Snippets are only for
>> removing non-free software?
>
> I would use a snippet for something internal to Guix, or specific to the
> way we handle things, while a patch is intended to be passed upstream (or
> sometimes taken from upstream already) to correct a bug in the software.

It should be noted that changes performed by snippets directly affect
the resulting tarball produced by ‘guix build -S’.  Whenever you want
the sources for a package to be permanently modified for the users, a
snippet would be the right way to achieve that.

Re: [PATCH] Add MAFFT.

[Andreas Enge]

On Sun, Jul 26, 2015 at 11:43:38AM +0200, Andreas Enge wrote:
> I would use a snippet for something internal to Guix, or specific to the
> way we handle things, while a patch is intended to be passed upstream (or
> sometimes taken from upstream already) to correct a bug in the software.

Well, I am contradicting myself. Right now, I am creating a patch for some-
thing internal to Guix, just because it is much easier to do so than to
use a snippet. I think there are no fixed rules...

Andreas

Re: [PATCH] Add MAFFT.

[Ben Woodcroft]

On 26/07/15 20:29, Andreas Enge wrote:
> On Sun, Jul 26, 2015 at 11:43:38AM +0200, Andreas Enge wrote:
>> I would use a snippet for something internal to Guix, or specific to the
>> way we handle things, while a patch is intended to be passed upstream (or
>> sometimes taken from upstream already) to correct a bug in the software.
> Well, I am contradicting myself. Right now, I am creating a patch for some-
> thing internal to Guix, just because it is much easier to do so than to
> use a snippet. I think there are no fixed rules...
Thanks for the tips Andreas, Ricardo. Is anything amiss with the 
original patch?

ben

Re: [PATCH] Add MAFFT.

[Ludovic Courtès]

Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:

> On 26/07/15 20:29, Andreas Enge wrote:
>> On Sun, Jul 26, 2015 at 11:43:38AM +0200, Andreas Enge wrote:
>>> I would use a snippet for something internal to Guix, or specific to the
>>> way we handle things, while a patch is intended to be passed upstream (or
>>> sometimes taken from upstream already) to correct a bug in the software.
>> Well, I am contradicting myself. Right now, I am creating a patch for some-
>> thing internal to Guix, just because it is much easier to do so than to
>> use a snippet. I think there are no fixed rules...
> Thanks for the tips Andreas, Ricardo. Is anything amiss with the
> original patch?

It seems this message was left unanswered.  Andreas?

Ludo’.

Re: [PATCH] Add MAFFT.

[Andreas Enge]

On Tue, Aug 25, 2015 at 10:58:42PM +0200, Ludovic Courtès wrote:
> It seems this message was left unanswered.  Andreas?

I had a cursory look at the patch, here are some comments:

On Thu, Jul 23, 2015 at 10:43:31PM +1000, Ben Woodcroft wrote:
> +       #:make-flags (list (string-append "PREFIX=" (assoc-ref %outputs "out"))
> +                          (string-append "BINDIR=" (string-append
> +                                                    (assoc-ref %outputs "out")
> +                                                    "/bin")))

Would it make sense to include this into a 
  (let ((out (assoc-ref %outputs out))) ...)
?

> +         (add-after 'enter-dir 'patch-makefile
> +                    (lambda _
> +                      (substitute* "Makefile"
> +                        (("^SCRIPTS = mafft mafft-homologs.rb")
> +                         "SCRIPTS = mafft")
> +                        (((string-append "^PROGS = dvtditr dndfast7 dndblast"
> +                                         " sextet5 mafft-distance"))
> +                         "PROGS = dvtditr dndfast7 dndblast sextet5")

This "string-append" to concatenate two literal strings is difficult to read;
I would suggest to move the "(lambda" more to the left so that everything
fits into one line.

> +    (synopsis
> +     "Multiple sequence alignment program for unix-like operating systems")

Maybe drop "for unix-like operating systems"; instead, you could add
"for nucleotide and protein sequences" from the description.

> +protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
> +of <???200 sequences) and FFT-NS-2 (fast; for alignment of <???30,000

Is this an artefact of the mailer?

I did not check whether the package passes "guix lint".

Could you send an updated patch?

Andreas

Re: [PATCH] Add MAFFT.

[Ricardo Wurmus]

Andreas Enge <andreas@enge.fr> writes:

>> +protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
>> +of <???200 sequences) and FFT-NS-2 (fast; for alignment of <???30,000
>
> Is this an artefact of the mailer?

In the original patch I see this instead:

> +     "MAFFT offers a range of multiple alignment methods for nucleotide and
> +protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
> +of <∼200 sequences) and FFT-NS-2 (fast; for alignment of <∼30,000
> +sequences).")

It’s not a problem with the patch.

~~ Ricardo

Re: [PATCH] Add MAFFT.

[Ben Woodcroft]

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

On 07/09/15 18:55, Andreas Enge wrote:
> On Tue, Aug 25, 2015 at 10:58:42PM +0200, Ludovic Courtès wrote:
>> It seems this message was left unanswered.  Andreas?
> I had a cursory look at the patch, here are some comments:
>
> [..]
>> +protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
>> +of <???200 sequences) and FFT-NS-2 (fast; for alignment of <???30,000
> Is this an artefact of the mailer?
>
> I did not check whether the package passes "guix lint".
>
> Could you send an updated patch?
Attached - I was only able to remove some of the long string-append 
lines. Apologies for that second "patch" - not intentional. Thanks for 
the review. I've changed the character to a different "~" so hopefully 
more standard now. Guix lint doesn't complain.

ben

[-- Attachment #2: 0001-gnu-Add-mafft.patch --]
[-- Type: text/x-patch, Size: 3665 bytes --]

From 0796e8be05977e6d30eab787a1550bd8d1f12b2d Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Sat, 12 Sep 2015 22:10:20 +1000
Subject: [PATCH] gnu: Add mafft.

* gnu/packages/bioinformatics.scm (mafft): New variable.
---
 gnu/packages/bioinformatics.scm | 60 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index 03eb2df..e0b7e40 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -1543,6 +1543,66 @@ resolution of binding sites through combining the information of both
 sequencing tag position and orientation.")
     (license license:bsd-3)))
 
+(define-public mafft
+  (package
+    (name "mafft")
+    (version "7.221")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "http://mafft.cbrc.jp/alignment/software/mafft-" version
+                    "-without-extensions-src.tgz"))
+              (file-name (string-append name "-" version ".tgz"))
+              (sha256
+               (base32
+                "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; no automated tests, though there are tests in the read me
+       #:make-flags (let ((out (assoc-ref %outputs "out")))
+                      (list (string-append "PREFIX=" out)
+                            (string-append "BINDIR="
+                                           (string-append out "/bin"))))
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'enter-dir
+            (lambda _ (chdir "core") #t))
+         (add-after 'enter-dir 'patch-makefile
+            (lambda _
+              ;; on advice from the MAFFT authors, there is no need to
+              ;; distribute mafft-profile, mafft-distance, or
+              ;; mafft-homologs.rb as they are too "specialised".
+              (substitute* "Makefile"
+                ;; remove mafft-homologs.rb from SCRIPTS
+                (("^SCRIPTS = mafft mafft-homologs.rb")
+                  "SCRIPTS = mafft")
+                ;; remove mafft-distance from PROGS
+                (("^PROGS = dvtditr dndfast7 dndblast sextet5 mafft-distance")
+                  "PROGS = dvtditr dndfast7 dndblast sextet5")
+                ;; remove mafft-profile from PROGS
+                (((string-append "splittbfast disttbfast tbfast mafft-profile"
+                                 " f2cl mccaskillwrap"))
+                 "splittbfast disttbfast tbfast f2cl mccaskillwrap")
+                (("^rm -f mafft-profile mafft-profile.exe") "#")
+                (("^rm -f mafft-distance mafft-distance.exe") ")#")
+                ;; do not install MAN pages in libexec folder
+                (((string-append "^\t\\$\\(INSTALL\\) -m 644 \\$\\(MANPAGES"
+                                 "\\) \\$\\(DESTDIR\\)\\$\\(LIBDIR\\)"))
+                 "#"))
+              #t))
+         (delete 'configure))))
+    (inputs
+     `(("perl" ,perl)))
+    (home-page "http://mafft.cbrc.jp/alignment/software/")
+    (synopsis
+     "Multiple sequence alignment program")
+    (description
+     "MAFFT offers a range of multiple alignment methods for nucleotide and
+protein sequences.  For instance, it offers L-INS-i (accurate; for alignment
+of <~200 sequences) and FFT-NS-2 (fast; for alignment of <~30,000
+sequences).")
+   (license (license:non-copyleft "file://license"
+                                  "See license in the distribution."))))
 
 (define-public metabat
   (package
-- 
2.4.3

Re: [PATCH] Add MAFFT.

[Ludovic Courtès]

Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:

> From 0796e8be05977e6d30eab787a1550bd8d1f12b2d Mon Sep 17 00:00:00 2001
> From: Ben Woodcroft <donttrustben@gmail.com>
> Date: Sat, 12 Sep 2015 22:10:20 +1000
> Subject: [PATCH] gnu: Add mafft.
>
> * gnu/packages/bioinformatics.scm (mafft): New variable.

I’ll leave it to Ricardo, but two minor comments:

> +              (substitute* "Makefile"
> +                ;; remove mafft-homologs.rb from SCRIPTS
> +                (("^SCRIPTS = mafft mafft-homologs.rb")
> +                  "SCRIPTS = mafft")
> +                ;; remove mafft-distance from PROGS
> +                (("^PROGS = dvtditr dndfast7 dndblast sextet5 mafft-distance")
> +                  "PROGS = dvtditr dndfast7 dndblast sextet5")
> +                ;; remove mafft-profile from PROGS
> +                (((string-append "splittbfast disttbfast tbfast mafft-profile"
> +                                 " f2cl mccaskillwrap"))
> +                 "splittbfast disttbfast tbfast f2cl mccaskillwrap")
> +                (("^rm -f mafft-profile mafft-profile.exe") "#")
> +                (("^rm -f mafft-distance mafft-distance.exe") ")#")
> +                ;; do not install MAN pages in libexec folder
> +                (((string-append "^\t\\$\\(INSTALL\\) -m 644 \\$\\(MANPAGES"
> +                                 "\\) \\$\\(DESTDIR\\)\\$\\(LIBDIR\\)"))
> +                 "#"))

Maybe not a blocker, but I would really prefer to patterns to be literal
strings.  If they are too long, we can always split the literal over two
lines:

    "like \
this"

> +   (license (license:non-copyleft "file://license"
> +                                  "See license in the distribution."))))

I think this should be something like:

  (license (license:non-copyleft
             "http://mafft.cbrc.jp/alignment/software/license.txt"
             "BSD-3 with different formatting"))

Thanks,
Ludo’.

Re: [PATCH] Add MAFFT.

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:
>
>> From 0796e8be05977e6d30eab787a1550bd8d1f12b2d Mon Sep 17 00:00:00 2001
>> From: Ben Woodcroft <donttrustben@gmail.com>
>> Date: Sat, 12 Sep 2015 22:10:20 +1000
>> Subject: [PATCH] gnu: Add mafft.
>>
>> * gnu/packages/bioinformatics.scm (mafft): New variable.
>
> I’ll leave it to Ricardo, but two minor comments:

My apologies for the delay.  I flagged this email (along with a couple
of others) as important but it was drowned out by a stream of other
“important” emails... Bleh.

> Maybe not a blocker, but I would really prefer to patterns to be literal
> strings.  If they are too long, we can always split the literal over two
> lines:
>
>     "like \
> this"

I’ve corrected the indentation for the phases and split the string
literals where appropriate instead of using ‘string-append’.

>
>> +   (license (license:non-copyleft "file://license"
>> +                                  "See license in the distribution."))))
>
> I think this should be something like:
>
>   (license (license:non-copyleft
>              "http://mafft.cbrc.jp/alignment/software/license.txt"
>              "BSD-3 with different formatting"))

I replaced the license statement with your suggestion and pushed it.

Thank you, Ben, for the patch and your patience; thank you, Ludo, for
the reviews!

~~ Ricardo