Re: [PATCH] builtin-fast-export.c: add default case to avoid crash on corrupt repo

[Junio C Hamano <gitster@pobox.com>]

Erik Faye-Lund <kusmabite@googlemail.com> writes:

> Anyway, I guess this makes the most sense as a four-patch series:
> 1) Add test-cases for tags of tag objects, tag objects of tag objects,
> tags of trees and tag objects of trees.
> 2) Turn the existing error into a warning
> 3) Add the missing warning and remove the crash
> 4) Fix fast-export to export tags pointing to tags.
>
> Makes sense?

Yes.

I suspect we will hear an argument that say the dying is deliberate
because pretending to have produced a faithful export while some
information is lacking is bad and warnings are easy to miss, and I would
certainly understand that argument.

We have similar issue with signed tags and filter-branch, which strips the
signature when it rewrites the history.  I recall the code originally died
for the same reasoning, but asking "it died; what can the user do now?"
made us realize that it is the best we can do to make best-effort
reproduction with a warning when losing information.

This falls into the same category.  I think "we die by default if we
cannot give a faithful reproduction, and the user needs to give an
explicit permission (e.g. --force option) to lose information", is a very
sensible thing to do, but "we die if we cannot export some things, and we
refuse to produce an approximation." without an escape hatch is not.