From: Felipe Contreras Date: Mon, 01 Sep 2008 21:58:59 GMT Subject: Re: [PATCH 1/1] fast-import: show a warning for non-existent files. 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 wrote: > Felipe Contreras 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. -- Felipe Contreras