From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 17:29:58
When running 'git clone --recurse-submodules', developers expect various
other commands such as 'pull' and 'checkout' to also run recursively into
submodules.The submitted code updates the 'submodule.recurse' config value
to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla mahikolla@google.com
Mahi Kolla (2):
clone: update submodule.recurse in config when using
--recurse-submodule
clone: update submodule.recurse in config when using
--recurse-submodule
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
2 files changed, 8 insertions(+)
base-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1006
--
gitgitgadget
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 17:30:01
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla <redacted>
---
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
2 files changed, 8 insertions(+)
@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 17:30:03
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla <redacted>
---
t/t5606-clone-options.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 22:38:41
When running 'git clone --recurse-submodules', developers expect various
other commands such as 'pull' and 'checkout' to also run recursively into
submodules.The submitted code updates the 'submodule.recurse' config value
to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla mahikolla@google.com
Mahi Kolla (3):
clone: update submodule.recurse in config when using
--recurse-submodule
clone: update submodule.recurse in config when using
--recurse-submodule
clone test: update whitespace according to style guide
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
2 files changed, 8 insertions(+)
base-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1006
Range-diff vs v1:
1: fea3d6d72b6 = 1: fea3d6d72b6 clone: update submodule.recurse in config when using --recurse-submodule
2: dd13a65ef0f = 2: dd13a65ef0f clone: update submodule.recurse in config when using --recurse-submodule
-: ----------- > 3: 020eaa2c819 clone test: update whitespace according to style guide
--
gitgitgadget
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 22:38:43
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla <redacted>
---
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
2 files changed, 8 insertions(+)
@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 22:38:43
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla <redacted>
---
t/t5606-clone-options.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 22:38:44
From: Mahi Kolla <redacted>
Previously, the code used spaces to appropriately format. The spaces have been replaced with tabs to follow style guide standards.
Signed-off-by: Mahi Kolla <redacted>
---
t/t5606-clone-options.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 23:23:15
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla <redacted>
---
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
2 files changed, 8 insertions(+)
@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 23:23:17
When running 'git clone --recurse-submodules', developers expect various
other commands such as 'pull' and 'checkout' to also run recursively into
submodules.The submitted code updates the 'submodule.recurse' config value
to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla mahikolla@google.com
Mahi Kolla (4):
clone: update submodule.recurse in config when using
--recurse-submodule
clone: update submodule.recurse in config when using
--recurse-submodule
clone test: update whitespace according to style guide
clone: update whitespace according to style guide
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
2 files changed, 8 insertions(+)
base-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1006
Range-diff vs v2:
1: fea3d6d72b6 = 1: fea3d6d72b6 clone: update submodule.recurse in config when using --recurse-submodule
2: dd13a65ef0f = 2: dd13a65ef0f clone: update submodule.recurse in config when using --recurse-submodule
3: 020eaa2c819 = 3: 020eaa2c819 clone test: update whitespace according to style guide
-: ----------- > 4: f3ddb344b49 clone: update whitespace according to style guide
--
gitgitgadget
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 23:23:17
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla <redacted>
---
t/t5606-clone-options.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 23:23:23
From: Mahi Kolla <redacted>
Previously, the code used spaces to appropriately format. The spaces have been replaced with tabs to follow style guide standards.
Signed-off-by: Mahi Kolla <redacted>
---
t/t5606-clone-options.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-02 23:23:24
From: Mahi Kolla <redacted>
Previously, the code used spaces to appropriately format. The spaces have been replaced with tabs to follow style guide standards.
Signed-off-by: Mahi Kolla <redacted>
---
builtin/clone.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1130,7 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}-string_list_append(&option_config,"submodule.recurse=true");+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Philippe Blain <hidden> Date: 2021-08-03 03:08:35
Hi Mahi,
Le 2021-08-02 à 19:23, Mahi Kolla via GitGitGadget a écrit :
When running 'git clone --recurse-submodules', developers expect various
other commands such as 'pull' and 'checkout' to also run recursively into
submodules.The submitted code updates the 'submodule.recurse' config value
to true when 'git clone' is run with the '--recurse-submodules' option.
Signed-off-by: Mahi Kolla mahikolla@google.com
Mahi Kolla (4):
clone: update submodule.recurse in config when using
--recurse-submodule
clone: update submodule.recurse in config when using
--recurse-submodule
clone test: update whitespace according to style guide
clone: update whitespace according to style guide
This could all be done in a single patch. As long as the patches are not merged
to the 'next' branch, you can simply force-push and re '/submit' on Gitgitgadget.
I think this change would require a mention in the 'git-clone(1)' man page.
I'm very much in favor of a change like this. It is a pretty big change in
behaviour, but I really believe this is a good way forward. Thanks for getting
the ball rolling on this!
From: Philippe Blain <hidden> Date: 2021-08-03 03:20:44
Le 2021-08-02 à 19:23, Mahi Kolla via GitGitGadget a écrit :
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.
missing space after period here. Also, maybe I would say "some developers"
or "developers might expect", or something like that.
The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
We usually use the imperative form for commit messages, like if giving an order to the code base
to improve itself. So you might word this simply as:
Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.
@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
I think that this change is a big enough behaviour change to warrant that
an "advice" message be shown to the user, informing them that 'submodule.recurse'
has been set to true, and maybe point users to 'gitsubmodules(7) to learn what this
configuration entails.
This should be 'test_cmp_config', which checks that a specific config variable has
the expected value. 'test_config' is used to set a config for the duration of the test,
which is not what you want to do here. I see you've changed it in a following
patch, but not to the right thing. As I wrote in my response to your cover letter,
this series should be as a single patch.
Thanks for working on this!
Philippe.
Thanks Philippe for the feedback! As a first time contributor, I
definitely struggled a bit with understanding how to submit patches
and how to amend commits versus stacking changes across commits. Thank
you for your patience! :)
On Mon, Aug 2, 2021 at 8:20 PM Philippe Blain
[off-list ref] wrote:
Le 2021-08-02 à 19:23, Mahi Kolla via GitGitGadget a écrit :
quoted
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.
missing space after period here. Also, maybe I would say "some developers"
or "developers might expect", or something like that.
That makes sense, will update.
The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
We usually use the imperative form for commit messages, like if giving an order to the code base
to improve itself. So you might word this simply as:
Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.
@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
I think that this change is a big enough behaviour change to warrant that
an "advice" message be shown to the user, informing them that 'submodule.recurse'
has been set to true, and maybe point users to 'gitsubmodules(7) to learn what this
configuration entails.
I will definitely update the 'git-clone' man page to include some
documentation on this change. Should the "advice" to the user show up
in the output message once the clone command is run?
This should be 'test_cmp_config', which checks that a specific config variable has
the expected value. 'test_config' is used to set a config for the duration of the test,
which is not what you want to do here. I see you've changed it in a following
patch, but not to the right thing. As I wrote in my response to your cover letter,
this series should be as a single patch.
Thanks for working on this!
Philippe.
Ah, thank you for pointing me to the right testing function.
Definitely, agree with the single patch. I'll update the corresponding
files and submit them all as one V2 patch.
Thank you so much!
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-09 19:11:50
From: Mahi Kolla <redacted>
When running 'git clone --recurse-submodules', developers might expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules. Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.
Since V1: Updated test and 'git clone' man page. Also updated commit message.
Signed-off-by: Mahi Kolla <redacted>
---
clone: update submodule.recurse in config when using --recurse-submodule
When running 'git clone --recurse-submodules', developers might expect
various other commands such as 'pull' and 'checkout' to also run
recursively into submodules. Set 'submodule.recurse' to true when 'git
clone' is run with '--recurse-submodules'.
Since V1: Updated test and 'git clone' man page. Also updated commit
message.
Signed-off-by: Mahi Kolla mahikolla@google.com
cc: Philippe Blain levraiphilippeblain@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1006
Range-diff vs v3:
1: fea3d6d72b6 ! 1: 73937d48a53 clone: update submodule.recurse in config when using --recurse-submodule
@@ Metadata
## Commit message ##
clone: update submodule.recurse in config when using --recurse-submodule
- When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.
+ When running 'git clone --recurse-submodules', developers might expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules. Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.
+
+ Since V1: Updated test and 'git clone' man page. Also updated commit message.
Signed-off-by: Mahi Kolla [off-list ref]
+ ## Documentation/git-clone.txt ##
+@@ Documentation/git-clone.txt: branch of some repository for search indexing.
+ This option can be given multiple times for pathspecs consisting
+ of multiple entries. The resulting clone has `submodule.active` set to
+ the provided pathspec, or "." (meaning all submodules) if no
+- pathspec is provided.
++ pathspec is provided. In addition, `submodule.recurse` is set to true.
+ +
+ Submodules are initialized and cloned using their default settings. This is
+ equivalent to running
+
## builtin/clone.c ##
@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
strbuf_detach(&sb, NULL));
}
-+ string_list_append(&option_config, "submodule.recurse=true");
++ string_list_append(&option_config, "submodule.recurse=true");
if (option_required_reference.nr &&
option_optional_reference.nr)
die(_("clone --recursive is not compatible with "
@@ t/t5606-clone-options.sh: test_expect_success 'setup' '
+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '
+
-+ git clone --recurse-submodules parent clone-rec-submodule &&
-+ test_config_global submodule.recurse true
++ git clone --recurse-submodules parent clone-rec-submodule &&
++ test_cmp_config -C clone-rec-submodule true submodule.recurse
+
+'
+
2: dd13a65ef0f < -: ----------- clone: update submodule.recurse in config when using --recurse-submodule
3: 020eaa2c819 < -: ----------- clone test: update whitespace according to style guide
4: f3ddb344b49 < -: ----------- clone: update whitespace according to style guide
Documentation/git-clone.txt | 2 +-
builtin/clone.c | 1 +
t/t5606-clone-options.sh | 7 +++++++
3 files changed, 9 insertions(+), 1 deletion(-)
@@ -276,7 +276,7 @@ branch of some repository for search indexing. This option can be given multiple times for pathspecs consisting of multiple entries. The resulting clone has `submodule.active` set to the provided pathspec, or "." (meaning all submodules) if no- pathspec is provided.+ pathspec is provided. In addition, `submodule.recurse` is set to true. + Submodules are initialized and cloned using their default settings. This is equivalent to running
@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+string_list_append(&option_config,"submodule.recurse=true");if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-12 02:46:12
From: Mahi Kolla <redacted>
Currently, when running 'git clone --recurse-submodules', developers do not expect other commands such as 'pull' or 'checkout' to run recursively into active submodules. However, setting 'submodule.recurse' to true at this step could make for a simpler workflow by eliminating the '--recurse-submodules' option in subsequent commands. To collect more data on developers' preference in regards to making 'submodule.recurse=true' a default config value in the future, deploy this feature under the opt in feature.experimental flag.
Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.
Signed-off-by: Mahi Kolla <redacted>
---
clone: set submodule.recurse=true if feature.experimental flag enabled
Based on current experience, when running git clone
--recurse-submodules, developers do not expect other commands such as
pull or checkout to run recursively into active submodules. However,
setting submodule.recurse=true at this step could make for a simpler
workflow by eliminating the need for the --recurse-submodules option in
subsequent commands. To collect more data on developers' preference in
regards to making submodule.recurse=true a default config value in the
future, deploy this feature under the opt in feature.experimental flag.
Since V1: Made this an opt in feature under the experimental flag.
Updated tests to reflect this design change. Also updated commit
message.
Signed-off-by: Mahi Kolla mahikolla@google.com
cc: Philippe Blain levraiphilippeblain@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1006
Range-diff vs v4:
1: 73937d48a53 ! 1: 2c6ffe00736 clone: update submodule.recurse in config when using --recurse-submodule
@@ Metadata
Author: Mahi Kolla [off-list ref]
## Commit message ##
- clone: update submodule.recurse in config when using --recurse-submodule
+ clone: set submodule.recurse=true if user enables feature.experimental flag
- When running 'git clone --recurse-submodules', developers might expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules. Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.
+ Currently, when running 'git clone --recurse-submodules', developers do not expect other commands such as 'pull' or 'checkout' to run recursively into active submodules. However, setting 'submodule.recurse' to true at this step could make for a simpler workflow by eliminating the '--recurse-submodules' option in subsequent commands. To collect more data on developers' preference in regards to making 'submodule.recurse=true' a default config value in the future, deploy this feature under the opt in feature.experimental flag.
- Since V1: Updated test and 'git clone' man page. Also updated commit message.
+ Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.
Signed-off-by: Mahi Kolla [off-list ref]
- ## Documentation/git-clone.txt ##
-@@ Documentation/git-clone.txt: branch of some repository for search indexing.
- This option can be given multiple times for pathspecs consisting
- of multiple entries. The resulting clone has `submodule.active` set to
- the provided pathspec, or "." (meaning all submodules) if no
-- pathspec is provided.
-+ pathspec is provided. In addition, `submodule.recurse` is set to true.
- +
- Submodules are initialized and cloned using their default settings. This is
- equivalent to running
-
## builtin/clone.c ##
+@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
+ struct remote *remote;
+ int err = 0, complete_refs_before_fetch = 1;
+ int submodule_progress;
++ int experimental_flag;
+
+ struct transport_ls_refs_options transport_ls_refs_options =
+ TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
strbuf_detach(&sb, NULL));
}
-+ string_list_append(&option_config, "submodule.recurse=true");
++ if(!git_config_get_bool("feature.experimental", &experimental_flag) &&
++ experimental_flag) {
++ string_list_append(&option_config, "submodule.recurse=true");
++ }
++
if (option_required_reference.nr &&
option_optional_reference.nr)
die(_("clone --recursive is not compatible with "
@@ t/t5606-clone-options.sh: test_expect_success 'setup' '
'
-+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '
++test_expect_success 'feature.experimental flag manipulates submodule.recurse value' '
++
++ test_config_global feature.experimental true &&
++ git clone --recurse-submodules parent clone_recurse_true &&
++ test_cmp_config -C clone_recurse_true true submodule.recurse &&
+
-+ git clone --recurse-submodules parent clone-rec-submodule &&
-+ test_cmp_config -C clone-rec-submodule true submodule.recurse
++ test_config_global feature.experimental false &&
++ git clone --recurse-submodules parent clone_recurse_false &&
++ test_expect_code 1 git -C clone_recurse_false config --get submodule.recurse
+
+'
+
builtin/clone.c | 6 ++++++
t/t5606-clone-options.sh | 12 ++++++++++++
2 files changed, 18 insertions(+)
@@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+if(!git_config_get_bool("feature.experimental",&experimental_flag)&&+experimental_flag){+string_list_append(&option_config,"submodule.recurse=true");+}+if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Mahi Kolla via GitGitGadget <hidden> Date: 2021-08-14 01:10:02
From: Mahi Kolla <redacted>
Based on current experience, when running git clone --recurse-submodules,
developers do not expect other commands such as pull or checkout to run
recursively into active submodules. However, setting submodule.recurse=true
at this step could make for a simpler workflow by eliminating the need for
the --recurse-submodules option in subsequent commands. To collect more
data on developers' preference in regards to making submodule.recurse=true
a default config value in the future, deploy this feature under the opt in
submodule.stickyRecursiveClone flag.
Signed-off-by: Mahi Kolla <redacted>
---
clone: set submodule.recurse=true if submodule.stickyRecursiveClone
enabled
Based on current experience, when running git clone
--recurse-submodules, developers do not expect other commands such as
pull or checkout to run recursively into active submodules. However,
setting submodule.recurse=true at this step could make for a simpler
workflow by eliminating the need for the --recurse-submodules option in
subsequent commands. To collect more data on developers' behavior and
preferences when making submodule.recurse=true a default, deploy this
feature under the opt in submodule.stickyRecursiveClone flag.
Signed-off-by: Mahi Kolla mkolla2@illinois.edu
Since V1: Made this an opt in feature under a custom
submodule.stickyRecursiveClone flag as opposed to feature.experimental.
Updated tests to reflect this design change. Also updated commit
message. Additionally, I will be contributing from my personal email
going forward as opposed to my @google email.
cc: Philippe Blain levraiphilippeblain@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1006
Range-diff vs v5:
1: 2c6ffe00736 ! 1: e668fa403cf clone: set submodule.recurse=true if user enables feature.experimental flag
@@
## Metadata ##
-Author: Mahi Kolla [off-list ref]
+Author: Mahi Kolla [off-list ref]
## Commit message ##
- clone: set submodule.recurse=true if user enables feature.experimental flag
+ clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled
- Currently, when running 'git clone --recurse-submodules', developers do not expect other commands such as 'pull' or 'checkout' to run recursively into active submodules. However, setting 'submodule.recurse' to true at this step could make for a simpler workflow by eliminating the '--recurse-submodules' option in subsequent commands. To collect more data on developers' preference in regards to making 'submodule.recurse=true' a default config value in the future, deploy this feature under the opt in feature.experimental flag.
+ Based on current experience, when running git clone --recurse-submodules,
+ developers do not expect other commands such as pull or checkout to run
+ recursively into active submodules. However, setting submodule.recurse=true
+ at this step could make for a simpler workflow by eliminating the need for
+ the --recurse-submodules option in subsequent commands. To collect more
+ data on developers' preference in regards to making submodule.recurse=true
+ a default config value in the future, deploy this feature under the opt in
+ submodule.stickyRecursiveClone flag.
- Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.
-
- Signed-off-by: Mahi Kolla [off-list ref]
+ Signed-off-by: Mahi Kolla [off-list ref]
## builtin/clone.c ##
@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
struct remote *remote;
int err = 0, complete_refs_before_fetch = 1;
int submodule_progress;
-+ int experimental_flag;
++ int sticky_recursive_clone;
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
strbuf_detach(&sb, NULL));
}
-+ if(!git_config_get_bool("feature.experimental", &experimental_flag) &&
-+ experimental_flag) {
++ if (!git_config_get_bool("submodule.stickyRecursiveClone", &sticky_recursive_clone)
++ && sticky_recursive_clone) {
+ string_list_append(&option_config, "submodule.recurse=true");
+ }
+
@@ t/t5606-clone-options.sh: test_expect_success 'setup' '
'
-+test_expect_success 'feature.experimental flag manipulates submodule.recurse value' '
++test_expect_success 'submodule.stickyRecursiveClone flag manipulates submodule.recurse value' '
+
-+ test_config_global feature.experimental true &&
++ test_config_global submodule.stickyRecursiveClone true &&
+ git clone --recurse-submodules parent clone_recurse_true &&
+ test_cmp_config -C clone_recurse_true true submodule.recurse &&
+
-+ test_config_global feature.experimental false &&
++ test_config_global submodule.stickyRecursiveClone false &&
+ git clone --recurse-submodules parent clone_recurse_false &&
+ test_expect_code 1 git -C clone_recurse_false config --get submodule.recurse
+
builtin/clone.c | 6 ++++++
t/t5606-clone-options.sh | 12 ++++++++++++
2 files changed, 18 insertions(+)
@@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)strbuf_detach(&sb,NULL));}+if(!git_config_get_bool("submodule.stickyRecursiveClone",&sticky_recursive_clone)+&&sticky_recursive_clone){+string_list_append(&option_config,"submodule.recurse=true");+}+if(option_required_reference.nr&&option_optional_reference.nr)die(_("clone --recursive is not compatible with "
From: Philippe Blain <hidden> Date: 2021-08-18 22:41:04
Hi Mahi, Emily and Junio,
Le 2021-08-13 à 21:09, Mahi Kolla via GitGitGadget a écrit :
From: Mahi Kolla <redacted>
Based on current experience, when running git clone --recurse-submodules,
developers do not expect other commands such as pull or checkout to run
recursively into active submodules. However, setting submodule.recurse=true
at this step could make for a simpler workflow by eliminating the need for
the --recurse-submodules option in subsequent commands. To collect more
data on developers' preference in regards to making submodule.recurse=true
a default config value in the future, deploy this feature under the opt in
submodule.stickyRecursiveClone flag.
As I mentioned upthread [1], I'm really not sure that we need a new config
variable. If people want to have "--recurse-submodules" the default behaviour
for all commands that know this flag, they can set 'submodule.recurse' in their
~/.gitconfig, which enables the behaviour for all those commands (except clone
and ls-files). And orgs shipping Git to their devs can set it in their system
gitconfig. I've been using this setup for over two years, with almost zero
adverse effect on repositories that do not contain submodules. The *only* bug that
I encountered that affects git commands when *no* submodules are at play was:
c56c48dd07 (grep: ignore --recurse-submodules if --no-index is given, 2020-01-30)
I understand that once submodule.recurse is set in the global or system config file,
then Git will start recursing in repos that were cloned prior to
that config being enabled, as Emily mentions in [2]. Personnally I think it's a positive
point. That *might* be a deal-breaker for other people,
but in any case it would be good that this alternative - just using 'submodule.recurse'
- is mentioned in the commit message and that it mentions that caveat, i.e. why we need
a separate config.
Le 2021-08-13 à 16:38, Emily Shaffer a écrit :
and apparently
'submodule.recurse=true' has some weird edge cases for commands which
are happy to run out-of-repo.
It would be nice if those known edge cases were documented somewhere (on the list,
on Gitgitgadget's issues list or at https://bugs.chromium.org/p/git). Apart from
the 'grep --no-index' glitch that I mentioned above, I did not encounter any
myself.
Signed-off-by: Mahi Kolla <redacted>
---
clone: set submodule.recurse=true if submodule.stickyRecursiveClone
enabled
Based on current experience, when running git clone
--recurse-submodules, developers do not expect other commands such as
pull or checkout to run recursively into active submodules. However,
setting submodule.recurse=true at this step could make for a simpler
workflow by eliminating the need for the --recurse-submodules option in
subsequent commands. To collect more data on developers' behavior and
preferences when making submodule.recurse=true a default, deploy this
feature under the opt in submodule.stickyRecursiveClone flag.
Signed-off-by: Mahi Kolla mkolla2@illinois.edu
Mahi, you can keep just the "Since v1" part in the GGG PR description (and the
automatically added CC's). No need to also repeat the commit message.
Since V1: Made this an opt in feature under a custom
submodule.stickyRecursiveClone flag as opposed to feature.experimental.
Updated tests to reflect this design change. Also updated commit
message. Additionally, I will be contributing from my personal email
going forward as opposed to my @google email.
cc: Philippe Blain levraiphilippeblain@gmail.com
Small nit: since you used '/submit' several times on Gitgitgadget, the previous version
you sent was actually sent as v5, and this here version is v6. So for next round, you could
write "Changes since v6" :)
OK, so the expectation with 'submodule.stickyRecursiveClone' is that :
- if it's true, then 'submodule.recurse' is set to true in the clone's local config file.
That makes sense.
- if it's false, then 'submodule.recurse' is not set in the clone. This means that
if 'submodule.recurse' is already set in ~/.gitconfig (or the system config file)
then the clone will respect the configured value. That also makes sense, I think.