John, > On 9/19/06, Jean Delvare <[EMAIL PROTECTED]> wrote: > > There is now one test file in the quilt test suite which requires > > diffstat (dir-a-b.test). It causes "make check" to freeze when quilt > > has been configured --without-diffstat. The reason is that we create a > > diffstat wrapper (compat/diffstat) in this case. The wrapper is > > supposed to call the real diffstat, and that would "work" (i.e. fail > > cleanly) when the wrapper is installed. However it does NOT work when > > the wrapper is still in compat, and compat is in the PATH (which is the > > case when running the test suite): the wrapper will call itself in > > loops forever. > > > > This raises two questions: > > 1* Why do we create a diffstat wrapper when --without-diffstat has been > > passed to configure (or configure simply failed to find diffstat)? I > > don't see the point. I would understand a wrapper saying "Sorry, > > diffstat isn't installed" as we have for sendmail, but a wrapper > > calling a binary we know isn't available, I don't get it. > > We created wrappers so the code base didnt need to be littered with > @[EMAIL PROTECTED] When diffstat wasnt found, the build system would fall > back > to a 'compat' program that failed... > > http://cvs.savannah.nongnu.org/viewcvs/*checkout*/quilt/quilt/compat/diffstat.in?rev=1.2
I assume you meant rev=1.1. > That has been updated to try to execute the named program in case the > program was added afterwards (e.g. via rpm) ... > > http://cvs.savannah.nongnu.org/viewcvs/*checkout*/quilt/quilt/compat/diffstat.in?rev=1.2 I understand this, but it still doesn't make sense. I can't see any difference between having this wrapper and having no wrapper at all. In both cases, "quilt refresh --diffstat" will fail if diffstat isn't installed, and succeed if it is. You have an additional level of indirection with no benefit that I can see. So I'd rather have no wrapper at all. > It looks like a packaging issue, so maybe there is a packaging solution to > this. I don't quite get what you mean here, can you be more precise? > > Here is the patch I have come up with, which I'll apply to CVS soon > > unless somebody objects. It reverts the first chunk of: > > http://cvs.savannah.nongnu.org/viewcvs/quilt/configure.ac?root=quilt&r1=1.55&r2=1.56 > > and deletes compat/diffstat.in. It also removes the use of diffstat in > > the test suite. > > > > With this patch, the test suite passes again on a system I have where > > diffstat is not installed. > > > > John, want to comment on this? > > On inspection, this patch doesnt appear to work correctly; the program > supplied to --with-diffstat wont be hardwired into the quilt source. Ah, yes, good point. I only tested the case where diffstat is found, and the case where it is not - not the case where a specific path is provided by the user. I need to fix this. Any idea how I can do that? Your QUILT_COMPAT_PROG_PATH macro doesn't appear to support optional binaries, and I would like to avoid duplicating code if possible. Thanks, -- Jean Delvare _______________________________________________ Quilt-dev mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/quilt-dev
