Re: [PATCH] git-cvs*: Make building (and testing) of CVS interface scripts optionally selectable

"Robin H. Johnson" [off-list ref] writes:

Presently, the CVS interface scripts are always built, and their
test-suites run based on a binary named 'cvs' happening to return zero.
If there something other than the real CVS there, bad things happened
during the test-suite run.

Is that a problem?

It makes sense to let people whose system happens to have a working cvs to
omit cvs interoperability bits when they know the do not want them, and
that alone would be a good enough motivation.

I'd even consider the above justification of yours detrimental---it would
be an excuse for other people to add patches to support NO_CAT, NO_DIFF,
NO_LS, ... saying "if a binary 'cat' that is not cat is there, things
break, so work it around".  That is a road to nonsense land.

This patch implements NO_CVS in the manner of NO_PERL, and ensures that
the CVS scripts get the unimplemented variant when appropriate, as well
as making sure that the tests properly declare CVS as a prerequisite
(shortcut to test_done like the Perl prerequisites).

While the patch looks good, some people seem to prefer skipping individual
tests without shortcut; see 900eab4 (t/t9600-cvsimport.sh: change from
skip_all=* to prereq skip, 2010-08-13) for example.  I am slightly in
favor of the short-cut as I haven't heard convincing argument against it
other than "skipped statistics" which I don't think is interesting nor
accurate anyway.

I wonder if "check PERL and CVS prerequisite and say test_done" should be
made into a helper in lib-cvs.sh or somewhere instead of repeating them in
individual tests, but that is a minor point.

On Fri, Jan 07, 2011 at 02:05:59PM -0800, Junio C Hamano wrote:

quoted

Presently, the CVS interface scripts are always built, and their
test-suites run based on a binary named 'cvs' happening to return zero.
If there something other than the real CVS there, bad things happened
during the test-suite run.

It makes sense to let people whose system happens to have a working cvs to
omit cvs interoperability bits when they know the do not want them, and
that alone would be a good enough motivation.

Ok, i'll respin to a different explanation.

While the patch looks good, some people seem to prefer skipping individual
tests without shortcut; see 900eab4 (t/t9600-cvsimport.sh: change from
skip_all=* to prereq skip, 2010-08-13) for example.  I am slightly in
favor of the short-cut as I haven't heard convincing argument against it
other than "skipped statistics" which I don't think is interesting nor
accurate anyway.

I did it this was because of your prior comment on my perl prereq patch
for the send-email tests.

I wonder if "check PERL and CVS prerequisite and say test_done" should be
made into a helper in lib-cvs.sh or somewhere instead of repeating them in
individual tests, but that is a minor point.

I'll see if I can respin to do this.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85