Update mafft to 7.245.

Update mafft to 7.245.

[Ben Woodcroft]

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

Also had to fix the inputs. Hard not to notice these things in the 
environment container - worked well thanks.

[-- Attachment #2: 0001-gnu-mafft-Update-to-7.245.patch --]
[-- Type: text/x-patch, Size: 2039 bytes --]

From a540e28db46d7914e66f448ad49850f4365902cb Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Tue, 10 Nov 2015 22:10:39 +1000
Subject: [PATCH] gnu: mafft: Update to 7.245.

* gnu/packages/bioinformatics.scm (mafft): Update to 7.245.
Fix inputs.
---
 gnu/packages/bioinformatics.scm | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index f13e405..011ce78 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -40,6 +40,7 @@
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cpio)
   #:use-module (gnu packages file)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages java)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages machine-learning)
@@ -1688,7 +1689,7 @@ sequencing tag position and orientation.")
 (define-public mafft
   (package
     (name "mafft")
-    (version "7.221")
+    (version "7.245")
     (source (origin
               (method url-fetch)
               (uri (string-append
@@ -1697,7 +1698,7 @@ sequencing tag position and orientation.")
               (file-name (string-append name "-" version ".tgz"))
               (sha256
                (base32
-                "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
+                "0m1l7gdrmfxjw54d3a0g18w6rwqrra9w9zvxix7dcddw6d1qyir2"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f ; no automated tests, though there are tests in the read me
@@ -1731,8 +1732,11 @@ sequencing tag position and orientation.")
 \\$\\(DESTDIR\\)\\$\\(LIBDIR\\)") "#"))
             #t))
          (delete 'configure))))
-    (inputs
-     `(("perl" ,perl)))
+    (propagated-inputs
+     `(("perl" ,perl)
+       ("gawk" ,gawk)
+       ("coreutils" ,coreutils)
+       ("grep" ,grep)))
     (home-page "http://mafft.cbrc.jp/alignment/software/")
     (synopsis "Multiple sequence alignment program")
     (description
-- 
2.4.3

Re: Update mafft to 7.245.

[Ricardo Wurmus]

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

> Also had to fix the inputs.

In this case, please also mention these input changes in the commit
message.

>           (delete 'configure))))
> -    (inputs
> -     `(("perl" ,perl)))
> +    (propagated-inputs
> +     `(("perl" ,perl)
> +       ("gawk" ,gawk)
> +       ("coreutils" ,coreutils)
> +       ("grep" ,grep)))

Is it really necessary to propagate these inputs?  Or could mafft just
reference them by their full path?  It’s very inelegant to propagate
packages.

In the case of coreutils I’m not sure if this is needed at all.  Instead
of propagating these inputs I’d suggest adding a phase that patches the
sources with ‘(substitute* ...)’ such that any call to awk, grep, perl,
or the coreutils is prefixed with the full store path.

Then you don’t need to propagate inputs and users won’t run into
conflicts.

~~ Ricardo

Re: Update mafft to 7.245.

[Efraim Flashner]

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

On Tue, 10 Nov 2015 22:18:10 +1000
Ben Woodcroft <b.woodcroft@uq.edu.au> wrote:

> Also had to fix the inputs. Hard not to notice these things in the 
> environment container - worked well thanks.

Do they need to be propagated inputs? Did you try them as native-inputs?
Often that's enough to take care of it.

Also, instead of:
Fix inputs.
the commit message is more along the lines of:
  [inputs]: Move inputs to propagated-inputs and add missing dependencies.
but I normally have to check against older commit messages to see what others
are doing and I try to copy that.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Update mafft to 7.245.

[Ben Woodcroft]

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

Two reviews in record time, nice.

On 10/11/15 23:12, Efraim Flashner wrote:
> On Tue, 10 Nov 2015 22:18:10 +1000
> Ben Woodcroft <b.woodcroft@uq.edu.au> wrote:
>
>> Also had to fix the inputs. Hard not to notice these things in the
>> environment container - worked well thanks.
> Do they need to be propagated inputs? Did you try them as native-inputs?
> Often that's enough to take care of it.
The main program 'mafft' is actually a reasonably long shell script, 
which itself calls awk, grep, perl, etc. collectively many times (>100 I 
would guess). I think it is intended to be run where these programs are 
available. I could do as Ricardo suggests and run substitute* but this 
seems a bit error-prone and not very future-proof to me, especially when 
the shell script is difficult to exhaustively test. WDYT?

Note that mafft is probably not going to remain as a leaf because a 
number of bioinformatic tools not yet packaged rely on it.
> Also, instead of:
> Fix inputs.
> the commit message is more along the lines of:
>    [inputs]: Move inputs to propagated-inputs and add missing dependencies.
> but I normally have to check against older commit messages to see what others
> are doing and I try to copy that.
Hopefully the attached is a better attempt? I also added another small 
improvement, not distributing the manpage of a program that is excluded 
from distribution.

Thanks.



[-- Attachment #2: 0001-gnu-mafft-Update-to-7.245.patch --]
[-- Type: text/x-patch, Size: 2705 bytes --]

From e05c50d2b40bf3290ec247223b3504429e9ad44e Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Wed, 11 Nov 2015 08:04:55 +1000
Subject: [PATCH] gnu: mafft: Update to 7.245.

* gnu/packages/bioinformatics.scm (mafft): Update to 7.245.
[arguments]: Don't include mafft-homologs manpage.
[inputs]: Move inputs to propagated-inputs and add missing dependencies.
---
 gnu/packages/bioinformatics.scm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index f13e405..9ee669e 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -40,6 +40,7 @@
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cpio)
   #:use-module (gnu packages file)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages java)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages machine-learning)
@@ -1688,7 +1689,7 @@ sequencing tag position and orientation.")
 (define-public mafft
   (package
     (name "mafft")
-    (version "7.221")
+    (version "7.245")
     (source (origin
               (method url-fetch)
               (uri (string-append
@@ -1697,7 +1698,7 @@ sequencing tag position and orientation.")
               (file-name (string-append name "-" version ".tgz"))
               (sha256
                (base32
-                "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
+                "0m1l7gdrmfxjw54d3a0g18w6rwqrra9w9zvxix7dcddw6d1qyir2"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f ; no automated tests, though there are tests in the read me
@@ -1718,6 +1719,9 @@ sequencing tag position and orientation.")
               ;; remove mafft-homologs.rb from SCRIPTS
               (("^SCRIPTS = mafft mafft-homologs.rb")
                "SCRIPTS = mafft")
+              ;; remove mafft-homologs from MANPAGES
+              (("^MANPAGES = mafft.1 mafft-homologs.1")
+               "MANPAGES = mafft.1")
               ;; remove mafft-distance from PROGS
               (("^PROGS = dvtditr dndfast7 dndblast sextet5 mafft-distance")
                "PROGS = dvtditr dndfast7 dndblast sextet5")
@@ -1731,8 +1735,11 @@ sequencing tag position and orientation.")
 \\$\\(DESTDIR\\)\\$\\(LIBDIR\\)") "#"))
             #t))
          (delete 'configure))))
-    (inputs
-     `(("perl" ,perl)))
+    (propagated-inputs
+     `(("perl" ,perl)
+       ("gawk" ,gawk)
+       ("coreutils" ,coreutils)
+       ("grep" ,grep)))
     (home-page "http://mafft.cbrc.jp/alignment/software/")
     (synopsis "Multiple sequence alignment program")
     (description
-- 
2.4.3

Re: Update mafft to 7.245.

[Ricardo Wurmus]

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


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

> Two reviews in record time, nice.
>
> On 10/11/15 23:12, Efraim Flashner wrote:
>> On Tue, 10 Nov 2015 22:18:10 +1000
>> Ben Woodcroft <b.woodcroft@uq.edu.au> wrote:
>>
>>> Also had to fix the inputs. Hard not to notice these things in the
>>> environment container - worked well thanks.
>> Do they need to be propagated inputs? Did you try them as native-inputs?
>> Often that's enough to take care of it.
> The main program 'mafft' is actually a reasonably long shell script, 
> which itself calls awk, grep, perl, etc. collectively many times (>100 I 
> would guess). I think it is intended to be run where these programs are 
> available. I could do as Ricardo suggests and run substitute* but this 
> seems a bit error-prone and not very future-proof to me, especially when 
> the shell script is difficult to exhaustively test. WDYT?

I looked at the instances of “perl”, “awk”, and “grep” and they seem
manageable.  I didn’t test this but attached is a different version of
your patch that does what I suggested.  It might work.  Could you try to
confirm that it’s okay?

What do you think?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-mafft-Update-to-7.245.patch --]
[-- Type: text/x-patch, Size: 3159 bytes --]

From 7f52cd8576df7cb4355d3281ca0d00fc661a0221 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Wed, 11 Nov 2015 08:04:55 +1000
Subject: [PATCH] gnu: mafft: Update to 7.245.

* gnu/packages/bioinformatics.scm (mafft): Update to 7.245.
[arguments]: Don't include mafft-homologs manpage.
[inputs]: Add gawk and grep.
[propagated-inputs]: Add coreutils.
---
 gnu/packages/bioinformatics.scm | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index 64dd280..0a3de06 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -40,6 +40,7 @@
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cpio)
   #:use-module (gnu packages file)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages java)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages machine-learning)
@@ -1789,7 +1790,7 @@ sequencing tag position and orientation.")
 (define-public mafft
   (package
     (name "mafft")
-    (version "7.221")
+    (version "7.245")
     (source (origin
               (method url-fetch)
               (uri (string-append
@@ -1798,7 +1799,7 @@ sequencing tag position and orientation.")
               (file-name (string-append name "-" version ".tgz"))
               (sha256
                (base32
-                "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
+                "0m1l7gdrmfxjw54d3a0g18w6rwqrra9w9zvxix7dcddw6d1qyir2"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f ; no automated tests, though there are tests in the read me
@@ -1819,6 +1820,9 @@ sequencing tag position and orientation.")
               ;; remove mafft-homologs.rb from SCRIPTS
               (("^SCRIPTS = mafft mafft-homologs.rb")
                "SCRIPTS = mafft")
+              ;; remove mafft-homologs from MANPAGES
+              (("^MANPAGES = mafft.1 mafft-homologs.1")
+               "MANPAGES = mafft.1")
               ;; remove mafft-distance from PROGS
               (("^PROGS = dvtditr dndfast7 dndblast sextet5 mafft-distance")
                "PROGS = dvtditr dndfast7 dndblast sextet5")
@@ -1831,9 +1835,22 @@ sequencing tag position and orientation.")
               (("^\t\\$\\(INSTALL\\) -m 644 \\$\\(MANPAGES\\) \
 \\$\\(DESTDIR\\)\\$\\(LIBDIR\\)") "#"))
             #t))
+         (add-after 'enter-dir 'patch-paths
+           (lambda* (#:key inputs #:allow-other-keys)
+             (substitute* '("pairash.c"
+                            "mafft.tmpl")
+               (("perl") (which "perl"))
+               (("(\"|`|\| )awk" _ prefix)
+                (string-append prefix (which "awk")))
+               (("grep") (which "grep")))
+             #t))
          (delete 'configure))))
     (inputs
-     `(("perl" ,perl)))
+     `(("perl" ,perl)
+       ("gawk" ,gawk)
+       ("grep" ,grep)))
+    (propagated-inputs
+     `(("coreutils" ,coreutils)))
     (home-page "http://mafft.cbrc.jp/alignment/software/")
     (synopsis "Multiple sequence alignment program")
     (description
-- 
2.1.0


[-- Attachment #3: Type: text/plain, Size: 12 bytes --]


~~ Ricardo

Re: Update mafft to 7.245.

[Ben Woodcroft]

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



On 11/12/15 02:16, Ricardo Wurmus wrote:
> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
>
>> Two reviews in record time, nice.
>>
>> On 10/11/15 23:12, Efraim Flashner wrote:
>>> On Tue, 10 Nov 2015 22:18:10 +1000
>>> Ben Woodcroft <b.woodcroft@uq.edu.au> wrote:
>>>
>>>> Also had to fix the inputs. Hard not to notice these things in the
>>>> environment container - worked well thanks.
>>> Do they need to be propagated inputs? Did you try them as native-inputs?
>>> Often that's enough to take care of it.
>> The main program 'mafft' is actually a reasonably long shell script,
>> which itself calls awk, grep, perl, etc. collectively many times (>100 I
>> would guess). I think it is intended to be run where these programs are
>> available. I could do as Ricardo suggests and run substitute* but this
>> seems a bit error-prone and not very future-proof to me, especially when
>> the shell script is difficult to exhaustively test. WDYT?
> I looked at the instances of “perl”, “awk”, and “grep” and they seem
> manageable.  I didn’t test this but attached is a different version of
> your patch that does what I suggested.  It might work.  Could you try to
> confirm that it’s okay?
>
> What do you think?
I think you are quite valiant. As I say, I cannot be confident in my 
testing (even those in the readme). The diff looked mostly fine by eye, 
but there was some issues near the end which mangled things somewhat 
(although maybe not the result, not sure). How's the attached? I upped 
version too. OK?

Thanks,
ben

[-- Attachment #2: 0001-PATCH-gnu-mafft-Update-to-7.267.patch --]
[-- Type: text/x-patch, Size: 3166 bytes --]

From e256089eadca19e31fa986d2ce7c13f7fbc3311b Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Tue, 15 Dec 2015 20:34:52 +1000
Subject: [PATCH] [PATCH] gnu: mafft: Update to 7.267.

* gnu/packages/bioinformatics.scm (mafft): Update to 7.267.
[arguments]: Don't include mafft-homologs manpage.
[inputs]: Add gawk and grep.
[propagated-inputs]: Add coreutils.
---
 gnu/packages/bioinformatics.scm | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index 7c573e1..21b85db 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -40,6 +40,7 @@
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cpio)
   #:use-module (gnu packages file)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages java)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages machine-learning)
@@ -1690,7 +1691,7 @@ sequencing tag position and orientation.")
 (define-public mafft
   (package
     (name "mafft")
-    (version "7.221")
+    (version "7.267")
     (source (origin
               (method url-fetch)
               (uri (string-append
@@ -1699,7 +1700,7 @@ sequencing tag position and orientation.")
               (file-name (string-append name "-" version ".tgz"))
               (sha256
                (base32
-                "0xi7klbsgi049vsrk6jiwh9wfj3b770gz3c8c7zwij448v0dr73d"))))
+                "1xl6xq1rfxkws0svrlhyqxhhwbv6r77jwblsdpcyiwzsscw6wlk0"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f ; no automated tests, though there are tests in the read me
@@ -1720,6 +1721,9 @@ sequencing tag position and orientation.")
               ;; remove mafft-homologs.rb from SCRIPTS
               (("^SCRIPTS = mafft mafft-homologs.rb")
                "SCRIPTS = mafft")
+              ;; remove mafft-homologs from MANPAGES
+              (("^MANPAGES = mafft.1 mafft-homologs.1")
+               "MANPAGES = mafft.1")
               ;; remove mafft-distance from PROGS
               (("^PROGS = dvtditr dndfast7 dndblast sextet5 mafft-distance")
                "PROGS = dvtditr dndfast7 dndblast sextet5")
@@ -1732,9 +1736,22 @@ sequencing tag position and orientation.")
               (("^\t\\$\\(INSTALL\\) -m 644 \\$\\(MANPAGES\\) \
 \\$\\(DESTDIR\\)\\$\\(LIBDIR\\)") "#"))
             #t))
+         (add-after 'enter-dir 'patch-paths
+           (lambda* (#:key inputs #:allow-other-keys)
+             (substitute* '("pairash.c"
+                            "mafft.tmpl")
+               (("perl") (which "perl"))
+               (("([\"`| ])awk" _ prefix)
+                (string-append prefix (which "awk")))
+               (("grep") (which "grep")))
+             #t))
          (delete 'configure))))
     (inputs
-     `(("perl" ,perl)))
+     `(("perl" ,perl)
+       ("gawk" ,gawk)
+       ("grep" ,grep)))
+    (propagated-inputs
+     `(("coreutils" ,coreutils)))
     (home-page "http://mafft.cbrc.jp/alignment/software/")
     (synopsis "Multiple sequence alignment program")
     (description
-- 
2.5.0

Re: Update mafft to 7.245.

[Ricardo Wurmus]

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

>> What do you think?
> I think you are quite valiant. As I say, I cannot be confident in my 
> testing (even those in the readme). The diff looked mostly fine by eye, 
> but there was some issues near the end which mangled things somewhat

What was mangled?

> (although maybe not the result, not sure). How's the attached? I upped 
> version too. OK?

I think this line may be problematic:

> +               (("([\"`| ])awk" _ prefix)

Is your intent really to replace “ awk” and “|awk”?  In my previous
patch I tried to more explicit by using alternatives in the group:

> +               (("(\"|`|\| )awk" _ prefix)

Was there a problem with the version above?  It does not replace “ awk”
and “|awk” but only “| awk” (in addition to the other two variants).

Other than that the patch does look fine.  If you confirm that this is
what you intended then I’ll push it as is.

Thanks!

~~ Ricardo

Re: Update mafft to 7.245.

[Ben Woodcroft]

On 17/12/15 22:47, Ricardo Wurmus wrote:
> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
>
>>> What do you think?
>> I think you are quite valiant. As I say, I cannot be confident in my
>> testing (even those in the readme). The diff looked mostly fine by eye,
>> but there was some issues near the end which mangled things somewhat
> What was mangled?
Original:

tmpawk=`which nawk 2>/dev/null | awk '{print $1}'`
if [ -x "$tmpawk" ]; then
         prog="$tmpawk"
fi

tmpawk=`which gawk 2>/dev/null | awk '{print $1}'`
if [ -x "$tmpawk" ]; then
         prog="$tmpawk"
fi


After your patch:

tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk=`which 
n/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk 
2>/dev/null | /gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/aw
if [ -x 
"$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk" ]; then
prog="$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk"
fi

tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk=`which 
g/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk 
2>/dev/null | /gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/aw
if [ -x 
"$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk" ]; then
prog="$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk"
fi


The most recent patch:

tmpawk=`which nawk 2>/dev/null | 
/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk '{print $1}'`
if [ -x "$tmpawk" ]; then
         prog="$tmpawk"
fi

tmpawk=`which gawk 2>/dev/null | 
/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk '{print $1}'`
if [ -x "$tmpawk" ]; then
         prog="$tmpawk"
fi


>
>> (although maybe not the result, not sure). How's the attached? I upped
>> version too. OK?
> I think this line may be problematic:
>
>> +               (("([\"`| ])awk" _ prefix)
> Is your intent really to replace “ awk” and “|awk”?
Yes, is that not a good idea?
> In my previous
> patch I tried to more explicit by using alternatives in the group:
>
>> +               (("(\"|`|\| )awk" _ prefix)
> Was there a problem with the version above?  It does not replace “ awk”
> and “|awk” but only “| awk” (in addition to the other two variants).
OK. Seems something went astray though.
> Other than that the patch does look fine.  If you confirm that this is
> what you intended then I’ll push it as is.
Thanks, if you are happy. This was just supposed to be a simple update..

I tried adding a check procedure but this didn't work: mafft refused to 
run, when it runs just fine from the store. I was loath to debug that. 
Instead, I was wondering if there was a way to test after installation? 
If these tests could be run in a container that excluded native-inputs 
(but perhaps some extra test-specific dependencies if required), then I 
think such a procedure could be generally quite useful. It would catch 
the errors I made in the original patch, for instance.

Thank you.
ben

Re: Update mafft to 7.245.

[Ricardo Wurmus]

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

> On 17/12/15 22:47, Ricardo Wurmus wrote:
>> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
>>
>>>> What do you think?
>>> I think you are quite valiant. As I say, I cannot be confident in my
>>> testing (even those in the readme). The diff looked mostly fine by eye,
>>> but there was some issues near the end which mangled things somewhat
>> What was mangled?
> Original:
>
> tmpawk=`which nawk 2>/dev/null | awk '{print $1}'`
> if [ -x "$tmpawk" ]; then
>          prog="$tmpawk"
> fi
>
> tmpawk=`which gawk 2>/dev/null | awk '{print $1}'`
> if [ -x "$tmpawk" ]; then
>          prog="$tmpawk"
> fi
>
>
> After your patch:
>
> tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk=`which 
> n/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk 
> 2>/dev/null | /gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/aw
> if [ -x 
> "$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk" ]; then
> prog="$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk"
> fi
>
> tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk=`which 
> g/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk 
> 2>/dev/null | /gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/aw
> if [ -x 
> "$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk" ]; then
> prog="$tmp/gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk"
> fi

Oh, yes, that’s terrible.  Thanks for catching this!  (I don’t see why
my regex would do that, but I’m too tired to check this now.)

> The most recent patch:
>
> tmpawk=`which nawk 2>/dev/null | 
> /gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk '{print $1}'`
> if [ -x "$tmpawk" ]; then
>          prog="$tmpawk"
> fi
>
> tmpawk=`which gawk 2>/dev/null | 
> /gnu/store/k8qgvgwn5anbfy8r70h938kxgd46cyxx-gawk-4.1.3/bin/awk '{print $1}'`
> if [ -x "$tmpawk" ]; then
>          prog="$tmpawk"
> fi

Nice!

>> Other than that the patch does look fine.  If you confirm that this is
>> what you intended then I’ll push it as is.
> Thanks, if you are happy. This was just supposed to be a simple update..

Sorry for the delay!  It just never feels good to me to propagate
inputs.  If ever this can be avoided with a bit of patching I’d like to
try that first.

> I tried adding a check procedure but this didn't work: mafft refused to 
> run, when it runs just fine from the store. I was loath to debug that. 
> Instead, I was wondering if there was a way to test after installation? 
> If these tests could be run in a container that excluded native-inputs 
> (but perhaps some extra test-specific dependencies if required), then I 
> think such a procedure could be generally quite useful. It would catch 
> the errors I made in the original patch, for instance.

You could reorder the phases such that the check phase runs after
installation.  We do this for some Python packages as well.  It’s just a
matter of

   (delete 'check)
   (add-after 'install 'check
     (lambda ...))

Do you want to give this a try or shall I just apply the latest patch
you sent, leaving this for some time later?

Thank you for your patience!

~~ Ricardo

Re: Update mafft to 7.245.

[Ben Woodcroft]

Hi,
>>> Other than that the patch does look fine.  If you confirm that this is
>>> what you intended then I’ll push it as is.
>> Thanks, if you are happy. This was just supposed to be a simple update..
> Sorry for the delay!  It just never feels good to me to propagate
> inputs.  If ever this can be avoided with a bit of patching I’d like to
> try that first.
Oh you misunderstand. I just meant when I started out I figured this 
would be a simple version bump, but things got a bit more complicated. 
The package is much better for your review.
>> I tried adding a check procedure but this didn't work: mafft refused to
>> run, when it runs just fine from the store. I was loath to debug that.
>> Instead, I was wondering if there was a way to test after installation?
>> If these tests could be run in a container that excluded native-inputs
>> (but perhaps some extra test-specific dependencies if required), then I
>> think such a procedure could be generally quite useful. It would catch
>> the errors I made in the original patch, for instance.
> You could reorder the phases such that the check phase runs after
> installation.  We do this for some Python packages as well.  It’s just a
> matter of
>
>     (delete 'check)
>     (add-after 'install 'check
>       (lambda ...))
Sure, but this lambda will be run with the native-inputs present, no?
> Do you want to give this a try or shall I just apply the latest patch
> you sent, leaving this for some time later?
Let's just push if you don't mind. The test themselves also fail, 
presumably because they are out of date, with some minor alignment 
issues. I'll talk to upstream and maybe snag a check target in the process.

Thanks,
ben

Re: Update mafft to 7.245.

[Ricardo Wurmus]

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

> Let's just push if you don't mind. The test themselves also fail, 
> presumably because they are out of date, with some minor alignment 
> issues. I'll talk to upstream and maybe snag a check target in the process.

Okay, I just pushed the last version you sent as 02f35bb5.  Thanks
again and thanks for talking to upstream!

~~ Ricardo