Re: [PATCH] git-daemon server

Looks very nice.  Some comments.

    diff -u b/daemon.c b/daemon.c
    --- b/daemon.c
    +++ b/daemon.c
------------
    +
    +/* Protocol is symmetric, both client and server
    + * use the same commands.
    + *
    + * version\n -> error -- 0 <Version string>\n
    + *
    + * head <head-id>\n -> sha1 <head-id> <sha1>\n
    + *
    + * head <head-id> <old-sha1> <new-sha1>\n -> sha1 <head-id> <new-sha1>\n
    + *
    + * request <sha1>\n -> send <sha1> <hex-bytes>\n<bytes...>
    + *
    + * send <sha1> <hex-bytes>\n<bytes...> -> sha1 -- <sha1>\n
    + *
    + * exists <sha1>\n -> sha1 -- <sha1>\n
    + *
    + * sha1 <any> <sha1>\n -> no-op
    + *
    + * error <key> <hex-code> <error string>\n -> no-op
    + *
    + */

This is good for the first cut, but I have a latency concern about "single request - single send" style of communication. This being a dedicated GIT specific sync mechanism, you may want to give more smarts to the server, so that the client can say "I have these commits as HEADs in my forest, here are their SHA1s, now sync me up to the head you said you have whose SHA1 is this", implying he has all their HEADs dependents. Of course this can come later.

------------

    +static int verify_file(int fd, unsigned long mapsize, const unsigned char *sha1, char *type)
    +{
    +	void *map, *buffer;
   ~~~ 
    +		if (buffer && !strcmp(type, "delta")) {
    +			void *ref = NULL, *delta = buffer;
    +			unsigned long ref_size, delta_size = size;
    +			buffer = NULL;
   ~~~
    +			buffer = patch_delta(ref, ref_size,
    +					  delta+20, delta_size-20, 
    +					  &size);
    +			free(delta);
    +			free(ref);
    +		}

A possibility is to chuck the above special case for "delta", and instead introduce "delta" subclass in struct object family and make the base object of "delta" simply one object on the obj->refs list on such a "delta" object. I would imagine that this would let you reuse the sha1_retrieve() loop that you already do for "commit", "tree", and "tag".

    +		if (!strcmp(type, "blob")) {
    +			struct blob *blob = lookup_blob(sha1);
    +			parse_blob_buffer(blob, buffer, size);
   ~~~
    +			obj = &tag->object;
    +		} else {
    +			obj = NULL;
    +		}
    +
    +		free(buffer);
    +
    +		if (obj) {
    +			struct object_list *refs;
    +
    +			for (refs = obj->refs; refs ; refs = refs->next) {
    +				err = sha1_retrieve(refs->item->sha1);
    +				if (err < 0)
    +					return err;
    +			}
    +		
    +			return 0;
    +		}
    +
    +	}
    +	return -1;
    +}

------------

    +static int send_send(const unsigned char *sha1, int size, void *data)
    +{
   ~~~
    +}
   ~~~
    +static int cmd_request(int argc, char **argv)
    +{
    +	int err;
    +
    +	if (argc == 2) {
    +		char sha1[40];
    +		void *data;
    +		unsigned long size;
   ~~~
    +		err = send_send(sha1, size, data);
    +		if (err < 0)
    +			return err;

By definition, size of SHA1 blob is "unsigned long" so send_send() should take such not "int".