Re: kernel.org and GIT tree rebuilding

LT> I actually like this approach better than having delta-objects in the LT> filesystem. Partly because the pack-file is self-contained, partly because LT> it also solves the fs blocking issue, yet is still efficient to look up LT> the results without having hardlinks etc to duplicate objects virtually. LT> And when you do the packing by hand as an "archival" mechanism, it also LT> doesn't have any of the downsides that Chris' packing approach had.

After analyzing what is involved in making packed GIT integrated into read_sha1_file() [*1*], I agree 100% with the above. I mean no disrespect to what Nico has done (and I myself have done some code to work with Nico's deltified objects when I did diffs and pull fixes), but it would help the code very much if we do not have to worry about "delta" objects in GIT_OBJECT_DIRECTORY.

My preference is to do things in this order:

 (0) concatenate pack and idx files;

 (1) teach read_sha1_file() to read from packed GIT;

 (2) teach fsck-cache about packed GIT;

 (3) have people with deltified repositories convert them back
     to undeltified (I think git-pack-objects would barf on such
     repository);

 (4) drop "delta" objects from GIT_OBJECT_DIRECTORY; this means
     that git-deltafy-script and git-mkdelta have to go.

 (5) tell git-*-pull about packed GIT;

[Footnotes]

*1* Here is the analysis I did last night, still assuming that we would support "delta" objects in GIT_OBJECT_DIRECTORY. The "trickier" map_sha1_file() users almost all involve "delta" objects, and that is why I prefer dropping them.

 - Enhance GIT_ALTERNATE_OBJECT_DIRECTORIES mechanism so that
   its component can be either a directory or a packed file.

 - sha1_file.c::find_sha1_file() has to be enhanced to express
   not just path (in the current "individual object file"
   case) but a pointer to a structure that describes a packed
   file in the GIT_ALTERNATE_OBJECT_DIRECTORIES list with the
   offset for the entry.

 - The change necessary to sha1_file.c::has_sha1_file() is
   minimum.  find_sha1_file() updated along the above lines
   would say if the thing exists or not anyway, so it can just
   return true/false as it currently does pretty easily.

 - sha1_file.c::read_sha1_file() would be the primary piece to
   unpack from the packed representation.

 - sha1_file.c::map_sha1_file() is trickier.  It has handful
   callers outside sha1_file.c for valid reasons, so we will
   need to audit the callers and have them fall back on
   read_sha1_file() as appropriate.  Here is the result of my
   first pass:

   - (easy) sha1_delta_base() is used only when an object is
     delitified, and if true get to the base object.  We can
     just tell the caller our object is not deltified when it
     resides in a packed file.

   - (easy) sha1_file_size() is used by diffcore to measure the
     expanded blob size.  Although the implementation obviously
     has to be different, it would be trivial to find the size
     if the object resides in a packed file.

   - (easy) pack-objects.c::check_object() uses map_sha1_file() so that
     it can unpack small to get the type of the object.  We
     should be able to introduce a new interface (say,
     sha1_file.c::sha1_object_type()) for doing this sort of
     stuff.

   - (harder) mkdelta.c::get_buffer(), object.c::parse_object()
     and delta.c::process_delta() are trickier, because they
     want to treat "delta" as a raw object (otherwise we would
     have just done sha1_read_file() instead of
     map/unpack_sha1_file pair).

   - (harder) ssh-push.c::serve_object() also wants raw
     representation to directly ship to the other end.