On Fri, Nov 22, 2013 at 12:24 PM, Jeff King <p...@peff.net> wrote:
> 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:
>> 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,
>> }
>> 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().

Yeah, you are right. Sorry for overlooking this.

But anyway it looks redundant to me to have both this REPLACE flag and
the read_replace_refs global variable, so I think a proper solution
would involve some significant refactoring.

And if we decide to keep a REPLACE flag we might need to add one to
sha1_object_info_extended() too.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to