On Fri, Sep 2, 2016 at 12:39 PM, Shawn Pearce <spearce@xxxxxxxxxxx> wrote: > On Fri, Sep 2, 2016 at 10:15 AM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: >> >> + if (is_null_oid(&old_oid)) { >> + if (strcmp(name, "capabilities^{}")) > > Its not the zero ID that is special, its the "capabilities^{}" name > that is special when its the first entry in the stream. In the wire > protocol a "x^{}" line is a modifier to a prior "x" line to add a > peeled object to the prior line. But if we see "^{}" on the first line > that is non-sense, there is no prior line to modify with this > identifier. > > Further ^{} is used here because its invalid in a name. A server > really cannot have a reference that ends with the sequence ^{}. And a > server should not have a reference named "capabilities" without a > "refs/" prefix on it. > > So the entire "capabilities^{}" on the first line is a bunch of > contradictions that violate a number of things about the protocol, > which is why clients should ignore it. > > I think the test should be about: > > !*list && !strcmp(name, "capabilities^{}") > >> + warning("zero object ID received that is not accompanied by a " >> + "capability declaration, ignoring and continuing anyway"); > > Annoyingly a zero object ID is sort of possible; with a probability of > 1/2^160 or something. Its just a very very unlikely value. Slightly > stronger to test against the known invalid name. That ship has sailed long ago I would think? There are tests for null sha1 in the refs code, e.g. for deletion/creation of a branch we consider the null sha1 as the counterpart. So while it may be possible to have SHA1 producing a "0"x40, you cannot e.g. push it?