[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add fraggenescan.
From: |
Leo Famulari |
Subject: |
Re: [PATCH] Add fraggenescan. |
Date: |
Mon, 28 Dec 2015 00:43:08 -0500 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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