Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

[Michael Haggerty <mhagger@alum.mit.edu>]

On 01/11/2013 04:32 AM, Junio C Hamano wrote:
> From: "Eric S. Raymond" <esr@thyrsus.com>
> 
> The combination of git-cvsimport and cvsps had serious problems.

Agreed.

> [...]
> This patch also removes Michael Haggerty's git-cvsimport tests
> (t960[123]) from the git tree.  These are actually conversion-engine
> tests and have been merged into a larger cvsps test suite, which I
> intend to spin out into a general CVS-lifting test that can also be
> applied to utilities such as cvs2git and parsecvs.  The t9604 test
> will move in a future patch, when I likewise have it integrated
> into the general test suite.
> 
> The following known bug has not been fixed: "If any files were ever
> "cvs import"ed more than once (e.g., import of more than one vendor
> release) the HEAD contains the wrong content." However, cvsps now
> emits a warning in this case. There is also one pathological tagging
> case that was successful in the former t9602 test that now fails
> (with a warning).
> 
> I plan to address these problems. This patch at least gets the
> cvsps-3.x/git-cvsimport combination to a state that is not too
> broken to ship - that is, in all failure cases known to me it
> now emits useful warnings rather than silently botching the
> import.

I don't understand the logic of removing the cvsimport tests, at least
not at this time.  It is true that the tests mostly ensure that the
conversion engine is working correctly, especially with your new version
of cvsps.  But I think the git project, by implicitly endorsing the use
of cvsps, has some responsibility to verify that the combination cvsps +
git-cvsimport continues to work and to document any known breakages via
its test suite.

Otherwise, how do we know that cvsps currently works with git-cvsimport?
 (OK, you claim that it does, but in the next breath you admit that
there is a new failure in "one pathological tagging case".)  How can we
understand its strengths/weaknesses?  How can we gain confidence that it
works on different platforms?  How will we find out if a future versions
of cvsps stops working (e.g., because of a breakage or a
non-backwards-compatible change)?

Normally one would expect an improvement like this to be combined with
patches that turn test expected failures into expected successes, not to
rip out the very tests that establish the correctness of the change that
is being proposed!


Let me describe what I would consider to be the optimum state of the
test suite.  Maybe your idea of "optimum" differs from mine, or maybe
the optimum is unrealistic due to lack of resources or for some other
reason.  But if so, let's explicitly spell out why we are deviating from
whatever optimum we define.

* The old tests should be retained (and possibly new tests added to show
off your improvements).

* There should be a way for users to choose which cvsps executable to
use when running test suite.  (In the future, the selection might be
expanded to cover altogether different conversion engines.)

* The tests should determine which version of cvsps has been selected
(e.g., by running "cvsps --version").

* The individual tests should be marked expected success/expected
failure based on the selected version of cvsps; in other words, some
tests might be marked "expected failure" if cvsps 2.x is being used but
"expected success" if cvsps 3.x is being used.


Regarding your claim that "within a few months the Perl git-cvsimport is
going to cease even pretending to work": It might be that the old
git-cvsimport will stop working *for people who upgrade to cvsps 3.x*.
But it is not realistic to expect people to synchronize their git and
cvsps version upgrades.  It is even quite possible that this or that
Linux distribution will package incompatible versions of the two packages.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/