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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2008, 19:04 UTC
Message-ID: <7vwshvvgap.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1220275214-6178-1-git-send-email-felipe.contreras@gmail.com>

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?

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

> The delete command was ignoring the issue completely. The rename/copy commands
> where throwing a fatal exception.

Yes.  I think this has to be two patch series, that:

 (1) Adds the equivalent "you cannot delete what you do not have" die() in
     the delete codepath; and

 (2) Adds an option that demotes *all* the "don't touch (rename, modify,
     copy or delete) what you do not have" die()s to warning().

provided if we can give a good rationale to the latter. Otherwise we should just do (1) and forget about (2).