Re: [Quilt-dev] diffstat and test suite

[John Vandenberg]

Hi Jean,

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

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

It looks like a packaging issue, so maybe there is a packaging solution to this.

> > 2* Do we really want to use a quilt command which requires diffstat in
> > the test suite? It is an optional component, and I believe everyone
> > should be able to run the test suite. If we really want to test
> > diffstat, I think we should do so in a separate test file, and only run
> > it if diffstat was found.

I agree that it should be a separate test case. invoked only if
diffstat was found at configure time.

> 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.

--
John