On Fri, Sep 6, 2013 at 8:25 PM, Nicolas Pitre <nico@xxxxxxxxxxx> wrote: > On Fri, 6 Sep 2013, Duy Nguyen wrote: > >> On Fri, Sep 6, 2013 at 10:23 AM, Nicolas Pitre <nico@xxxxxxxxxxx> wrote: >> > On Fri, 6 Sep 2013, Nguyn Thái Ng÷c Duy wrote: >> > >> >> >> >> Signed-off-by: Nguyn Thái Ng÷c Duy <pclouds@xxxxxxxxx> >> >> --- >> >> Should be up to date with Nico's latest implementation and also cover >> >> additions to the format that everybody seems to agree on: >> >> >> >> - new types for canonical trees and commits >> >> - sha-1 table covering missing objects in thin packs >> > >> > Great! I've merged this into my branch with the following amendment: >> >> I'd like to propose another change in the format to basically limit >> the use of sha1ref format "\0<SHA-1>" to tree entries only. All forms >> of deltas must have non-zero sha1 index (i.e. reference to SHA-1 >> table). It will simplify handling code, and I think it makes sense too > > Why? > > This is still some artificial limitation and I tend not to like them. > > "Simplifying handling code" is not a good enough reason on its own, > especially if it reduce flexibility for possible future layout > optimizations. > > In what way is the code more difficult? That'll be two ways of linking to another in-pack object. The linked object must be in the pack anyway, its sha-1 should be present in the sha-1 table. "\0<sha1>" is a longer encoding for nothing. If the linked sha1 is not in the pack (similar to the old ref-delta), it makes the pack depend on another one, which is what we've been avoding. In your code you reject sha1ref starting with zero too (sha1_file.c::get_base_delta and packv4-parse.c::decode_entries) -- Duy -- 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