Re: [PATCH v3 0/2] cvsimport: add test illustrating a bug in cvsps

> Heiko Voigt <git-list@hvoigt.net> writes:
> 
>> This is an updated version of the first patch and an addition to ensure
>> correct handling of branches in fixes. 
> 
> I've already merged the first one to 'next' so this needs to be turned
> into an incremental update if we were to continue building on top in the
> git project.
> 
> I however have a bigger issue with this, perhaps because I do not have the
> feeling that I fully understand where these patches are going.
> 
> Your approach seems to me to:
> 
>  - add tests to git test suite that expose issues the current cvsimport
>    that runs on an unpatched cvsps has;
> 
>  - diagnose and fix
> 
>    - the issues in cvsimport, if the problem is because cvsimport is
>      mishandling correct output from cvsps; or
> 
>    - the issues in cvsps (and adjust cvsimport to patched cvsps if
>      necessary), if the problem is because output from cvsps is incorrect.
> 
> That all feels sane, and having the tests to verify the end result would
> help the people who collaborate on these tasks.
> 
> But how much of the actual fix will be made to cvsps, and how much to
> cvsimport?  If the majority of the changes are to happen on cvsps (which
> is not unexpected, given that many people who tried and wrote various cvs
> importers put blame on the shortcomings of its output), I am afraid that
> it would not help majority of git users until the fixes to cvsps that come
> out of this effort hit their distros for me to keep these tests in the
> git.git repository.  I do not build and install custom cvsps (because I
> haven't had to work with complex history in CVS that your improvements to
> cvsps are need to deal with correctly), and I suspect many others are in
> the same boat.  In addition, if your tests are in the git.git repository,
> they need to say test_expect_success for people with patched cvsps and
> test_expect_failure for people without, and because I suspect that the
> majority of git developers do not run bleeding edge cvsps, it does not do
> anything but slowing down the test suite.
> 
> It feels as if you are scratching my feet through my shoes while I still
> am wearing them.  I wonder if it would be more direct and simpler approach
> to add tests to cvsps and handle these improvements as part of the cvsps
> maintenance/development effort, not as part of cvsimport fixes, at least
> initially.
> 
> I think it is great that you started actively working on identifying and
> fixing issues with cvsps, that many others have gave up and gone to
> different avenues, and I certainly do not mind keeping the new tests in
> 'pu' for wider exposure, in order to make it easier for other people who
> use cvsimport and want to collaborate with you improving it through
> improving cvsps.
> 
> But I am starting to think that it was a mistake on my part to have merged
> the initial set of tests to 'next'.
> 
> Thoughts?