Re: [PATCH] fast-import: catch deletion of non-existent file in input

[Jeff King <peff@peff.net>]

On Sun, Jul 15, 2012 at 01:11:51PM -0500, Jonathan Nieder wrote:

> > Subject: fast-import: catch deletion of non-existent file in input
> [...]
> > We silently ignored the bogus "D foo" directive, and the
> > resulting tree incorrectly contained "bar". With this patch,
> > we notice the bogus input and die.
> 
> This breaks svn-fe, which relies on the existing semantics when asked
> to copy an empty directory.

Thanks for the report. I had a worry while writing this that somebody
was relying on the behavior. Let's just drop it, then. It's nice to
catch errors in exporters, but not at the expense of compatibility
issues.

We could introduce a new feature bit, but I'm not sure it is really
worthwhile. The older versions of bzr-fast-export would not set the bit
anyway, and newer versions are already fixed, so it is kind of closing
the barn door after the horse has left (we might catch other bugs, but
this one is kind of oddly specific; if somebody wanted to audit
fast-import for other similar cases and introduce a "strict" feature
bit, that might be worthwhile. But for this single change, I don't think
so).

> Let's repeat that for emphasis: API breaks in fast-import not guarded
> with a new "feature" type are not ok.

Totally agree. The question in my mind was whether this was a bug fix or
an API change, and it sounds like it is too far towards the latter.

-Peff