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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 7, 2011, 22:05 UTC
Message-ID: <7v39p45pyw.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1294433290-9262-1-git-send-email-robbat2@gentoo.org>

"Robin H. Johnson" <robbat2@gentoo.org> 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.