* [PATCH] Add fraggenescan. @ 2015-12-20 12:36 Ben Woodcroft 2015-12-20 13:03 ` Ricardo Wurmus 0 siblings, 1 reply; 8+ messages in thread From: Ben Woodcroft @ 2015-12-20 12:36 UTC (permalink / raw) To: guix-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 39 bytes --] Turned out to be a little messy. TIA.. [-- Attachment #2: 0001-gnu-Add-fraggenescan.patch --] [-- Type: text/x-patch, Size: 4716 bytes --] From f10c4178cef360a8472c988c9ea8aace15daa954 Mon Sep 17 00:00:00 2001 From: Ben Woodcroft <donttrustben@gmail.com> Date: Sun, 20 Dec 2015 22:23:17 +1000 Subject: [PATCH] gnu: Add fraggenescan. * gnu/packages/bioinformatics.scm (fraggenescan): New variable. --- gnu/packages/bioinformatics.scm | 80 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm index 7c573e1..3e4e05b 100644 --- a/gnu/packages/bioinformatics.scm +++ b/gnu/packages/bioinformatics.scm @@ -1354,6 +1354,86 @@ supports next-generation sequencing data in fasta/q and csfasta/q format from Illumina, Roche 454, and the SOLiD platform.") (license license:gpl3))) +(define-public fraggenescan + (package + (name "fraggenescan") + (version "1.20") + (source + (origin + (method url-fetch) + (uri + (string-append "mirror://sourceforge/fraggenescan/" + "FragGeneScan" version ".tar.gz")) + (sha256 + (base32 "1zzigqmvqvjyqv4945kv6nc5ah2xxm1nxgrlsnbzav3f5c0n0pyj")))) + (build-system gnu-build-system) + (arguments + `(#:phases + (modify-phases %standard-phases + (delete 'configure) + (add-before 'build 'patch-run-script + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (string-append (assoc-ref outputs "out"))) + (share (string-append out "/share/fraggenescan/"))) + (substitute* "run_FragGeneScan.pl" + (("system\\(\"rm") + (string-append "system(\"" (which "rm"))) + (("system\\(\"mv") + (string-append "system(\"" (which "mv"))) + ;; This script and other programs expect the training files + ;; to be in the non-standard location bin/train/XXX. Change + ;; this to be share/fraggenescan/train/XXX instead. + (("^\\$train.file = \\$dir.\\\"train/\\\".\\$FGS_train_file;") + (string-append "$train_file = \"" + share + "train/\".$FGS_train_file;"))) + (substitute* "run_hmm.c" + (("^ strcat\\(train_dir, \\\"train/\\\"\\);") + (string-append " strcpy(train_dir, \"" share "/train/\");"))) + (substitute* "post_process.pl" + (("^my \\$dir = substr\\(\\$0, 0, length\\(\\$0)-15\\);") + (string-append "my $dir = \"" share "\";")))) + #t)) + (replace 'build + (lambda _ (and (zero? (system* "make" "clean")) + (zero? (system* "make" "fgs"))))) + (delete 'check) + (replace 'install + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (string-append (assoc-ref outputs "out"))) + (bin (string-append out "/bin/")) + (share (string-append out "/share/fraggenescan/train"))) + (install-file "run_FragGeneScan.pl" bin) + (install-file "FragGeneScan" bin) + (install-file "FGS_gff.py" bin) + (install-file "post_process.pl" bin) + (copy-recursively "train" share)))) + (add-after 'install 'post-install-check + ;; In lieu of 'make check', run one of the examples and check the + ;; output files gets created. + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (string-append (assoc-ref outputs "out"))) + (bin (string-append out "/bin/"))) + (and (zero? (system* (string-append bin "run_FragGeneScan.pl") + "-genome=./example/NC_000913.fna" + "-out=./test2" + "-complete=1" + "-train=complete")) + (file-exists? "test2.faa") + (file-exists? "test2.ffn") + (file-exists? "test2.gff") + (file-exists? "test2.out")))))))) + (inputs + `(("perl" ,perl) + ("python" ,python-2))) ;not compatible with python 3. + (home-page "https://sourceforge.net/projects/fraggenescan/") + (synopsis "Finds potentially fragmented genes in short reads") + (description + "FragGeneScan is a program for predicting bacterial and archaeal genes in +short and error-prone DNA sequencing reads. It can also be applied to predict +genes in incomplete assemblies or complete genomes.") + (license license:gpl1))) + (define-public grit (package (name "grit") -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-20 12:36 [PATCH] Add fraggenescan Ben Woodcroft @ 2015-12-20 13:03 ` Ricardo Wurmus 2015-12-27 21:53 ` Ben Woodcroft 0 siblings, 1 reply; 8+ messages in thread From: Ricardo Wurmus @ 2015-12-20 13:03 UTC (permalink / raw) To: Ben Woodcroft; +Cc: guix-devel@gnu.org Hi Ben, thanks for your patch! > From f10c4178cef360a8472c988c9ea8aace15daa954 Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft <donttrustben@gmail.com> > Date: Sun, 20 Dec 2015 22:23:17 +1000 > Subject: [PATCH] gnu: Add fraggenescan. > * gnu/packages/bioinformatics.scm (fraggenescan): New variable. > --- > gnu/packages/bioinformatics.scm | 80 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 80 insertions(+) [...] > +(define-public fraggenescan > + (package > + (name "fraggenescan") > + (version "1.20") > + (source > + (origin > + (method url-fetch) > + (uri > + (string-append "mirror://sourceforge/fraggenescan/" > + "FragGeneScan" version ".tar.gz")) > + (sha256 > + (base32 "1zzigqmvqvjyqv4945kv6nc5ah2xxm1nxgrlsnbzav3f5c0n0pyj")))) > + (build-system gnu-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (delete 'configure) > + (add-before 'build 'patch-run-script This phase does more than just patch the run script (it changes paths in two scripts and a C source file). Could you find a better name? > + (lambda* (#:key outputs #:allow-other-keys) > + (let* ((out (string-append (assoc-ref outputs "out"))) > + (share (string-append out "/share/fraggenescan/"))) > + (substitute* "run_FragGeneScan.pl" > + (("system\\(\"rm") > + (string-append "system(\"" (which "rm"))) > + (("system\\(\"mv") > + (string-append "system(\"" (which "mv"))) > + ;; This script and other programs expect the training files > + ;; to be in the non-standard location bin/train/XXX. Change > + ;; this to be share/fraggenescan/train/XXX instead. > + (("^\\$train.file = \\$dir.\\\"train/\\\".\\$FGS_train_file;") > + (string-append "$train_file = \"" > + share > + "train/\".$FGS_train_file;"))) I might look clearer if you captured a part of the matches, (("(^\\$train\\.file = )\\$dir\\.\\\"(train/\\\"\\.\\$FGS_train_file;)" _ pre post) (string-append pre "\"" share post)) Or since there’s really just one line beginning with “$train.file” maybe you could do this: (("(^\\$train\\.file = ).*" _ prefix) (string-append prefix "\"" share "train/\".$FGS_train_file;")) The regular expressions above look quite scary, so maybe the latter proposal is best here. > + (substitute* "run_hmm.c" > + (("^ strcat\\(train_dir, \\\"train/\\\"\\);") > + (string-append " strcpy(train_dir, \"" share "/train/\");"))) Why do you replace “strcat” with “strcpy” here? > + (substitute* "post_process.pl" > + (("^my \\$dir = substr\\(\\$0, 0, length\\(\\$0)-15\\);") > + (string-append "my $dir = \"" share "\";")))) I would also suggest simplifying the regular expression here. There is only one line matching “^my \\$dir =”. > + #t)) > + (replace 'build > + (lambda _ (and (zero? (system* "make" "clean")) > + (zero? (system* "make" "fgs"))))) Why must “make clean” be run first? I know the README says so, but is it really required? If it is not you could just use the default build phase, possibly specifying “fgs” as the target. > + (delete 'check) How about “#:tests? #f” instead? > + (replace 'install > + (lambda* (#:key outputs #:allow-other-keys) > + (let* ((out (string-append (assoc-ref outputs "out"))) > + (bin (string-append out "/bin/")) > + (share (string-append out "/share/fraggenescan/train"))) > + (install-file "run_FragGeneScan.pl" bin) > + (install-file "FragGeneScan" bin) > + (install-file "FGS_gff.py" bin) > + (install-file "post_process.pl" bin) > + (copy-recursively "train" share)))) > + (add-after 'install 'post-install-check > + ;; In lieu of 'make check', run one of the examples and check the > + ;; output files gets created. Oh, I see. Maybe you could delete the “check” phase right before this, so that it’s obvious you are moving it after the “install” phase. > + (lambda* (#:key outputs #:allow-other-keys) > + (let* ((out (string-append (assoc-ref outputs "out"))) > + (bin (string-append out "/bin/"))) > + (and (zero? (system* (string-append bin "run_FragGeneScan.pl") > + "-genome=./example/NC_000913.fna" > + "-out=./test2" > + "-complete=1" > + "-train=complete")) > + (file-exists? "test2.faa") > + (file-exists? "test2.ffn") > + (file-exists? "test2.gff") > + (file-exists? "test2.out")))))))) > + (inputs > + `(("perl" ,perl) > + ("python" ,python-2))) ;not compatible with python 3. > + (home-page "https://sourceforge.net/projects/fraggenescan/") > + (synopsis "Finds potentially fragmented genes in short reads") > + (description > + "FragGeneScan is a program for predicting bacterial and archaeal genes in > +short and error-prone DNA sequencing reads. It can also be applied to predict > +genes in incomplete assemblies or complete genomes.") > + (license license:gpl1))) I didn’t see any mention of a particular GPL version. The README says this: License ============ Copyright (C) 2010 Mina Rho, Yuzhen Ye and Haixu Tang. You may redistribute this software under the terms of the GNU General Public License. This looks like it could be any version of the GPL (as it is implied when R packages just declare “GPL” as a license). I would do this, but I’m not sure that’s okay: ;; Released under any version of the GPL (license license:gpl3+) ~~ Ricardo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-20 13:03 ` Ricardo Wurmus @ 2015-12-27 21:53 ` Ben Woodcroft 2015-12-28 0:38 ` Leo Famulari 0 siblings, 1 reply; 8+ messages in thread From: Ben Woodcroft @ 2015-12-27 21:53 UTC (permalink / raw) To: Ricardo Wurmus; +Cc: guix-devel@gnu.org [-- Attachment #1: Type: text/plain, Size: 6568 bytes --] Heya, On 20/12/15 23:03, Ricardo Wurmus wrote: > thanks for your patch! and thank you. >> +(define-public fraggenescan >> + (package >> + (name "fraggenescan") >> + (version "1.20") >> + (source >> + (origin >> + (method url-fetch) >> + (uri >> + (string-append "mirror://sourceforge/fraggenescan/" >> + "FragGeneScan" version ".tar.gz")) >> + (sha256 >> + (base32 "1zzigqmvqvjyqv4945kv6nc5ah2xxm1nxgrlsnbzav3f5c0n0pyj")))) >> + (build-system gnu-build-system) >> + (arguments >> + `(#:phases >> + (modify-phases %standard-phases >> + (delete 'configure) >> + (add-before 'build 'patch-run-script > This phase does more than just patch the run script (it changes paths in > two scripts and a C source file). Could you find a better name? ok, done. >> + (lambda* (#:key outputs #:allow-other-keys) >> + (let* ((out (string-append (assoc-ref outputs "out"))) >> + (share (string-append out "/share/fraggenescan/"))) >> + (substitute* "run_FragGeneScan.pl" >> + (("system\\(\"rm") >> + (string-append "system(\"" (which "rm"))) >> + (("system\\(\"mv") >> + (string-append "system(\"" (which "mv"))) >> + ;; This script and other programs expect the training files >> + ;; to be in the non-standard location bin/train/XXX. Change >> + ;; this to be share/fraggenescan/train/XXX instead. >> + (("^\\$train.file = \\$dir.\\\"train/\\\".\\$FGS_train_file;") >> + (string-append "$train_file = \"" >> + share >> + "train/\".$FGS_train_file;"))) > I might look clearer if you captured a part of the matches, > > (("(^\\$train\\.file = )\\$dir\\.\\\"(train/\\\"\\.\\$FGS_train_file;)" _ pre post) > (string-append pre "\"" share post)) > > Or since there’s really just one line beginning with “$train.file” maybe > you could do this: > > (("(^\\$train\\.file = ).*" _ prefix) > (string-append prefix "\"" share "train/\".$FGS_train_file;")) > > The regular expressions above look quite scary, so maybe the latter > proposal is best here. Indeed they are scary, and writing regexes for Guix rarely fails to trip me up. I'd actually prefer a less powerful alternative (just a regular string replacement), and better yet one that stops the build process when it matches 0 or 2 or more times. There's nothing like that around is there? But for now I just took your suggestions about shortening the regexes. > >> + (substitute* "run_hmm.c" >> + (("^ strcat\\(train_dir, \\\"train/\\\"\\);") >> + (string-append " strcpy(train_dir, \"" share "/train/\");"))) > Why do you replace “strcat” with “strcpy” here? The line above does a strcpy we don't want, so strcat would keep that. I could remove the line above if you want, but this effectively makes no difference to the running of the program. [..] >> + #t)) >> + (replace 'build >> + (lambda _ (and (zero? (system* "make" "clean")) >> + (zero? (system* "make" "fgs"))))) > Why must “make clean” be run first? I know the README says so, but is > it really required? If it is not you could just use the default build > phase, possibly specifying “fgs” as the target. Yeh the tarball comes with compiled files. I've added a comment. > >> + (delete 'check) > How about “#:tests? #f” instead? > >> + (replace 'install >> + (lambda* (#:key outputs #:allow-other-keys) >> + (let* ((out (string-append (assoc-ref outputs "out"))) >> + (bin (string-append out "/bin/")) >> + (share (string-append out "/share/fraggenescan/train"))) >> + (install-file "run_FragGeneScan.pl" bin) >> + (install-file "FragGeneScan" bin) >> + (install-file "FGS_gff.py" bin) >> + (install-file "post_process.pl" bin) >> + (copy-recursively "train" share)))) >> + (add-after 'install 'post-install-check >> + ;; In lieu of 'make check', run one of the examples and check the >> + ;; output files gets created. > Oh, I see. Maybe you could delete the “check” phase right before this, > so that it’s obvious you are moving it after the “install” phase. ok, done. > >> + (lambda* (#:key outputs #:allow-other-keys) >> + (let* ((out (string-append (assoc-ref outputs "out"))) >> + (bin (string-append out "/bin/"))) >> + (and (zero? (system* (string-append bin "run_FragGeneScan.pl") >> + "-genome=./example/NC_000913.fna" >> + "-out=./test2" >> + "-complete=1" >> + "-train=complete")) >> + (file-exists? "test2.faa") >> + (file-exists? "test2.ffn") >> + (file-exists? "test2.gff") >> + (file-exists? "test2.out")))))))) >> + (inputs >> + `(("perl" ,perl) >> + ("python" ,python-2))) ;not compatible with python 3. >> + (home-page "https://sourceforge.net/projects/fraggenescan/") >> + (synopsis "Finds potentially fragmented genes in short reads") >> + (description >> + "FragGeneScan is a program for predicting bacterial and archaeal genes in >> +short and error-prone DNA sequencing reads. It can also be applied to predict >> +genes in incomplete assemblies or complete genomes.") >> + (license license:gpl1))) > I didn’t see any mention of a particular GPL version. The README says > this: > > License > ============ > Copyright (C) 2010 Mina Rho, Yuzhen Ye and Haixu Tang. > You may redistribute this software under the terms of the GNU General Public License. > > This looks like it could be any version of the GPL (as it is implied > when R packages just declare “GPL” as a license). I would do this, but > I’m not sure that’s okay: > > ;; Released under any version of the GPL > (license license:gpl3+) It seems your interpretation was better than mine. The authors said gpl3+ over email. Thanks, ben [-- Attachment #2: 0001-gnu-Add-fraggenescan.patch --] [-- Type: text/x-patch, Size: 4717 bytes --] From a140b0816f095a7a13d6bdd4dcbddbae1d020fbb Mon Sep 17 00:00:00 2001 From: Ben Woodcroft <donttrustben@gmail.com> Date: Sun, 20 Dec 2015 22:23:17 +1000 Subject: [PATCH] gnu: Add fraggenescan. * gnu/packages/bioinformatics.scm (fraggenescan): New variable. --- gnu/packages/bioinformatics.scm | 81 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm index 7c573e1..afa32f2 100644 --- a/gnu/packages/bioinformatics.scm +++ b/gnu/packages/bioinformatics.scm @@ -1354,6 +1354,87 @@ supports next-generation sequencing data in fasta/q and csfasta/q format from Illumina, Roche 454, and the SOLiD platform.") (license license:gpl3))) +(define-public fraggenescan + (package + (name "fraggenescan") + (version "1.20") + (source + (origin + (method url-fetch) + (uri + (string-append "mirror://sourceforge/fraggenescan/" + "FragGeneScan" version ".tar.gz")) + (sha256 + (base32 "1zzigqmvqvjyqv4945kv6nc5ah2xxm1nxgrlsnbzav3f5c0n0pyj")))) + (build-system gnu-build-system) + (arguments + `(#:phases + (modify-phases %standard-phases + (delete 'configure) + (add-before 'build 'patch-paths + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (string-append (assoc-ref outputs "out"))) + (share (string-append out "/share/fraggenescan/"))) + (substitute* "run_FragGeneScan.pl" + (("system\\(\"rm") + (string-append "system(\"" (which "rm"))) + (("system\\(\"mv") + (string-append "system(\"" (which "mv"))) + ;; This script and other programs expect the training files + ;; to be in the non-standard location bin/train/XXX. Change + ;; this to be share/fraggenescan/train/XXX instead. + (("^\\$train.file = \\$dir.*") + (string-append "$train_file = \"" + share + "train/\".$FGS_train_file;"))) + (substitute* "run_hmm.c" + (("^ strcat\\(train_dir, \\\"train/\\\"\\);") + (string-append " strcpy(train_dir, \"" share "/train/\");"))) + (substitute* "post_process.pl" + (("^my \\$dir = substr.*") + (string-append "my $dir = \"" share "\";")))) + #t)) + (replace 'build + (lambda _ (and (zero? (system* "make" "clean")) + (zero? (system* "make" "fgs"))))) + (replace 'install + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (string-append (assoc-ref outputs "out"))) + (bin (string-append out "/bin/")) + (share (string-append out "/share/fraggenescan/train"))) + (install-file "run_FragGeneScan.pl" bin) + (install-file "FragGeneScan" bin) + (install-file "FGS_gff.py" bin) + (install-file "post_process.pl" bin) + (copy-recursively "train" share)))) + (delete 'check) + (add-after 'install 'post-install-check + ;; In lieu of 'make check', run one of the examples and check the + ;; output files gets created. + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (string-append (assoc-ref outputs "out"))) + (bin (string-append out "/bin/"))) + (and (zero? (system* (string-append bin "run_FragGeneScan.pl") + "-genome=./example/NC_000913.fna" + "-out=./test2" + "-complete=1" + "-train=complete")) + (file-exists? "test2.faa") + (file-exists? "test2.ffn") + (file-exists? "test2.gff") + (file-exists? "test2.out")))))))) + (inputs + `(("perl" ,perl) + ("python" ,python-2))) ;not compatible with python 3. + (home-page "https://sourceforge.net/projects/fraggenescan/") + (synopsis "Finds potentially fragmented genes in short reads") + (description + "FragGeneScan is a program for predicting bacterial and archaeal genes in +short and error-prone DNA sequencing reads. It can also be applied to predict +genes in incomplete assemblies or complete genomes.") + ;; GPL3+ according to private correspondense with the authors. + (license license:gpl3+))) + (define-public grit (package (name "grit") -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-27 21:53 ` Ben Woodcroft @ 2015-12-28 0:38 ` Leo Famulari 2015-12-28 0:56 ` Ben Woodcroft 0 siblings, 1 reply; 8+ messages in thread From: Leo Famulari @ 2015-12-28 0:38 UTC (permalink / raw) To: Ben Woodcroft; +Cc: guix-devel@gnu.org On Mon, Dec 28, 2015 at 07:53:27AM +1000, Ben Woodcroft wrote: > On 20/12/15 23:03, Ricardo Wurmus wrote: [...] > >>+ (substitute* "run_hmm.c" > >>+ (("^ strcat\\(train_dir, \\\"train/\\\"\\);") > >>+ (string-append " strcpy(train_dir, \"" share "/train/\");"))) > >Why do you replace “strcat” with “strcpy” here? > The line above does a strcpy we don't want, so strcat would keep that. I > could remove the line above if you want, but this effectively makes no > difference to the running of the program. It looks like this substition has two parts: 1) Provide the correct path to some resources ("train files") for the program. 2) Change a function call from strcat() to strcpy(). Can you elaborate more on part 2? I undertand that the intended effect is to discard a value assigned to 'train_dir' [0] in the previous function call. This discarded value is 'argv[0]' minus 12 bytes [1]. The reason for this is that the program is looking for the "train files" in the same directory as 'argv[0]', but we have installed the "train files" somewhere else. Is that right? You mention in a comment in the patch that this program and other programs expect the "train files" to be in one place, but we are patching this line to account for the fact that we have installed them in another location. Will the other programs have to be patched as well? If so, do you think it would be better to let the files be installed in the unusual location chosen by upstream? [0] 'train_dir' is something like the --prefix parameter in a ./configure script. Many paths are generated based on it, and they all seem to be assigned to buffers with a length of 4096 bytes. [1] The magic number 12 is equal to the length of the basename of argv[0], "fraggenescan". For the benefit of those reading along... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-28 0:38 ` Leo Famulari @ 2015-12-28 0:56 ` Ben Woodcroft 2015-12-28 5:43 ` Leo Famulari 0 siblings, 1 reply; 8+ messages in thread From: Ben Woodcroft @ 2015-12-28 0:56 UTC (permalink / raw) To: Leo Famulari; +Cc: guix-devel@gnu.org On 28/12/15 10:38, Leo Famulari wrote: > On Mon, Dec 28, 2015 at 07:53:27AM +1000, Ben Woodcroft wrote: >> On 20/12/15 23:03, Ricardo Wurmus wrote: > [...] > >>>> + (substitute* "run_hmm.c" >>>> + (("^ strcat\\(train_dir, \\\"train/\\\"\\);") >>>> + (string-append " strcpy(train_dir, \"" share "/train/\");"))) >>> Why do you replace “strcat” with “strcpy” here? >> The line above does a strcpy we don't want, so strcat would keep that. I >> could remove the line above if you want, but this effectively makes no >> difference to the running of the program. > It looks like this substition has two parts: > 1) Provide the correct path to some resources ("train files") for the > program. > 2) Change a function call from strcat() to strcpy(). > > Can you elaborate more on part 2? > > I undertand that the intended effect is to discard a value assigned to > 'train_dir' [0] in the previous function call. This discarded value is > 'argv[0]' minus 12 bytes [1]. The reason for this is that the program is > looking for the "train files" in the same directory as 'argv[0]', but we > have installed the "train files" somewhere else. Is that right? Yes that is right. As is often the case for bioinformatics programs, it isn't intended to be 'installed', you are just expected to add the directory where compilation happens to your $PATH. So they don't mind stretching the conventions. > You mention in a comment in the patch that this program and other > programs expect the "train files" to be in one place, but we are > patching this line to account for the fact that we have installed them > in another location. Will the other programs have to be patched as > well? Yep, and I believe I have done so. Did I miss something? If you mean other programs that use fraggenescan, they would most likely only call "run_FragGeneScan.pl" so I don't see an issue there. > If so, do you think it would be better to let the files be > installed in the unusual location chosen by upstream? Not after the work I did trying to do the 'right' thing.. I would imagine this program will not likely be updated with much, and if there was major updates I would guess that they would require substantive changes to the package definition anyway. [..] Thanks for the thoughts. I hope there wasn't too much gut feeling in my reply. ben ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-28 0:56 ` Ben Woodcroft @ 2015-12-28 5:43 ` Leo Famulari 2015-12-29 23:23 ` Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Leo Famulari @ 2015-12-28 5:43 UTC (permalink / raw) To: Ben Woodcroft; +Cc: guix-devel@gnu.org On Mon, Dec 28, 2015 at 10:56:51AM +1000, Ben Woodcroft wrote: > > > On 28/12/15 10:38, Leo Famulari wrote: > >On Mon, Dec 28, 2015 at 07:53:27AM +1000, Ben Woodcroft wrote: > >>On 20/12/15 23:03, Ricardo Wurmus wrote: > >[...] > > > >>>>+ (substitute* "run_hmm.c" > >>>>+ (("^ strcat\\(train_dir, \\\"train/\\\"\\);") > >>>>+ (string-append " strcpy(train_dir, \"" share "/train/\");"))) > >>>Why do you replace “strcat” with “strcpy” here? > >>The line above does a strcpy we don't want, so strcat would keep that. I > >>could remove the line above if you want, but this effectively makes no > >>difference to the running of the program. > >It looks like this substition has two parts: > >1) Provide the correct path to some resources ("train files") for the > >program. > >2) Change a function call from strcat() to strcpy(). > > > >Can you elaborate more on part 2? > > > >I undertand that the intended effect is to discard a value assigned to > >'train_dir' [0] in the previous function call. This discarded value is > >'argv[0]' minus 12 bytes [1]. The reason for this is that the program is > >looking for the "train files" in the same directory as 'argv[0]', but we > >have installed the "train files" somewhere else. Is that right? > Yes that is right. As is often the case for bioinformatics programs, it > isn't intended to be 'installed', you are just expected to add the directory > where compilation happens to your $PATH. So they don't mind stretching the > conventions. Sounds like a good use case for `guix environment`. Okay, I just wanted to make sure I understood correctly. Can't be too careful with strings in C! > >You mention in a comment in the patch that this program and other > >programs expect the "train files" to be in one place, but we are > >patching this line to account for the fact that we have installed them > >in another location. Will the other programs have to be patched as > >well? > Yep, and I believe I have done so. Did I miss something? > > If you mean other programs that use fraggenescan, they would most likely > only call "run_FragGeneScan.pl" so I don't see an issue there. Okay. > >If so, do you think it would be better to let the files be > >installed in the unusual location chosen by upstream? > Not after the work I did trying to do the 'right' thing.. I would imagine > this program will not likely be updated with much, and if there was major > updates I would guess that they would require substantive changes to the > package definition anyway. Fair enough, I just wanted to ask since I'm not familiar with this program or the ones that will be interacting with it. > > [..] > > Thanks for the thoughts. I hope there wasn't too much gut feeling in my > reply. Not at all! Thanks for wrangling all those backslashes! > ben ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-28 5:43 ` Leo Famulari @ 2015-12-29 23:23 ` Ludovic Courtès 2015-12-30 1:19 ` Leo Famulari 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2015-12-29 23:23 UTC (permalink / raw) To: Leo Famulari; +Cc: guix-devel@gnu.org Leo, feel free to apply the version that looks good to you. Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add fraggenescan. 2015-12-29 23:23 ` Ludovic Courtès @ 2015-12-30 1:19 ` Leo Famulari 0 siblings, 0 replies; 8+ messages in thread From: Leo Famulari @ 2015-12-30 1:19 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guix-devel@gnu.org On Wed, Dec 30, 2015 at 12:23:31AM +0100, Ludovic Courtès wrote: > Leo, feel free to apply the version that looks good to you. I pushed the 2nd version (integrating Ricardo's suggestions) as 19f4554c94. Thanks, Ben! > > Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-12-30 1:19 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-20 12:36 [PATCH] Add fraggenescan Ben Woodcroft 2015-12-20 13:03 ` Ricardo Wurmus 2015-12-27 21:53 ` Ben Woodcroft 2015-12-28 0:38 ` Leo Famulari 2015-12-28 0:56 ` Ben Woodcroft 2015-12-28 5:43 ` Leo Famulari 2015-12-29 23:23 ` Ludovic Courtès 2015-12-30 1:19 ` Leo Famulari
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).