Yeah I can understand your desire here. I generally don't love using magic sentinel values to mean semantically special things (like nonexistent).

The docs for this function don't say that NULL is permitted for create_matching (and indeed taking an old_id is the only distinction between this and reference_create).

I think that my preference would be to change create_matching to treat a NULL old_id here to mean nonexistent instead of using the all zeros oid.

@carlosmn what are your thoughts here?

Taking a null OID would work, but it would require changes in NodeGit, which presently doesn't allow a null OID there. Possibly also in other clients.

Also, it might be nice to allow a null OID to mean "don't care", so that callers can call the only one function to implement things like git update-ref, rather than having to say if (...).

Also, if someone is already passing a null old_id, yes, they are technically doing something that has no documentation. But their code works, and this will (potentially) break it.

NULL is absence of the value, so it shouldn't indicate anything other than the caller does not want to pass in a value. Without looking at the docs I would expect a NULL value to mean that we don't want to make any assertions about the previous value. The all-zeroes oid is already used in quite a few places to indicate absense, and using it as such would reduce the distinction between git and libgit2 semantics.

If we take a look at git help update-ref we can see that for both the single-command mode (which is basically what we're doing here) as well as for the multi-command --stdin mode, update and verify both take a "zero" value to indicate absence (this is also how you can delete via update).

So yeah, this behaviour seems fine.

The all-zeroes oid is already used in quite a few places to indicate absense

I don't think that I'd go so far as to say "quite a few". Grepping, I see one in diff where we sort of expose that all zeros is meaningful, and another in blame. More relevantly, I don't see any place where we take an ID of zeros and treat it as a meaningful value. I think that this is introduces a new concept.

@novalis can you update the docs to reflect this change?

I was thinking more in the "Git world"; you are right that we don't have this concept much in the library and I think for the most part we don't need it as we'd just expect the user to check for absence etc on their own.

I updated the docs -- can we merge this?

I added a test that exercises the failure case here and manually merged.