Re: [PATCH] Add fraggenescan.

[Leo Famulari <leo@famulari.name>]

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