Re: [Quilt-dev] diffstat and test suite

[Jean Delvare]

John,

> On 9/19/06, Jean Delvare <address@hidden> 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
> @address@hidden  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