Re: [PATCH v7 2/2] submodule: skip redundant active entries when pattern covers path
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 4, 2025, 04:22 UTC
- Message-ID
- <xmqqh60wf850.fsf@gitster.g>
- In-Reply-To
- <CA+rGoLfpj2tepMSWLfNeVkwXfzHZB7Vc8_GJ+_=bWkQSzZ+Sjg@mail.gmail.com>
JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:
Show 9 quoted lines
>> The new helper is a maintenance burden to keep in sync with >> submodule.c:is_tree_submodule_active(); if we really want to go this >> route, the patch should extract that "ah, submodule.active is set so >> let's turn it into pathspec and see if the path matches" part of the >> logic to make sure the logic is shared. But I am wondering if we >> can do this without any new helper. > > I've actually done something like this, but I've wrapped the core logic within > the if else after checking [1]
I do not think I follow what you want to say here. Care to rephrase?
Show 23 quoted lines
>> I.e. Can we replace this if() condition with something like this?
>>
>> /*
>> * Explicitly set 'submodule.<name>.active' only if it is not
>> * 'active' due to other reasons.
>> */
>> if (!is_submodule_active(the_repository, add_data->sm_path)) {
>>
>> That is, we ask if the submodule is already active (we are before
>> adding submodule.<name>.active for this thing---if may be active due
>> to submodule.active or submodule.<name>.url) and enter the block
>> only when it is not yet.
>>
>> That way, this codepath does not have to worry about the exact logic
>> that determines if a submodule is 'active' even when its .active
>> configuration variable is not set.
>>
>> > key = xstrfmt("submodule.%s.active", add_data->sm_name);
>> > git_config_set_gently(key, "true");
>> > free(key);
>
>
> I've done the exact thing in a bit different wayI am confused again, as this reads as if it is an oxymoron --- did you write the exactly the same thing as suggested above, or did you write something different?