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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]