On 05/15/2014 07:29 PM, Ronnie Sahlberg wrote: > Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete. > This flag indicates that the ref does not exist as a loose ref andf only as > a packed ref. If this is the case we then change the commit code so that > we skip taking out a lock file and we skip calling delete_ref_loose. > Check for this flag and die(BUG:...) if used with _update or _create. > > At the start of the transaction, before we even start locking any refs, > we add all such REF_ISPACKONLY refs to delnames so that we have a list of > all pack only refs that we will be deleting during this transaction. Let me make a few comments about how I think ref packing fits into the scheme for pluggable reference back ends. The comments may or may not be relevant to this particular commit. We want to have a reference API that can be implemented by various back ends. Obviously we need operations to read a reference, enumerate existing references, update references within a transaction, and probably do similar things with symbolic references. The status quo is that we have a single reference back end consisting of loose references sitting on top of packed references. But really, loose references and packed references are two relatively independent reference back ends [1]. We just happen to use them layered on top of each other. This suggests to me that our current structure is best modeled as two independent reference back ends, with a third implementation of the same reference API whose job it is to compose the first two. In pseudocode, interface ReferenceBackend: read_ref(refname) __iter__(...) begin_transaction(...) update_reference(...) ... commit_transaction(...) class LooseReferenceBackend(ReferenceBackend): ... class PackedReferenceBackend(ReferenceBackend): ... class StackedReferenceBackend(ReferenceBackend): def __init__(self, backend1, backend2): self.backend1 = backend1 self.backend2 = backend2 def read_ref(refname): try: return backend1.read_ref(refname) except ReferenceNotFound: return backend2.read_ref(refname) def __iter__(...): i1 = self.backend1.iterate() i2 = self.backend2.iterate() while i1.has_next() and i2.has_next(): if i1.peek_next() < i2.peek_next(): yield i1.next() elif i1.peek_next() == i2.peek_next(): yield i1.next() i2.next() # discard else yield i2.next() while i1.has_next(): yield i1.next() while i2.has_next(): yield i2.next() def pack_refs(...): ... et cetera. >From this point of view it is clear that packing refs is not an operation that belongs in the ReferenceBackend API, but rather in the StackedReferenceBackend interface. The reason is that it doesn't affect how references appear to the outside world, but rather just reorganizes them between the two backends to which StackedReferenceBackend delegates. The *implementation* of pack_refs() involves adding references to backend2 and deleting them from backend1. But since adding and deleting references *are* part of the ReferenceBackend API, StackedReferenceBackend can compose *any arbitrary two* reference backends (including other StackedReferenceBackends). Pruning references that have been packed is also not part of the ReferenceBackend API. Rather it is a plain old deletion of a reference from backend1 of a StackedReferenceBackend. I haven't thought through all of the ramifications of this design, but I'm pretty sure it's where we want to be headed. Michael [1] Forget for the sake of this discussion that we can't store symbolic references as packed refs. -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- 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