Re: [RFC_PATCHv4 3/7] submodule-config: add method to check for being labeled

From: Stefan Beller <sbeller@google.com>
Date: Mar 23, 2016, 21:05 UTC
Message-ID: <CAGZ79kYtS_TBMnNhK6FjkQcJt=r1hnFsx=TyatoENjEXBAmRug@mail.gmail.com>
In-Reply-To: <xmqqegb2f8nn.fsf@gitster.mtv.corp.google.com>

On Tue, Mar 22, 2016 at 3:30 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> In later patches we need to tell if a submodule is labeled by
>> the given labels.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> Hmph, I would have expected that something like this would touch the
> module_list() implementation.  Probably that would happen in future
> steps, I guess?

I would have guessed, too. And I was about to say Jens argued against that in an earlier patch series [1]. However I miss remembered and Jens pointed at "git submodule init --label/groups" being a bad idea, not in the helper.

So we could still have a "git submodule--helper labels_apply <submodule>" query to expose this to shell parts. Most of the shell parts use module_list so we could integrate that there too. ("git submodule_helper list --labeled-only")

[1] $gmane/281670, specifically $gmane/281720

Show 73 quoted lines

>
>>  submodule-config.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  submodule-config.h |  3 +++
>>  2 files changed, 51 insertions(+)
>>
>> diff --git a/submodule-config.c b/submodule-config.c
>> index 7b48e59..b10a773 100644
>> --- a/submodule-config.c
>> +++ b/submodule-config.c
>> @@ -493,3 +493,51 @@ void submodule_free(void)
>>       cache_free(&cache);
>>       is_cache_init = 0;
>>  }
>> +
>> +int submodule_applicable_by_labels(const struct string_list *list,
>> +                                const struct submodule *sub)
>> +{
>> +     int label_apply = 0;
>> +     struct strbuf sb = STRBUF_INIT;
>> +
>> +     if (!list)
>> +             return 1;
>> +
>> +     if (sub->labels) {
>> +             struct string_list_item *item;
>> +             for_each_string_list_item(item, sub->labels) {
>> +                     strbuf_reset(&sb);
>> +                     strbuf_addf(&sb, "*%s", item->string);
>> +                     if (string_list_has_string(list, sb.buf)) {
>> +                             label_apply = 1;
>> +                             break;
>> +                     }
>> +             }
>> +     }
>> +     if (sub->path) {
>> +             /*
>> +              * NEEDSWORK: This currently works only for
>> +              * exact paths, but we want to enable
>> +              * inexact matches such wildcards.
>> +              */
>> +             strbuf_reset(&sb);
>> +             strbuf_addf(&sb, "./%s", sub->path);
>> +             if (string_list_has_string(list, sb.buf)) {
>> +                     label_apply = 1;
>> +             }
>> +     }
>> +     if (sub->name) {
>> +             /*
>> +              * NEEDSWORK: Same as with path. Do we want to
>> +              * support wildcards or such?
>> +              */
>> +             strbuf_reset(&sb);
>> +             strbuf_addf(&sb, ":%s", sub->name);
>> +             if (string_list_has_string(list, sb.buf)) {
>> +                     label_apply = 1;
>> +             }
>> +     }
>> +     strbuf_release(&sb);
>> +
>> +     return label_apply;
>> +}
>> diff --git a/submodule-config.h b/submodule-config.h
>> index 8d61df3..d67f666 100644
>> --- a/submodule-config.h
>> +++ b/submodule-config.h
>> @@ -30,4 +30,7 @@ const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
>>               const char *path);
>>  void submodule_free(void);
>>
>> +int submodule_applicable_by_labels(const struct string_list *list,
>> +                                const struct submodule *sub);
>> +
>>  #endif /* SUBMODULE_CONFIG_H */