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

["Felipe Contreras" <felipe.contreras@gmail.com>]

On Mon, Sep 1, 2008 at 10:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

> "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