Re: [PATCH] Make object contents optionally available

DB> I'm already going to add a per-type global to have the parse functions DB> also unpack the object contents user-visibly, for the case that Junio DB> pointed out. (Making it: parse_* doesn't change whether the contents are DB> unpacked, unless you tell it to unpack objects.)

That sounds better than the patch you sent last night, but are we sure that callers would be satisfied if you just make some _types_ more special than others?

Parse functions need to unpack anyway, so it might make more sense to add an optional callback just after where unpacking happens to ask the main program if it wants the unpacked raw data to be kept. So you would do something like:

    struct object *parse_object(unsigned char *sha1)
    {
   ...
                    unsigned long size;
                    void *buffer = unpack_sha1_file(map, mapsize, type, &size);
                    munmap(map, mapsize);
                    if (!buffer)
                            return NULL;
   ...             
                    } else {
                            obj = NULL;
                    }
+		    if (obj && keep_object_raw_data(sha1, type, size, buffer)) {
+                           obj.raw_data = buffer;
+                           obj.raw_size = size;
+                   } else 
                            free(buffer);
                    return obj;
            }
            return NULL;
    }

And put a dummy implementation of keep_object_raw_data() that says "I do not want anything to be kept" in the libgit.a. Main programs that _care_ about raw data can implement their own keep_object_raw_data() callback that is more intelligent. In addition you give them the interface to free raw data you already have.

DB> I think the only likely bug would be unpacking objects after parsing DB> them, instead of before, which is correct but inefficient. It should be DB> clear to a user whether the raw contents are available at any point in the DB> user code.

Another possibility is not to make double unpacking too costly by having an LRU of unpack_sha1_file(), but I am not sure how effective that would be.