Re: [PATCH 1/1] fast-import: show a warning for non-existent files.

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sep 1, 2008, 21:58 UTC
Message-ID: <94a0d4530809011458r25f73f49web2c3ae601a5544d@mail.gmail.com>
In-Reply-To: <7vwshvvgap.fsf@gitster.siamese.dyndns.org>

On Mon, Sep 1, 2008 at 10:04 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> This is useful in certain SCMs like monotone, where each 'merge revision' has
>> the changes of all the micro-branches merged. So it appears as duplicated commands.
>
> The patch appears to add warning to when you try to 'D'elete something
> that should not exist in the revision, whose moral equivalents are
> implemented in the codepath to deal with 'R'enaming and 'C'opying an
> non-existent path.
>
> But instead of making it die(), it merely warns, and even worse, you are
> demoting an existing die() in Rename/Copy codepath to mere warning
> unconditionally.  Why?

Right, I tried to do two things at once. I thought adding the code in 'D'elete in a separate branch was too much separation for such a trivial thing.

Show 6 quoted lines

> "This" that begins your proposed commit log message needs to be clarified,
> but I am guessing that you are defending your change to demote existing
> error check to die on inconsistent input to a mere warning.  I do not find
> it a particularly good defending argument.  It sounds more like you are
> papering over bugs in _one_ broken converter that produces and feeds an
> incorrect input to fast-import, breaking a safety valve for everybody else.

In monotone you can have something like:

 A
/ \
B D
| |
C E
\ /
 F

If you do a 'delete foo' in B, and a 'delete bar' in F, you will get this stored in the merge revision (F):

old_revision [C] delete "foo" delete "bar"

old_revision [E] delete "bar"

All the changes of the merged branches are stored again in the merge revision.

I guess ideally my converter should check which changes appear on both branches, and only apply those. Of course, only on revisions that are merges. That complicates the code quite a lot. I tried it and it didn't work quite well (I did something wrong).

So I tried to workaround this in fast-import, and it's simple, and it works.

<snip/>

-- 
Felipe Contreras