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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 13, 2013, 22:20 UTC
Message-ID: <7v8v7wiv3a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130112182649.GC4624@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 31 quoted lines

> Michael Haggerty wrote:
>
>> 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.
>
> Moreover, I feel an obligation to point the following out:
>
> In a hypothetical world where cvsps 3.x simply breaks "git cvsimport"
> it is likely that some distributions would just stick to the existing
> cvsps and not upgrade to 3.x.  Maybe that's a wrong choice, but that's
> a choice some would make.  An even more likely outcome in that
> hypothetical world is that they would ship it renamed to something
> like "cvsps3" alongside the existing cvsps.  Or they could rename the
> old version to "cvsps2".  If we were the last holdout, we could even
> bundle it as compat/cvsps.
>
> So please do not act as though the cvsps upgrade is a crisis that we
> need to break ourselves for at threat of no longer working at all.
> The threat doesn't hold water.
>
> Luckily you have already written patches to make "git cvsimport" work
> with cvsps 3.x, and through your work you are making a better
> argument: "The new cvsimport + cvsps will work better, at least for
> some users, than the old tool."
>
> Just don't pretend you have the power to force a change for a less
> sensible reason than that!

After a quick survey of various distros, I think it is very unlikely that we will see "distros move on to newer cvsps, leaving cvsimport broken" situation. If anything, it is more like "distros decide to ignore the new cvsps, until it is made to work with cvsimport" [*1*].

I think it is probably sensible to rename the current cvsimport to cvsimport-2, write a small wrapper git-cvsimport.sh which is something like this:

----- >8 ----- #!/bin/sh

if test -z "$GIT_CVSPS_VERSION"
then
	case "$(cvsps -h 2>&1 | grep 'cvsps version')" in
        2.*)
		GIT_CVSPS_VERSION=2
                ;;
	3.*)
		GIT_CVSPS_VERSION=3
                ;;
	esac
fi

if test -z "$GIT_CVSPS_VERSION" 
then
	echo >&2 "No supported cvsps available"
	exit 1
fi

exec git cvsimport-$GIT_CVSPS_VERSION "$@" ----- 8< -----

and put Eric's one as git-cvsimport-3 (after ripping out the code to fallback to the old cvsimport). The longer term trend will be to move away from cvsimport-2, as it is unlikely cvsps-2.x will gain improvements, if any; keeping fallback code outside cvsimport-3 will be a better first step in the healthier long term code evolution.

We will keep the current t96xx series of tests, and have them export GIT_CVSPS_VERSION=2 at the beginning, protect them with test prereq that requires presence of cvsps 2.x; this will still make sure that the current cvsimport users will not see any regressions.

Eric's one should be polished enough to produce good results on the simpler sample CVS histories t96xx deal with soonish if not right now, so we can use a method similar to how we shared tests between blame and annotate while both were _different_ implementations to make sure the newer blame did not inroduce regression by running the same set of tests. Where the result _ought_ to differ, we should also add tests that work only with the new cvsimport, of course.

I could help getting the ball rolling on this, if everybody agrees that the above is a sensible direction to go, given the real world constraints of distro inertia.

Agreed?

[References]

*1* Fedora, Debian and Ubuntu do not even have cvsps 3.x in their bleeding edges, OpenBSD and NetBSD do not seem to have it either, and Gentoo and ArchLinux have the cvsps 3.x blocked due to incompatiblity.

http://pkgs.fedoraproject.org/cgit/cvsps.git/ http://packages.debian.org/search?keywords=cvsps http://packages.ubuntu.com/search?keywords=cvsps

http://packages.gentoo.org/package/dev-vcs/cvsps https://bugs.gentoo.org/show_bug.cgi?id=450424

https://bugs.archlinux.org/task/33363?project=1&cat%5B0%5D=2&string=cvsps