Re: Index format v5

> I've looked over the writing side of git-convert-index.py version
> 81411fe6c98, and here are my first comments:
> 
> * Please remove trailing whitespace from the source code.
> 
> * I suggest that you move constants and code shared by
>   git-convert-index.py and git-read-index-v5.py into a library.  Though
>   actually, given that git doesn't seem to have infrastructure for
>   dealing with Python libraries, this might take some improvisation.

> * Please use constants for all of the struct formats.  Constants have
>   names, making them mostly self-documenting.
> 
> * write_directories() currently writes pathnames and fake data and
>   stores file offsets in memory.  Later write_directory_data() runs
>   through the file again, seek()ing over the filenames and filling in
>   real data.
> 
>   Wouldn't it be easier for the first pass just to *compute* and
>   record the offsets of the entries to RAM, without writing anything
>   to disk, and leave all of the writing to the second pass?

> * Instead of writing blank data, it is possible to seek() past it and
>   start writing the next thing.  The skipped-over file contents are
>   logically initialized to zero.
> 
> * When working with iteritems(), it is clearer to unpack the item
>   pairs and give them names rather than working with d[0] and d[1];
>   for example,
> 
>     -    for d in sorted(dirdata.iteritems()):
>     +    for (pathname,entry) in sorted(dirdata.iteritems()):
> 
> * write_directories() returns a "dirdata" that is just an empty
>   defaultdict.  This seems pointless.  Do you have future plans to
>   change write_directories() to store something into the dictionary?
> 
> * The documentation for binascii.crc32() mentions that it gives
>   inconsistent results (signed vs. unsigned) for different versions of
>   Python.  Please ensure that you are using it in a way that is
>   maximally portable.  (That seems to imply using (binascii.crc32(...)
>   & 0xffffffff) and treating the result as unsigned.)
> 
> * At first I thought it was a little bit odd that you pass data
>   structures around as dictionaries, but I didn't object.  But as I
>   look at more and more code it seems more and more cumbersome.
>   Therefore, I suggest that you define classes to hold the various
>   entities that are manipulated by your programs, because:
> 
>   * A class definition is a good place to document exactly what fields
>     an object is expected to have, and what they mean.
> 
>   * Access of instance fields (entry.path) is easier to read and type
>     than dictionary access (entry["path"]).
> 
>   * The class definitions will translate pretty directly to C structs.
> 
>   The fact that class instances use a bit more memory than
>   dictionaries is, I think, unimportant.  But if that really bothers
>   you, you can use __slots__ to save some of the instance memory.
> 
> At a higher level:
> 
> * What if the offsets to each section were stored in the header, and
>   the offsets recorded for dirs and files were relative to the start
>   of the section (rather than relative to the start of the file)?  I
>   think that this would leave open the possibility of formatting the
>   sections in memory in parallel in a single pass, then dumping the
>   sections to disk in a few big writes (though I'm not saying that this
>   should be the *default* way of writing).

> * Do you plan to write prototypes for some of the cool new
>   functionality that v5 is intended to make possible?  For example,
> 
>   * reading a few specific entries out of an index file
> 
>   * updating single entries
> 
>   * adding/removing conflict data to an existing file
> 
>   * dealing with all of the issues that will come with supporting the
>     mutation of an existing index file (i.e., locking, consistency
>     checks, etc)
> 
>   As you probably know from discussions on IRC, I think that the last
>   of these is the biggest risk to the success of the project.