From: Junio C Hamano Date: Sun, 22 Mar 2009 21:58:53 GMT Subject: Re: [PATCH] builtin-fast-export.c: add default case to avoid crash on corrupt repo Message-ID: <7vwsahcj8i.fsf@gitster.siamese.dyndns.org> In-Reply-To: <40aa078e0903220522g66cf2172l9f1a43ed562cc4d3@mail.gmail.com> Erik Faye-Lund 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.