Re: [PATCH] drop support for "experimental" loose objects

On Fri, Nov 22, 2013 at 12:04:01PM +0100, Christian Couder wrote:

In "sha1_file.c", there is:

void *read_sha1_file_extended(const unsigned char *sha1,
                              enum object_type *type,
                              unsigned long *size,
                              unsigned flag)
{
        void *data;
        char *path;
        const struct packed_git *p;
        const unsigned char *repl = (flag & READ_SHA1_FILE_REPLACE)
                ? lookup_replace_object(sha1) : sha1;

        errno = 0;
        data = read_object(repl, type, size);
...

And in cache.h, there is:

#define READ_SHA1_FILE_REPLACE 1
static inline void *read_sha1_file(const unsigned char *sha1, enum
object_type *type, unsigned long *size)
{
        return read_sha1_file_extended(sha1, type, size,
READ_SHA1_FILE_REPLACE);
}

So the READ_SHA1_FILE_REPLACE is a way to disable replacement at compile time.

Is it? I did not have the impression anyone would ever redefine
READ_SHA1_FILE_REPLACE at compile time, but that it was a flag that
individual callsites would pass to read_sha1_file_extended to tell them
whether they were interested in replacements. And the obvious reasons to
not be are:

  1. You are doing some operation which needs real objects, like fsck or
     generating a packfile.

  2. You have already resolved any replacements, and want to make sure
     you are getting the same object used elsewhere (e.g., because you
     already printed its size :) ).

The only site which calls read_sha1_file_extended directly and does not
pass the REPLACE flag is in streaming.c. And that looks to be a case of
(2), since we resolve the replacement at the start in open_istream().

I am kind of surprised we do not need to do so for (1) in places like
pack-objects.c. Most of that code does not use read_sha1_file,
preferring instead to find the individual pack entries (for reuse). But
there are some calls to read_sha1_file, and I wonder if there is a bug
lurking there.

-Peff