Re: [RFC_PATCHv4 5/7] submodule update: respect submodule.actionOnLabel
Stefan Beller writes: > Maybe we can revive the term "group" and call it submodule.defaultGroup. > The defaultGroup is defined by selection of names, paths and labels. There are many ways to specify one or more of submodules: - By giving "pathspecs", you would choose submodules whose paths match them; - By giving one or more "names", you would choose the submodule with one of these names; or - By giving one or more "labels", you would choose submodules that have one of these labels. It is perfectly fine to call the chosen set of submodules "group of submodules". It does not have any connotation on what is to be done about the group, so that is probably the best word suggested so far in the thread. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC_PATCHv4 5/7] submodule update: respect submodule.actionOnLabel
On Wed, Mar 23, 2016 at 5:13 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> On Tue, Mar 22, 2016 at 3:40 PM, Junio C Hamano wrote: >>> Stefan Beller writes: >>> This change introduces the 'submodule.actionOnLabel' variable in a repository configuration. Generally speaking 'submodule.actionOnLabel' restricts the action of a command when no submodules are selected via the command line explicitely to those submodules, which are selected by 'submodule.actionOnLabel'. It can occur multiple times and can specify the path, the name or one of the labels of a submodule to select that submodule. The introduction of 'submodule.actionOnLabel' starts with 'git submodule update' in this patch and other commands will follow in later patches. 'submodule.actionOnLabel' implies '--init' in 'git submodule update'. Signed-off-by: Stefan Beller TODO: generic documentation for submodule.actionOnLabel TODO: documentation for submodule update >>> >>> TODO: a name that matches the concept better. >> >> This is one of the hardest parts of the series so far. The last reviews >> were mostly bike shedding about the name of the concept and I thought >> we were settled to actionOnLabel as that fits best to what we want to do. >> >> So let's revisit that. My current understanding of the design: > > I am not questioning the name "label" to call the facility that > allows projects to group submodules together, and that serves as one > of the ways to choose what subset of submodules are worked on by > default. There is no need to revisit that part. > > What I am questioning is > > action On Label > > because > > (1) it sounds as if that configuration were a way to choose what > action is done to the chosen subset of submodules; > > (2) it sounds as if the only way to choose a subset of submodules > to be operated on by default is via the "label" mechanism. > > And from your writing (omitted), I think we agree that we definitely > want to avoid the misunderstanding that is (1). This variable does > not specify what is done--this specifies what subset of submodules > are to be operated on. Having "action" in the name of the variable > is wrong. > > And from the proposed log message, it is clear that "label" is not > the only way to specify the subset of submodules to be worked on, > i.e. "... can specify the path, name or the labels". Having > "label" in the variable name is wrong. > > I am tempted to suggest submodule.defaultOperand and I am fairly > sure "default" part of that name gets the concept much better than > "actionOnLabel", but there probably are much better words than > Operand. I immediately thought of defaultActionset, because "default", as well as the "actionset" conveys the concept of choosing a subset of submodules to operate on. However You may mistake it as defaultActionSet, i.e. what action is set by default. Maybe we can revive the term "group" and call it submodule.defaultGroup. The defaultGroup is defined by selection of names, paths and labels. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC_PATCHv4 5/7] submodule update: respect submodule.actionOnLabel
Stefan Beller writes: > On Tue, Mar 22, 2016 at 3:40 PM, Junio C Hamano wrote: >> Stefan Beller writes: >> >>> This change introduces the 'submodule.actionOnLabel' variable >>> in a repository configuration. Generally speaking 'submodule.actionOnLabel' >>> restricts the action of a command when no submodules are selected via the >>> command line explicitely to those submodules, which are selected by >>> 'submodule.actionOnLabel'. It can occur multiple times and can specify >>> the path, the name or one of the labels of a submodule to select that >>> submodule. >>> >>> The introduction of 'submodule.actionOnLabel' starts with >>> 'git submodule update' in this patch and other commands will follow >>> in later patches. >>> >>> 'submodule.actionOnLabel' implies '--init' in 'git submodule update'. >>> >>> Signed-off-by: Stefan Beller >>> >>> TODO: generic documentation for submodule.actionOnLabel >>> TODO: documentation for submodule update >> >> TODO: a name that matches the concept better. > > This is one of the hardest parts of the series so far. The last reviews > were mostly bike shedding about the name of the concept and I thought > we were settled to actionOnLabel as that fits best to what we want to do. > > So let's revisit that. My current understanding of the design: I am not questioning the name "label" to call the facility that allows projects to group submodules together, and that serves as one of the ways to choose what subset of submodules are worked on by default. There is no need to revisit that part. What I am questioning is action On Label because (1) it sounds as if that configuration were a way to choose what action is done to the chosen subset of submodules; (2) it sounds as if the only way to choose a subset of submodules to be operated on by default is via the "label" mechanism. And from your writing (omitted), I think we agree that we definitely want to avoid the misunderstanding that is (1). This variable does not specify what is done--this specifies what subset of submodules are to be operated on. Having "action" in the name of the variable is wrong. And from the proposed log message, it is clear that "label" is not the only way to specify the subset of submodules to be worked on, i.e. "... can specify the path, name or the labels". Having "label" in the variable name is wrong. I am tempted to suggest submodule.defaultOperand and I am fairly sure "default" part of that name gets the concept much better than "actionOnLabel", but there probably are much better words than Operand. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC_PATCHv4 5/7] submodule update: respect submodule.actionOnLabel
On Tue, Mar 22, 2016 at 3:40 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> This change introduces the 'submodule.actionOnLabel' variable
>> in a repository configuration. Generally speaking 'submodule.actionOnLabel'
>> restricts the action of a command when no submodules are selected via the
>> command line explicitely to those submodules, which are selected by
>> 'submodule.actionOnLabel'. It can occur multiple times and can specify
>> the path, the name or one of the labels of a submodule to select that
>> submodule.
>>
>> The introduction of 'submodule.actionOnLabel' starts with
>> 'git submodule update' in this patch and other commands will follow
>> in later patches.
>>
>> 'submodule.actionOnLabel' implies '--init' in 'git submodule update'.
>>
>> Signed-off-by: Stefan Beller
>>
>> TODO: generic documentation for submodule.actionOnLabel
>> TODO: documentation for submodule update
>
> TODO: a name that matches the concept better.
This is one of the hardest parts of the series so far. The last reviews
were mostly bike shedding about the name of the concept and I thought
we were settled to actionOnLabel as that fits best to what we want to do.
So let's revisit that. My current understanding of the design:
Generic properties in the data model:
* Each submodule has a set of "things" attached to it. (A submodule
can have none, one or many)
* A "thing" can be attached to many submodules (That's why
it was called groups in v1, and labels now)
* The attachments of "things" to submodules can be viewed as a bipartite
graph.
* The attachment needs to work in a way, such that upstream
can influence and redefine these attachments (e.g. .gitmodules file
as part of the repo; another approach would be to have yet another file
.gitlabels or such where you'd have a list of submodules belonging to a
"thing")
* If this feature is enabled, the user can select a set of submodules by
selecting a set of "things" and all submodules connected to these things
in the bipartite graph are selected. The expectation for a graph is to
select a lot fewer "things" than submodules. By having this indirection
via the graph, the selection of a subset of submodules is expected to
be easier.
Properties I derived from discussion and the data model:
* The user does not need to have a way of overwriting the bipartite graph,
because they can specify submodules by these "things", by path or by name.
(It would be convenient to do be able to overwrite these, but it is
not a strict
requirement as the you can get any specification via a set of paths)
* The user needs to make the explicit choice to use the new feature
or not, as it has implications on the default behavior of submodule
commands.
* To make change of selection easy (which happens e.g. when switching
branches or pulling in upstream changes), all submodules are initialized
by default.
* Once this feature is enabled a command doesn't apply to all initialized
submodules by default any more, but the
default set of submodules will be the selected set via the
bipartite graph of "things".
(Originally I typed out some implementation specific thoughts, but they are
loaded with even more assumptions, so maybe we'd want to stay on this
high level first)
So any other naming proposals?
Thanks,
Stefan
>
> So in general
>
> $ git submodule $subcmd .
>
> may be the way to say "do $subcmd to all submodules", and
>
> $ git submodule $subcmd
>
> may have been "operate on nothing" (or may have been "operate on
> everything"), but with this feature,
>
> $ git submodule $subcmd
>
> will by default operate on submodules that match the criteria the
> new configuration variable specifies?
>
> I suspect that copying this from .gitmodules to .git/config will
> have security implications and will not be done? What is the
> expected way for projects to suggest which set of submodules are the
> good ones to work on by default using this mechanism?
>
>> Signed-off-by: Stefan Beller
>> ---
>> builtin/submodule--helper.c | 22 -
>> t/t7400-submodule-basic.sh | 115
>>
>> 2 files changed, 136 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index a69b1f4..93760ec 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -573,6 +573,8 @@ struct submodule_update_clone {
>> int current;
>> struct module_list list;
>> unsigned warn_if_uninitialized : 1;
>> + /* patterns to initialize */
>> + struct string_list *initialize;
>>
>> /* update parameter passed via commandline */
>> struct submodule_update_strategy update;
>> @@ -590,7 +592,7 @@ struct submodule_update_clone {
>> /* If we want to stop as fast as possible and return an error */
>> unsigned quickstop : 1;
>> };
>> -#define SUBMODULE_UPDATE_CLONE_INIT {
Re: [RFC_PATCHv4 5/7] submodule update: respect submodule.actionOnLabel
Stefan Beller writes:
> This change introduces the 'submodule.actionOnLabel' variable
> in a repository configuration. Generally speaking 'submodule.actionOnLabel'
> restricts the action of a command when no submodules are selected via the
> command line explicitely to those submodules, which are selected by
> 'submodule.actionOnLabel'. It can occur multiple times and can specify
> the path, the name or one of the labels of a submodule to select that
> submodule.
>
> The introduction of 'submodule.actionOnLabel' starts with
> 'git submodule update' in this patch and other commands will follow
> in later patches.
>
> 'submodule.actionOnLabel' implies '--init' in 'git submodule update'.
>
> Signed-off-by: Stefan Beller
>
> TODO: generic documentation for submodule.actionOnLabel
> TODO: documentation for submodule update
TODO: a name that matches the concept better.
So in general
$ git submodule $subcmd .
may be the way to say "do $subcmd to all submodules", and
$ git submodule $subcmd
may have been "operate on nothing" (or may have been "operate on
everything"), but with this feature,
$ git submodule $subcmd
will by default operate on submodules that match the criteria the
new configuration variable specifies?
I suspect that copying this from .gitmodules to .git/config will
have security implications and will not be done? What is the
expected way for projects to suggest which set of submodules are the
good ones to work on by default using this mechanism?
> Signed-off-by: Stefan Beller
> ---
> builtin/submodule--helper.c | 22 -
> t/t7400-submodule-basic.sh | 115
>
> 2 files changed, 136 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a69b1f4..93760ec 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -573,6 +573,8 @@ struct submodule_update_clone {
> int current;
> struct module_list list;
> unsigned warn_if_uninitialized : 1;
> + /* patterns to initialize */
> + struct string_list *initialize;
>
> /* update parameter passed via commandline */
> struct submodule_update_strategy update;
> @@ -590,7 +592,7 @@ struct submodule_update_clone {
> /* If we want to stop as fast as possible and return an error */
> unsigned quickstop : 1;
> };
> -#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
> +#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, NULL, \
> SUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \
> STRING_LIST_INIT_DUP, 0}
>
> @@ -644,6 +646,15 @@ static int prepare_to_clone_next_submodule(const struct
> cache_entry *ce,
> strbuf_reset(&sb);
> strbuf_addf(&sb, "submodule.%s.url", sub->name);
> git_config_get_string(sb.buf, &url);
> + if (suc->initialize) {
> + if (!url) {
> + init_submodule(sub->path, suc->prefix, suc->quiet);
> + url = xstrdup(sub->url);
> + }
> + if (!submodule_applicable_by_labels(suc->initialize, sub)
> + && !suc->warn_if_uninitialized)
> + goto cleanup;
> + }
> if (!url) {
> /*
>* Only mention uninitialized submodules when their
> @@ -745,6 +756,7 @@ static int update_clone(int argc, const char **argv,
> const char *prefix)
> const char *update = NULL;
> int max_jobs = -1;
> struct string_list_item *item;
> + const struct string_list *list;
> struct pathspec pathspec;
> struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
>
> @@ -793,6 +805,14 @@ static int update_clone(int argc, const char **argv,
> const char *prefix)
> gitmodules_config();
> git_config(submodule_config, NULL);
>
> + list = git_config_get_value_multi("submodule.actionOnLabel");
> + if (list) {
> + suc.initialize = xmalloc(sizeof(*suc.initialize));
> + string_list_init(suc.initialize, 1);
> + for_each_string_list_item(item, list)
> + string_list_insert(suc.initialize, item->string);
> + }
> +
> if (max_jobs < 0)
> max_jobs = parallel_submodules();
>
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index fc948fd..dc45551 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1032,4 +1032,119 @@ test_expect_success 'submodule add records multiple
> labels' '
> test_cmp expected actual
> '
>
> +cat < expected
> +submodule
> +-submodule2
> +EOF
> +
> +test_expect_success 'update initializes all modules when action-on-label
> configured' '
> + test_when_finished "rm -rf super super_clone" &&
> + mkdir super &&
> + pwd=$(pwd) &&
> + (
> + cd super &&
> + git init &&
> + git submodule add --label l
[RFC_PATCHv4 5/7] submodule update: respect submodule.actionOnLabel
This change introduces the 'submodule.actionOnLabel' variable
in a repository configuration. Generally speaking 'submodule.actionOnLabel'
restricts the action of a command when no submodules are selected via the
command line explicitely to those submodules, which are selected by
'submodule.actionOnLabel'. It can occur multiple times and can specify
the path, the name or one of the labels of a submodule to select that
submodule.
The introduction of 'submodule.actionOnLabel' starts with
'git submodule update' in this patch and other commands will follow
in later patches.
'submodule.actionOnLabel' implies '--init' in 'git submodule update'.
Signed-off-by: Stefan Beller
TODO: generic documentation for submodule.actionOnLabel
TODO: documentation for submodule update
Signed-off-by: Stefan Beller
---
builtin/submodule--helper.c | 22 -
t/t7400-submodule-basic.sh | 115
2 files changed, 136 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a69b1f4..93760ec 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -573,6 +573,8 @@ struct submodule_update_clone {
int current;
struct module_list list;
unsigned warn_if_uninitialized : 1;
+ /* patterns to initialize */
+ struct string_list *initialize;
/* update parameter passed via commandline */
struct submodule_update_strategy update;
@@ -590,7 +592,7 @@ struct submodule_update_clone {
/* If we want to stop as fast as possible and return an error */
unsigned quickstop : 1;
};
-#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
+#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, NULL, \
SUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \
STRING_LIST_INIT_DUP, 0}
@@ -644,6 +646,15 @@ static int prepare_to_clone_next_submodule(const struct
cache_entry *ce,
strbuf_reset(&sb);
strbuf_addf(&sb, "submodule.%s.url", sub->name);
git_config_get_string(sb.buf, &url);
+ if (suc->initialize) {
+ if (!url) {
+ init_submodule(sub->path, suc->prefix, suc->quiet);
+ url = xstrdup(sub->url);
+ }
+ if (!submodule_applicable_by_labels(suc->initialize, sub)
+ && !suc->warn_if_uninitialized)
+ goto cleanup;
+ }
if (!url) {
/*
* Only mention uninitialized submodules when their
@@ -745,6 +756,7 @@ static int update_clone(int argc, const char **argv, const
char *prefix)
const char *update = NULL;
int max_jobs = -1;
struct string_list_item *item;
+ const struct string_list *list;
struct pathspec pathspec;
struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
@@ -793,6 +805,14 @@ static int update_clone(int argc, const char **argv, const
char *prefix)
gitmodules_config();
git_config(submodule_config, NULL);
+ list = git_config_get_value_multi("submodule.actionOnLabel");
+ if (list) {
+ suc.initialize = xmalloc(sizeof(*suc.initialize));
+ string_list_init(suc.initialize, 1);
+ for_each_string_list_item(item, list)
+ string_list_insert(suc.initialize, item->string);
+ }
+
if (max_jobs < 0)
max_jobs = parallel_submodules();
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index fc948fd..dc45551 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1032,4 +1032,119 @@ test_expect_success 'submodule add records multiple
labels' '
test_cmp expected actual
'
+cat < expected
+submodule
+-submodule2
+EOF
+
+test_expect_success 'update initializes all modules when action-on-label
configured' '
+ test_when_finished "rm -rf super super_clone" &&
+ mkdir super &&
+ pwd=$(pwd) &&
+ (
+ cd super &&
+ git init &&
+ git submodule add --label labelA file://"$pwd"/example2
submodule &&
+ git submodule add file://"$pwd"/example2 submodule2 &&
+ git commit -a -m "add two modules, one is labled"
+ ) &&
+ git clone super super_clone &&
+ (
+ cd super_clone &&
+ git config submodule.actionOnLabel \*labelA &&
+ git submodule update &&
+ git submodule status |cut -c1,42-52 | tr -d " " >../actual
+ ) &&
+ test_cmp actual expected
+'
+
+test_expect_success 'submodule update applies to action-on-label selection' '
+ test_when_finished "rm -rf super super_clone" &&
+ mkdir super &&
+ oldSubmoduleHead=$(cd example2 && git rev-parse HEAD) &&
+ pwd=$(pwd) &&
+ (
+ cd super &&
+ git init &&
+
