Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Wed, 22 Jul 2009, Junio C Hamano wrote:
>>
>> Hmm, we now have to remember what this patch did, if we ever wanted to
>> introduce negative refs later (see ef06b91 do_for_each_ref: perform the
>> same sanity check for leftovers., 2006-11-18). Not exactly nice to spread
>> the codepaths that need to be updated.
> ...
> And since git branch will do something _better_ than the 'has_sha1_file()'
> check (by virtue of actually looking up the commit), I don't think that
> part is an issue. So the only issue is the is_null_sha1() thing.
Exactly.
That is_null_sha1() thing was a remnant of your idea to represent deleted
ref that has a packed counterpart by storing 0{40} in a loose ref, so that
we can implement deletion efficiently.
Since we currently implement deletion by repacking packed refs if the ref
has a packed (possibly stale) one, we do not use such a "negative ref",
and skipping 0{40} done by the normal (i.e. non-raw) for_each_ref() family
is not necessary.
I was inclined to say that, because I never saw anybody complained that
deleting refs was too slow, we declare that we would forever stick to the
current implementation of ref deletion, and remove the is_null_sha1()
check from the do_one_ref() function, even for include-broken case.
But after thinking about it again, I'd say "if null, then skip" should be
outside the DO_FOR_EACH_INCLUDE_BROKEN anyway, because the null check is
not about brokenness of the ref, but is about a possible future expansion
to represent deleted ref with such a "negative ref" entry.
If we remove is_null_sha1() from do_one_ref(), or if we move it out of the
"include broken" thing, my "Not exactly nice" comment can be rescinded, as
doing the former (i.e. removal of is_null_sha1() check) is a promise that
we will never have to worry about negative refs, and doing the latter will
still protect callers of do_for_each_rawref() from negative refs if we
ever introduce them in some future.
--
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