Re: [PATCH v2 2/3] replace-objects: create wrapper around setting

From: Derrick Stolee <derrickstolee@github.com>
Date: Jun 5, 2023, 13:22 UTC
Message-ID: <0a8ab05c-f57e-8d88-30cb-3d979e65d148@github.com>
In-Reply-To: <a1967c58-48c5-6ae0-f60a-2d8c107f8f66@web.de>

On 6/3/2023 2:22 AM, René Scharfe wrote:

Show 20 quoted lines

> Am 02.06.23 um 16:29 schrieb Derrick Stolee via GitGitGadget:
>> diff --git a/replace-object.c b/replace-object.c
>> index ceec81c940c..cf91c3ba456 100644
>> --- a/replace-object.c
>> +++ b/replace-object.c
>> @@ -85,7 +85,14 @@ const struct object_id *do_lookup_replace_object(struct repository *r,
>>  	die(_("replace depth too high for object %s"), oid_to_hex(oid));
>>  }
>>
>> +static int read_replace_refs = 1;
>> +
> 
> This breaks compilation:
> 
>    replace-object.c:88:12: error: static declaration of 'read_replace_refs' follows non-static declaration
>    static int read_replace_refs = 1;
>               ^
>    ./replace-object.h:14:12: note: previous declaration is here
>    extern int read_replace_refs;
>               ^

Thanks for finding this issue within the patch. The removal of the global should have happened in this patch, but I missed it when adjusting things for this version.

Show 13 quoted lines

> And this variable is still referenced in two more places outside this
> file, which won't work now that it has become static (file-scoped):
> 
>    $ git grep read_replace_refs
>    builtin/commit-graph.c:extern int read_replace_refs;
>    config.c:               read_replace_refs = git_config_bool(var, value);
>    replace-object.c: * references, regardless of the value of read_replace_refs.
>    replace-object.c:static int read_replace_refs = 1;
>    replace-object.c:       read_replace_refs = 0;
>    replace-object.c:       return read_replace_refs;
>    replace-object.h:extern int read_replace_refs;
> 
> Perhaps postpone adding "static" to patch 3?

You're right that this won't work unless we fix the config-parsing code already here, so I should move the introduction of the static global until patch 3, as you suggest.

Thanks, -Stolee