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().