Re: [PATCHv2] write_index: optionally allow broken null sha1s
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 26, 2013, 16:02 UTC
- Message-ID
- <xmqq38pwjv89.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20130826143135.GB14858@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> I found this version more readable than Peff's (albeit slightly). > > OK. Do you want to apply with Jonathan's wording, then?
I can do that, as it seems all of us are in agreement.
Show 11 quoted lines
> There's one subtle thing I didn't mention in the "it is already on stack > overflow". If you have a version of git which complains about the null > sha1, then the SO advice is already broken. But if the SO works, then > you do not have a version of git which complains. So why do you care? > > And the answer is: you may be pushing to a remote with a version of git > that complains, and which has receive.fsckObjects set (and in many > cases, that remote is GitHub, since we have had that check on for a > while). > > I don't know if it is worth spelling that out or not.
Probably not.
You could aim to correct each and every wrong suggestions on a site where misguided leads other misguided, but it is a hopeless task.
Show 15 quoted lines
>> > After this patch, do you think (in a separate change) it would make >> > sense for cache-tree.c::update_one() to check for null sha1 and error >> > out unless GIT_ALLOW_NULL_SHA1 is true? That would let us get rid of >> > the caveat from the last paragraph. >> >> Hmm, interesting thought. > > I think it is worth doing. The main reason I put the original check on > writing to the index is that it more clearly pinpoints the source of the > error. If we just died during git-write-tree, then you know somebody > broke your index, but you don't know which command. > > But checking in both places would add extra protection, and would make > possible the "relax on read, strict on write" policy that filter-branch > wants to do.
Yeah, I agree with all of the above.