On Fri, Nov 22, 2013 at 12:24 PM, Jeff King <peff@xxxxxxxx> 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:
>>
>> #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().
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.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html