Re: [PATCH 0/3] fixup remaining cvsimport tests

From: Chris Rorvick <chris@rorvick.com>
Date: Jan 21, 2013, 01:34 UTC
Message-ID: <CAEUsAPYdpsbhCZfp-1w91ZiyqgEa=8TNf2MJihMViqVZmW3sRw@mail.gmail.com>
In-Reply-To: <CAEUsAPaw8EUcZFbODDj9Z-=3Ppd1CC=jvYDvuyntFkX_3V0ynQ@mail.gmail.com>

On Sun, Jan 20, 2013 at 2:17 PM, Chris Rorvick <chris@rorvick.com> wrote:

Show 54 quoted lines

> On Sun, Jan 20, 2013 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> John Keeping <john@keeping.me.uk> writes:
>>
>>> On Sun, Jan 20, 2013 at 09:22:03AM -0600, Chris Rorvick wrote:
>>>> On Sun, Jan 20, 2013 at 6:58 AM, John Keeping <john@keeping.me.uk> wrote:
>>>>> On Thu, Jan 10, 2013 at 10:27:16PM -0600, Chris Rorvick wrote:
>>>>>> These patchs apply on top of of Eric Raymond's cvsimport patch.  7 of 15
>>>>>> tests in t9600 fail, one of which is fixed w/ a cvsps patch I've sent
>>>>>> to Eric (fixes revision map.)
>>>>>
>>>>> Did you post the fix for the revision map publicly anywhere?
>>>>
>>>> It's in Eric's repo and included in version 3.8:
>>>>
>>>> https://gitorious.org/cvsps/cvsps/commit/abe81e1775a8959291f629029513d1b7160bbde6
>>>
>>> Thanks.  For some reason I thought the fix would be to
>>> git-cvsimport-3.py.  Obviously I should have read more carefully.
>>>
>>> Sorry for the noise.
>>
>> This is not a noise, though.
>>
>> Chris, how would we want to proceed?  I'd prefer at some point to
>> see cvsimport-3 to be in sync when the one patched and tested in
>> Eric's repository is proven enough.  Will Eric be the gatekeeper, or
>> will you be sending patches this way as well?
>
> I probably won't be sending any more patches on this.  My hope was to
> get cvsimport-3 (w/ cvsps as the engine) in a state such that one
> could transition from the previous version seamlessly.  But the break
> in t9605 has convinced me this is not worth the effort--even in this
> trivial case cvsps is broken.  The fuzzing logic aggregates commits
> into patch sets that have timestamps within a specified window and
> otherwise matching attributes.  This aggregation causes file-level
> commit timestamps to be lost and we are left with a single timestamp
> for the patch set: the minimum for all contained CVS commits.  When
> all commits have been processed, the patch sets are ordered
> chronologically and printed.
>
> The problem is that is that a CVS commit is rolled into a patch set
> regardless of whether the patch set's timestamp falls within the
> adjacent CVS file-level commits.  Even worse, since the patch set
> timestamp changes as subsequent commits are added (i.e., it's always
> picking the earliest) it is potentially indeterminate at the time a
> commit is added.  The result is that file revisions can be reordered
> in resulting Git import (see t9605.)  I spent some time last week
> trying to solve this but I coudln't think of anything that wasn't a
> substantial re-work of the code.
>
> I have never used cvs2git, but I suspect Eric's efforts in making it a
> potential backend for cvsimport are a better use of time.
>
> Chris

Hi Eric,

I noticed you were taken off this thread. As I mention above, I looked into the bug tested in the t9605 patch Junio applied on top of your cvsimport patch. The test was actually written for master to test the Perl/cvsps2 import, but with minor modification you can verify the problem still exists in the 3.x versions of cvsps.

I think the email above explains the problem pretty well. It's not clear to me what all the nastiness is that you've resolved with cvsps since taking over; I've been mostly concerned with importing an almost branchless repository which I thought avoided the types of problems you were addressing. But this bug can actually cause Git's main import branch to become inconsistent with CVS HEAD and you don't have to do anything too weird to get hit by it.

Fixing this seemed like it would require splitting the processing out into a couple phases and would be a fair amount of work, but maybe I'm just not looking at the problem right.

Chris