Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 2, 2013, 01:06 UTC
Message-ID: <7v1ue475re.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130102003344.GA9651@thyrsus.com>

"Eric S. Raymond" <esr@thyrsus.com> writes:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com>:
>> So..., is this a flag-day patch?
>> 
>> After this is merged, users who have been interoperating with CVS
>> repositories with the older cvsps have to install the updated cvsps
>> before using a new version of Git that ships with it?
>
> Yes, they must install an updated cvsps. But this is hardly a loss, as
> the old version was perilously broken.
>
> There was an error or typo in the branch-analysis code, dating from
> 2006 and possibly earlier, that meant that branch root points would
> almost always be attributed to parent patchsets one patchset earlier
> than they should have been.  Shocked me when I found it - how was this
> missed for six years?

Would it be that not many people use branchy history in CVS, as merging there was soooo painful?

Show 6 quoted lines

>> Or am I being too conservative?
>
> I think you are being too conservative.  This patch is *not* a mere
> feature upgrade. The branch-analysis bug I found three days ago is not
> a minor problem, it is a big ugly showstopper for any case beside the
> simplest linear histories.  Only linear histories will not break.

That is exactly my point. It never worked in a branchy history, and that is an indication that people who didn't complain and used the old cvsimport with branch-incapable cvsps happily would have been working with a linear history. Either nobody uses cvsimport in the daily work, in which case a flag-day is perfectly fine, or we will have many people who are forced to update to unproven version for no immediate upside because the upstream repositories they work with, or options they use cvsimport, do not trigger the multi-branch bug.

I however do understand that updating is the only sensible thing to do for them *in the longer term*, as older cvsimport and cvsps are no longer maintained, and sooner they update the better the chance the new cvsimport becomes perfect earlier.

> 'People with existing set-ups' should absolutely *not* 'keep using the
> old one'; we should yank that choice away from them and get the old
> cvsimport/cvsps pair out of use *as fast as possible*, because it
> silently mangles branchy imports.

I still am not convinced, especially without a "we make sure we do not regress in linear histories" side-by-side test in place. That sounds irresponsible.

But others may disagree, and I'd have to sleep on it.

I'd prefer to hear from somebody who is *not* defending on his newer implementation, but from somebody who is actively using cvsimport as an end user. On the end-users' side, there always is this anxiety that a radical rewrite will always introduce new bugs, even when they know the rewrite is done very competently.

> Here is what I have done to ease the transition:
>
> If you try to use old git-cvsimport with new cvsps, new cvsps will detect
> this and ship a message to stderr telling you to upgrade

Sounds sensible.

> If you try to use new git-cvsimport with old cvsps, old cvsps will complain
> of an invalid argument and git-cvsimport will quit.

With an error message that tells the user to update cvsps, this also sounds sensible.