Jeff King <peff@xxxxxxxx> 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. > 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. >> > 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html