Re: [PATCHv2] write_index: optionally allow broken null sha1s

Jeff King [off-list ref] writes:

quoted

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.

quoted

quoted

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.