{"thread":{"id":"56199","subject":"[PATCH 0/2] clone: update submodule.recurse in config when using --recurse-submodule","startedAt":"2021-08-02T17:29:58Z","lastAt":"2021-08-30T21:23:26Z","messageCount":46,"participants":["Mahi Kolla via GitGitGadget","Philippe Blain","Junio C Hamano","Mahi Kolla","Emily Shaffer","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"431733","messageId":"pull.1006.git.1627925390.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":null,"subject":"[PATCH 0/2] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T17:29:48Z","receivedAt":"2021-08-02T17:29:58Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"When running 'git clone --recurse-submodules', developers expect various\nother commands such as 'pull' and 'checkout' to also run recursively into\nsubmodules.The submitted code updates the 'submodule.recurse' config value\nto true when 'git clone' is run with the '--recurse-submodules' option.\n\nSigned-off-by: Mahi Kolla mahikolla@google.com\n\nMahi Kolla (2):\n clone: update submodule.recurse in config when using\n --recurse-submodule\n clone: update submodule.recurse in config when using\n --recurse-submodule\n\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\n\nbase-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1006\n-- \ngitgitgadget\n"},{"id":"431734","messageId":"fea3d6d72b63c06138d8eeb61e45edb30abbd79d.1627925390.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.git.1627925390.gitgitgadget@gmail.com","subject":"[PATCH 1/2] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T17:29:49Z","receivedAt":"2021-08-02T17:30:01Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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.\n\nSigned-off-by: Mahi Kolla \n---\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 66fe66679c8..f41fd1afb66 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+ string_list_append(&option_config, \"submodule.recurse=true\");\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3a595c0f82c..3daef8c941f 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -16,6 +16,13 @@ test_expect_success 'setup' '\n \n '\n \n+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n+\n+ git clone --recurse-submodules parent clone-rec-submodule &&\n+ test_config_global submodule.recurse true \n+\n+'\n+\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n-- \ngitgitgadget\n\n"},{"id":"431735","messageId":"dd13a65ef0f648f13cc220303092a657148c19ac.1627925390.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.git.1627925390.gitgitgadget@gmail.com","subject":"[PATCH 2/2] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T17:29:50Z","receivedAt":"2021-08-02T17:30:03Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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.\n\nSigned-off-by: Mahi Kolla \n---\n t/t5606-clone-options.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3daef8c941f..69c4bacf52f 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -19,7 +19,7 @@ test_expect_success 'setup' '\n test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n \n git clone --recurse-submodules parent clone-rec-submodule &&\n- test_config_global submodule.recurse true \n+ git config submodule.recurse true\n \n '\n \n-- \ngitgitgadget\n"},{"id":"431774","messageId":"pull.1006.v2.git.1627943914.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.git.1627925390.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T22:38:31Z","receivedAt":"2021-08-02T22:38:41Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"When running 'git clone --recurse-submodules', developers expect various\nother commands such as 'pull' and 'checkout' to also run recursively into\nsubmodules.The submitted code updates the 'submodule.recurse' config value\nto true when 'git clone' is run with the '--recurse-submodules' option.\n\nSigned-off-by: Mahi Kolla mahikolla@google.com\n\nMahi Kolla (3):\n clone: update submodule.recurse in config when using\n --recurse-submodule\n clone: update submodule.recurse in config when using\n --recurse-submodule\n clone test: update whitespace according to style guide\n\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\n\nbase-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1006\n\nRange-diff vs v1:\n\n 1: fea3d6d72b6 = 1: fea3d6d72b6 clone: update submodule.recurse in config when using --recurse-submodule\n 2: dd13a65ef0f = 2: dd13a65ef0f clone: update submodule.recurse in config when using --recurse-submodule\n -: ----------- > 3: 020eaa2c819 clone test: update whitespace according to style guide\n\n-- \ngitgitgadget\n"},{"id":"431775","messageId":"fea3d6d72b63c06138d8eeb61e45edb30abbd79d.1627943914.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v2.git.1627943914.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T22:38:32Z","receivedAt":"2021-08-02T22:38:43Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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.\n\nSigned-off-by: Mahi Kolla \n---\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 66fe66679c8..f41fd1afb66 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+ string_list_append(&option_config, \"submodule.recurse=true\");\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3a595c0f82c..3daef8c941f 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -16,6 +16,13 @@ test_expect_success 'setup' '\n \n '\n \n+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n+\n+ git clone --recurse-submodules parent clone-rec-submodule &&\n+ test_config_global submodule.recurse true \n+\n+'\n+\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n-- \ngitgitgadget\n\n"},{"id":"431776","messageId":"dd13a65ef0f648f13cc220303092a657148c19ac.1627943914.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v2.git.1627943914.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T22:38:33Z","receivedAt":"2021-08-02T22:38:43Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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.\n\nSigned-off-by: Mahi Kolla \n---\n t/t5606-clone-options.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3daef8c941f..69c4bacf52f 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -19,7 +19,7 @@ test_expect_success 'setup' '\n test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n \n git clone --recurse-submodules parent clone-rec-submodule &&\n- test_config_global submodule.recurse true \n+ git config submodule.recurse true\n \n '\n \n-- \ngitgitgadget\n\n"},{"id":"431777","messageId":"020eaa2c819779fbb21960c42981303f858e2674.1627943914.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v2.git.1627943914.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] clone test: update whitespace according to style guide","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T22:38:34Z","receivedAt":"2021-08-02T22:38:44Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nPreviously, the code used spaces to appropriately format. The spaces have been replaced with tabs to follow style guide standards.\n\nSigned-off-by: Mahi Kolla \n---\n t/t5606-clone-options.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 69c4bacf52f..1a3f1e9ab18 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -18,8 +18,8 @@ test_expect_success 'setup' '\n \n test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n \n- git clone --recurse-submodules parent clone-rec-submodule &&\n- git config submodule.recurse true\n+\tgit clone --recurse-submodules parent clone-rec-submodule &&\n+\tgit config submodule.recurse true\n \n '\n \n-- \ngitgitgadget\n"},{"id":"431779","messageId":"fea3d6d72b63c06138d8eeb61e45edb30abbd79d.1627946590.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"[PATCH v3 1/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T23:23:07Z","receivedAt":"2021-08-02T23:23:15Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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.\n\nSigned-off-by: Mahi Kolla \n---\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 66fe66679c8..f41fd1afb66 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+ string_list_append(&option_config, \"submodule.recurse=true\");\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3a595c0f82c..3daef8c941f 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -16,6 +16,13 @@ test_expect_success 'setup' '\n \n '\n \n+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n+\n+ git clone --recurse-submodules parent clone-rec-submodule &&\n+ test_config_global submodule.recurse true \n+\n+'\n+\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n-- \ngitgitgadget\n\n"},{"id":"431780","messageId":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v2.git.1627943914.gitgitgadget@gmail.com","subject":"[PATCH v3 0/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T23:23:06Z","receivedAt":"2021-08-02T23:23:17Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"When running 'git clone --recurse-submodules', developers expect various\nother commands such as 'pull' and 'checkout' to also run recursively into\nsubmodules.The submitted code updates the 'submodule.recurse' config value\nto true when 'git clone' is run with the '--recurse-submodules' option.\n\nSigned-off-by: Mahi Kolla mahikolla@google.com\n\nMahi Kolla (4):\n clone: update submodule.recurse in config when using\n --recurse-submodule\n clone: update submodule.recurse in config when using\n --recurse-submodule\n clone test: update whitespace according to style guide\n clone: update whitespace according to style guide\n\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\n\nbase-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1006\n\nRange-diff vs v2:\n\n 1: fea3d6d72b6 = 1: fea3d6d72b6 clone: update submodule.recurse in config when using --recurse-submodule\n 2: dd13a65ef0f = 2: dd13a65ef0f clone: update submodule.recurse in config when using --recurse-submodule\n 3: 020eaa2c819 = 3: 020eaa2c819 clone test: update whitespace according to style guide\n -: ----------- > 4: f3ddb344b49 clone: update whitespace according to style guide\n\n-- \ngitgitgadget\n"},{"id":"431781","messageId":"dd13a65ef0f648f13cc220303092a657148c19ac.1627946590.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"[PATCH v3 2/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T23:23:08Z","receivedAt":"2021-08-02T23:23:17Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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.\n\nSigned-off-by: Mahi Kolla \n---\n t/t5606-clone-options.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3daef8c941f..69c4bacf52f 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -19,7 +19,7 @@ test_expect_success 'setup' '\n test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n \n git clone --recurse-submodules parent clone-rec-submodule &&\n- test_config_global submodule.recurse true \n+ git config submodule.recurse true\n \n '\n \n-- \ngitgitgadget\n\n"},{"id":"431782","messageId":"020eaa2c819779fbb21960c42981303f858e2674.1627946590.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"[PATCH v3 3/4] clone test: update whitespace according to style guide","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T23:23:09Z","receivedAt":"2021-08-02T23:23:23Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nPreviously, the code used spaces to appropriately format. The spaces have been replaced with tabs to follow style guide standards.\n\nSigned-off-by: Mahi Kolla \n---\n t/t5606-clone-options.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 69c4bacf52f..1a3f1e9ab18 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -18,8 +18,8 @@ test_expect_success 'setup' '\n \n test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n \n- git clone --recurse-submodules parent clone-rec-submodule &&\n- git config submodule.recurse true\n+\tgit clone --recurse-submodules parent clone-rec-submodule &&\n+\tgit config submodule.recurse true\n \n '\n \n-- \ngitgitgadget\n\n"},{"id":"431783","messageId":"f3ddb344b49b00d62574135f857b472064219583.1627946590.git.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"[PATCH v3 4/4] clone: update whitespace according to style guide","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T23:23:10Z","receivedAt":"2021-08-02T23:23:24Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nPreviously, the code used spaces to appropriately format. The spaces have been replaced with tabs to follow style guide standards.\n\nSigned-off-by: Mahi Kolla \n---\n builtin/clone.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f41fd1afb66..c6bb38d2fde 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,7 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n- string_list_append(&option_config, \"submodule.recurse=true\");\n+\t\tstring_list_append(&option_config, \"submodule.recurse=true\");\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\n-- \ngitgitgadget\n"},{"id":"431797","messageId":"5e42756e-09bb-d51e-71c1-3aab93c2d27a@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-08-03T03:08:28Z","receivedAt":"2021-08-03T03:08:35Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Mahi,\n\nLe 2021-08-02 à 19:23, Mahi Kolla via GitGitGadget a écrit :\n> When running 'git clone --recurse-submodules', developers expect various\n> other commands such as 'pull' and 'checkout' to also run recursively into\n> submodules.The submitted code updates the 'submodule.recurse' config value\n> to true when 'git clone' is run with the '--recurse-submodules' option.\n> \n> Signed-off-by: Mahi Kolla mahikolla@google.com\n> \n> Mahi Kolla (4):\n> clone: update submodule.recurse in config when using\n> --recurse-submodule\n> clone: update submodule.recurse in config when using\n> --recurse-submodule\n> clone test: update whitespace according to style guide\n> clone: update whitespace according to style guide\n\n\nThis could all be done in a single patch. As long as the patches are not merged\nto the 'next' branch, you can simply force-push and re '/submit' on Gitgitgadget.\n\n> \n> builtin/clone.c | 1 +\n> t/t5606-clone-options.sh | 7 +++++++\n> 2 files changed, 8 insertions(+)\n\nI think this change would require a mention in the 'git-clone(1)' man page.\n\nI'm very much in favor of a change like this. It is a pretty big change in\nbehaviour, but I really believe this is a good way forward. Thanks for getting\nthe ball rolling on this!\n"},{"id":"431798","messageId":"e1bec4f1-d68f-2342-f759-8d0dc0de5eb4@gmail.com","threadId":"56199","inReplyTo":"fea3d6d72b63c06138d8eeb61e45edb30abbd79d.1627946590.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-08-03T03:20:39Z","receivedAt":"2021-08-03T03:20:44Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n\nLe 2021-08-02 à 19:23, Mahi Kolla via GitGitGadget a écrit :\n> From: Mahi Kolla \n> \n> When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.\n\nmissing space after period here. Also, maybe I would say \"some developers\"\nor \"developers might expect\", or something like that.\n\nThe submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.\n\nWe usually use the imperative form for commit messages, like if giving an order to the code base\nto improve itself. So you might word this simply as:\n\nSet 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.\n\n> \n> Signed-off-by: Mahi Kolla \n> ---\n> builtin/clone.c | 1 +\n> t/t5606-clone-options.sh | 7 +++++++\n> 2 files changed, 8 insertions(+)\n> \n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 66fe66679c8..f41fd1afb66 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t strbuf_detach(&sb, NULL));\n> \t\t}\n> \n> + string_list_append(&option_config, \"submodule.recurse=true\");\n> \t\tif (option_required_reference.nr &&\n> \t\t option_optional_reference.nr)\n> \t\t\tdie(_(\"clone --recursive is not compatible with \"\n\nI think that this change is a big enough behaviour change to warrant that\nan \"advice\" message be shown to the user, informing them that 'submodule.recurse'\nhas been set to true, and maybe point users to 'gitsubmodules(7) to learn what this\nconfiguration entails.\n\n> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\n> index 3a595c0f82c..3daef8c941f 100755\n> --- a/t/t5606-clone-options.sh\n> +++ b/t/t5606-clone-options.sh\n> @@ -16,6 +16,13 @@ test_expect_success 'setup' '\n> \n> '\n> \n> +test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n> +\n> + git clone --recurse-submodules parent clone-rec-submodule &&\n> + test_config_global submodule.recurse true\n\nThis should be 'test_cmp_config', which checks that a specific config variable has\nthe expected value. 'test_config' is used to set a config for the duration of the test,\nwhich is not what you want to do here. I see you've changed it in a following\npatch, but not to the right thing. As I wrote in my response to your cover letter,\nthis series should be as a single patch.\n\nThanks for working on this!\n\nPhilippe.\n"},{"id":"431892","messageId":"xmqq5ywmtalh.fsf@gitster.g","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-03T22:41:30Z","receivedAt":"2021-08-03T22:41:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mahi Kolla via GitGitGadget\" writes:\n\n> When running 'git clone --recurse-submodules', developers expect various\n> other commands such as 'pull' and 'checkout' to also run recursively into\n> submodules.\n\nSome developers might, but \"developers expect\" as if we speak for\neverybody is a bold statement to make that needs to be\nsubstantiated, I would think. Is this something easy to make\nopt-in, e.g. \"git clone --recurse-submodules=sticky\" or something?\n"},{"id":"432225","messageId":"CAN3QUFabO1PsW8te5zqwNRrHc99q4jO=6-pPc8cEv43Zevux4A@mail.gmail.com","threadId":"56199","inReplyTo":"e1bec4f1-d68f-2342-f759-8d0dc0de5eb4@gmail.com","subject":"Re: [PATCH v3 1/4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla","fromEmail":"mahikolla@google.com","sentAt":"2021-08-07T03:06:49Z","receivedAt":"2021-08-07T03:07:06Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"Thanks Philippe for the feedback! As a first time contributor, I\ndefinitely struggled a bit with understanding how to submit patches\nand how to amend commits versus stacking changes across commits. Thank\nyou for your patience! :)\n\nOn Mon, Aug 2, 2021 at 8:20 PM Philippe Blain\n wrote:\n>\n>\n>\n> Le 2021-08-02 à 19:23, Mahi Kolla via GitGitGadget a écrit :\n> > From: Mahi Kolla \n> >\n> > When running 'git clone --recurse-submodules', developers expect various other commands such as 'pull' and 'checkout' to also run recursively into submodules.\n>\n> missing space after period here. Also, maybe I would say \"some developers\"\n> or \"developers might expect\", or something like that.\n>\n\nThat makes sense, will update.\n\n> The submitted code updates the 'submodule.recurse' config value to true when 'git clone' is run with the '--recurse-submodules' option.\n>\n> We usually use the imperative form for commit messages, like if giving an order to the code base\n> to improve itself. So you might word this simply as:\n>\n> Set 'submodule.recurse' to true when 'git clone' is run with '--recurse-submodules'.\n>\n\nGot it, will update.\n\n> >\n> > Signed-off-by: Mahi Kolla \n> > ---\n> > builtin/clone.c | 1 +\n> > t/t5606-clone-options.sh | 7 +++++++\n> > 2 files changed, 8 insertions(+)\n> >\n> > diff --git a/builtin/clone.c b/builtin/clone.c\n> > index 66fe66679c8..f41fd1afb66 100644\n> > --- a/builtin/clone.c\n> > +++ b/builtin/clone.c\n> > @@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> > strbuf_detach(&sb, NULL));\n> > }\n> >\n> > + string_list_append(&option_config, \"submodule.recurse=true\");\n> > if (option_required_reference.nr &&\n> > option_optional_reference.nr)\n> > die(_(\"clone --recursive is not compatible with \"\n>\n> I think that this change is a big enough behaviour change to warrant that\n> an \"advice\" message be shown to the user, informing them that 'submodule.recurse'\n> has been set to true, and maybe point users to 'gitsubmodules(7) to learn what this\n> configuration entails.\n>\n\nI will definitely update the 'git-clone' man page to include some\ndocumentation on this change. Should the \"advice\" to the user show up\nin the output message once the clone command is run?\n\n> > diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\n> > index 3a595c0f82c..3daef8c941f 100755\n> > --- a/t/t5606-clone-options.sh\n> > +++ b/t/t5606-clone-options.sh\n> > @@ -16,6 +16,13 @@ test_expect_success 'setup' '\n> >\n> > '\n> >\n> > +test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n> > +\n> > + git clone --recurse-submodules parent clone-rec-submodule &&\n> > + test_config_global submodule.recurse true\n>\n> This should be 'test_cmp_config', which checks that a specific config variable has\n> the expected value. 'test_config' is used to set a config for the duration of the test,\n> which is not what you want to do here. I see you've changed it in a following\n> patch, but not to the right thing. As I wrote in my response to your cover letter,\n> this series should be as a single patch.\n>\n> Thanks for working on this!\n>\n> Philippe.\n\nAh, thank you for pointing me to the right testing function.\nDefinitely, agree with the single patch. I'll update the corresponding\nfiles and submit them all as one V2 patch.\n\nThank you so much!\n"},{"id":"432327","messageId":"pull.1006.v4.git.1628536305810.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v3.git.1627946590.gitgitgadget@gmail.com","subject":"[PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-09T19:11:45Z","receivedAt":"2021-08-09T19:11:50Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nWhen 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'.\n\nSince V1: Updated test and 'git clone' man page. Also updated commit message.\n\nSigned-off-by: Mahi Kolla \n---\n clone: update submodule.recurse in config when using --recurse-submodule\n \n When running 'git clone --recurse-submodules', developers might expect\n various other commands such as 'pull' and 'checkout' to also run\n recursively into submodules. Set 'submodule.recurse' to true when 'git\n clone' is run with '--recurse-submodules'.\n \n Since V1: Updated test and 'git clone' man page. Also updated commit\n message.\n \n Signed-off-by: Mahi Kolla mahikolla@google.com\n \n cc: Philippe Blain levraiphilippeblain@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1006\n\nRange-diff vs v3:\n\n 1: fea3d6d72b6 ! 1: 73937d48a53 clone: update submodule.recurse in config when using --recurse-submodule\n @@ Metadata\n ## Commit message ##\n clone: update submodule.recurse in config when using --recurse-submodule\n \n - 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.\n + 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'.\n +\n + Since V1: Updated test and 'git clone' man page. Also updated commit message.\n \n Signed-off-by: Mahi Kolla \n \n + ## Documentation/git-clone.txt ##\n +@@ Documentation/git-clone.txt: branch of some repository for search indexing.\n + \tThis option can be given multiple times for pathspecs consisting\n + \tof multiple entries. The resulting clone has `submodule.active` set to\n + \tthe provided pathspec, or \".\" (meaning all submodules) if no\n +-\tpathspec is provided.\n ++\tpathspec is provided. In addition, `submodule.recurse` is set to true.\n + +\n + Submodules are initialized and cloned using their default settings. This is\n + equivalent to running\n +\n ## builtin/clone.c ##\n @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n -+ string_list_append(&option_config, \"submodule.recurse=true\");\n ++\t\tstring_list_append(&option_config, \"submodule.recurse=true\");\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\n @@ t/t5606-clone-options.sh: test_expect_success 'setup' '\n \n +test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n +\n -+ git clone --recurse-submodules parent clone-rec-submodule &&\n -+ test_config_global submodule.recurse true \n ++\tgit clone --recurse-submodules parent clone-rec-submodule &&\n ++\ttest_cmp_config -C clone-rec-submodule true submodule.recurse\n +\n +'\n +\n 2: dd13a65ef0f < -: ----------- clone: update submodule.recurse in config when using --recurse-submodule\n 3: 020eaa2c819 < -: ----------- clone test: update whitespace according to style guide\n 4: f3ddb344b49 < -: ----------- clone: update whitespace according to style guide\n\n\n Documentation/git-clone.txt | 2 +-\n builtin/clone.c | 1 +\n t/t5606-clone-options.sh | 7 +++++++\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 3fe3810f1ce..1d6aeb9e367 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -276,7 +276,7 @@ branch of some repository for search indexing.\n \tThis option can be given multiple times for pathspecs consisting\n \tof multiple entries. The resulting clone has `submodule.active` set to\n \tthe provided pathspec, or \".\" (meaning all submodules) if no\n-\tpathspec is provided.\n+\tpathspec is provided. In addition, `submodule.recurse` is set to true.\n +\n Submodules are initialized and cloned using their default settings. This is\n equivalent to running\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 66fe66679c8..c6bb38d2fde 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,6 +1130,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+\t\tstring_list_append(&option_config, \"submodule.recurse=true\");\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3a595c0f82c..055b74069b3 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -16,6 +16,13 @@ test_expect_success 'setup' '\n \n '\n \n+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n+\n+\tgit clone --recurse-submodules parent clone-rec-submodule &&\n+\ttest_cmp_config -C clone-rec-submodule true submodule.recurse\n+\n+'\n+\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n\nbase-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\n-- \ngitgitgadget\n"},{"id":"432336","messageId":"xmqqzgtqe2w6.fsf@gitster.g","threadId":"56199","inReplyTo":"pull.1006.v4.git.1628536305810.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-09T21:15:05Z","receivedAt":"2021-08-09T21:15:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mahi Kolla via GitGitGadget\" writes:\n\n> From: Mahi Kolla \n>\n> 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'.\n\nPlease wrap overlong lines in your proposed log message to say 70 or\nso columns.\n\nSome developers might expect, but wouldn't some others want to see\nthis not set to true, but want to recurse only into some but not all\nsubmodules?\n\nIs it possible to avoid changing the behaviour unconditionally and\npotentially breaking existing users by making it an opt-in feature,\ne.g. \"git clone --recurse-submodules\" would work as the current\nusers would expect, while \"git clone --recurse-submodules=sticky\"\nwould set submodule.recurse to true, or something?\n"},{"id":"432353","messageId":"CAN3QUFYPjsvBRGegO-kC7+gcFDczOqQSw-UYphnLHx=6-6kkwA@mail.gmail.com","threadId":"56199","inReplyTo":"xmqqzgtqe2w6.fsf@gitster.g","subject":"Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla","fromEmail":"mahikolla@google.com","sentAt":"2021-08-10T07:26:01Z","receivedAt":"2021-08-10T07:26:17Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"Hi Junio,\n\nThank you for your feedback!\n\nOn Mon, Aug 9, 2021 at 2:15 PM Junio C Hamano wrote:\n>\n> \"Mahi Kolla via GitGitGadget\" writes:\n>\n> > From: Mahi Kolla \n> >\n> > 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'.\n>\n> Please wrap overlong lines in your proposed log message to say 70 or\n> so columns.\n>\n\nAh, my bad, will do so going forward.\n\n> Some developers might expect, but wouldn't some others want to see\n> this not set to true, but want to recurse only into some but not all\n> submodules?\n>\n\nI definitely agree with this. Currently, the `--recurse-submodules`\noption takes in 1 parameter, a pathspec to the submodule they would\nlike to initialize. `submodule.active` stores this path or `.` if no\npath is specified. Accordingly, `submodule.recurse=true` will only\napply to the submodules specified by the user in `submodule.active`.\nThis way users can ensure commands are run recursively only in\nsubmodules they have listed as active.\n\n\n> Is it possible to avoid changing the behaviour unconditionally and\n> potentially breaking existing users by making it an opt-in feature,\n> e.g. \"git clone --recurse-submodules\" would work as the current\n> users would expect, while \"git clone --recurse-submodules=sticky\"\n> would set submodule.recurse to true, or something?\n\nAs mentioned, the `submodule.recurse=true` will only apply to active\nsubmodules specified by the user. Setting this config value when the\nuser runs their initial `git clone` minimizes the number of times a\ndeveloper must use the `--recurse-submodule` option on other commands.\n\nHowever, this is a behavior change that may be surprising for\ndevelopers. To ensure a smooth rollout and easy adoption, I think\nadding a message using an `advice.*` config setting would be useful.\nWhen a user runs `git clone --recurse-submodules` an advice message\nwill pop up alerting them `submodule.recurse=true`, what this means\nfor other commands' functionality, and how to change it back (`git -C\ndst-dir config submodule.recurse false`). This also gives the user the\noption to turn off the advice message if they don't need it.\n\nI'll also update the appropriate documentation to better explain how\nsetting submodule.recurse=true effects workflow.\n\nLet me know what you think!\n\nBest,\nMahi Kolla\n"},{"id":"432400","messageId":"xmqqa6lpdu4z.fsf@gitster.g","threadId":"56199","inReplyTo":"CAN3QUFYPjsvBRGegO-kC7+gcFDczOqQSw-UYphnLHx=6-6kkwA@mail.gmail.com","subject":"Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-10T18:36:28Z","receivedAt":"2021-08-10T18:36:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mahi Kolla writes:\n\n>> Is it possible to avoid changing the behaviour unconditionally and\n>> potentially breaking existing users by making it an opt-in feature,\n>> e.g. \"git clone --recurse-submodules\" would work as the current\n>> users would expect, while \"git clone --recurse-submodules=sticky\"\n>> would set submodule.recurse to true, or something?\n>\n> As mentioned, the `submodule.recurse=true` will only apply to active\n> submodules specified by the user. Setting this config value when the\n> user runs their initial `git clone` minimizes the number of times a\n> developer must use the `--recurse-submodule` option on other commands.\n>\n> However, this is a behavior change that may be surprising for\n> developers. To ensure a smooth rollout and easy adoption, I think\n> adding a message using an `advice.*` config setting would be useful.\n\nIt may be better than nothing, but that still is a unilateral\nbehaviour change. Can't we come up with a way to make it an opt-in\nfeature? I've already suggested to allow the \"--recurse-submodules\"\noption of \"git clone\" to take an optional parameter (e.g. \"sticky\")\nso that the user can request configuration variable to be set, but\nyou seem to be ignoring or skirting it. Even though I am not\nmarried to the \"give optional parameter to --recurse-submodules\"\ndesign, unconditionally setting the variable, with or without advice\nor warning, is a regression we'd want to avoid.\n\n"},{"id":"432426","messageId":"8f24d532-b021-2a96-415b-467f715cb1ec@gmail.com","threadId":"56199","inReplyTo":"xmqqa6lpdu4z.fsf@gitster.g","subject":"Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-08-10T23:04:51Z","receivedAt":"2021-08-10T23:04:56Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Junio,\n\nLe 2021-08-10 à 14:36, Junio C Hamano a écrit :\n> Mahi Kolla writes:\n> \n>>> Is it possible to avoid changing the behaviour unconditionally and\n>>> potentially breaking existing users by making it an opt-in feature,\n>>> e.g. \"git clone --recurse-submodules\" would work as the current\n>>> users would expect, while \"git clone --recurse-submodules=sticky\"\n>>> would set submodule.recurse to true, or something?\n>>\n>> As mentioned, the `submodule.recurse=true` will only apply to active\n>> submodules specified by the user. Setting this config value when the\n>> user runs their initial `git clone` minimizes the number of times a\n>> developer must use the `--recurse-submodule` option on other commands.\n>>\n>> However, this is a behavior change that may be surprising for\n>> developers. To ensure a smooth rollout and easy adoption, I think\n>> adding a message using an `advice.*` config setting would be useful.\n> \n> It may be better than nothing, but that still is a unilateral\n> behaviour change. Can't we come up with a way to make it an opt-in\n> feature? I've already suggested to allow the \"--recurse-submodules\"\n> option of \"git clone\" to take an optional parameter (e.g. \"sticky\")\n> so that the user can request configuration variable to be set, but\n> you seem to be ignoring or skirting it. \n\nThe '--recures-submodule' option in 'git clone' already takes an optional\nargument, which is a pathspec and if given, only submodules matching the given\npathspec will be initialized (as opposed to all submodules if the flag is given without\nan argument). So, it does not seem to be possible to use this\nflag as a way to also set 'submodule.recurse'.\n\nWhen Emily (CC'ed) sent her roadmap for submodule enhancements in [1], the enhancement\nthat Mahi is suggesting was explicitely mentioned:\n\n> - git clone \n...\n> What doesn't already work:\n>\n> * --recurse-submodules should turn on submodule.recurse=true\n\nI don't know if Mahi is part of this effort or just came up with the same idea,\nbut in any case maybe Emily would be able to add more justification for this change.\n\n> Even though I am not\n> married to the \"give optional parameter to --recurse-submodules\"\n> design, unconditionally setting the variable, with or without advice\n> or warning, is a regression we'd want to avoid.\n> \n\nIn my opinion, it would not be a regression; it would a behaviour change that\nwould be a *vast improvement* for the majority of projects that use submodules, at\nleast those that use non-optional submodules (which, I believe, is the vast majority\nof projects that use submodules, judging by what I've read on the web over the past 3\nyears of my interest in the subject.)\n\nAs soon as you use submodules in a non-optional way, you really *want* submodule.recurse=true,\nbecause if not:\n\n1. 'git checkout' does not recursively check out your submodules, which probably breaks your build.\n You have to remember to always run 'git checkout --recurse-submodules' or run 'git submdule update'\n after each checkout, and teach your team to do the same.\n2. 'git pull' fetches submodules commits, but does not recursively check out your submodules,\n which also probably breaks your build. You have to remember to always run 'git pull --recurse-submodules',\n or run 'git submodule update' after each pull, and also teach your team to do so.\n3. If you forget to do 1. or 2., and then use 'git commit -am \"some message\" (as a lot\n of Git beginners unfortunately do), you regress the submodule commit, creating a lot\n of problems down the line.\n\nThese are the main reasons that I think Git should recurse by default. Setting 'submodule.recurse'\nalso brings other niceties, like 'git grep' recursing into submodules.\n\nIf we can agree that the behaviour *should* change eventually, then at least\n'git clone --recurse-submodules' could be changed *right now* to suggest setting\n'submodule.recurse' using the advice API, and stating that this will be the default\nsome day.\n\nEven if we don't agree that the behaviour should enventually change, I think\nhaving this advice would be a strict improvement because\nit would help user discover the setting, which would already go a long way.\n\nThanks,\n\nPhilippe.\n\n\n[1] https://lore.kernel.org/git/YHofmWcIAidkvJiD@google.com/\n"},{"id":"432428","messageId":"CAN3QUFaPBqZj68PYv_+=KV_cvbyhHfDtNPpWDbnnNeq+XD8MrQ@mail.gmail.com","threadId":"56199","inReplyTo":"8f24d532-b021-2a96-415b-467f715cb1ec@gmail.com","subject":"Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Mahi Kolla","fromEmail":"mahikolla@google.com","sentAt":"2021-08-10T23:59:11Z","receivedAt":"2021-08-10T23:59:28Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"Hi Phillippe and Junio,\n\nOn Tue, Aug 10, 2021 at 4:04 PM Philippe Blain\n wrote:\n>\n> Hi Junio,\n>\n> Le 2021-08-10 à 14:36, Junio C Hamano a écrit :\n> > Mahi Kolla writes:\n> >\n> >>> Is it possible to avoid changing the behaviour unconditionally and\n> >>> potentially breaking existing users by making it an opt-in feature,\n> >>> e.g. \"git clone --recurse-submodules\" would work as the current\n> >>> users would expect, while \"git clone --recurse-submodules=sticky\"\n> >>> would set submodule.recurse to true, or something?\n> >>\n> >> As mentioned, the `submodule.recurse=true` will only apply to active\n> >> submodules specified by the user. Setting this config value when the\n> >> user runs their initial `git clone` minimizes the number of times a\n> >> developer must use the `--recurse-submodule` option on other commands.\n> >>\n> >> However, this is a behavior change that may be surprising for\n> >> developers. To ensure a smooth rollout and easy adoption, I think\n> >> adding a message using an `advice.*` config setting would be useful.\n> >\n> > It may be better than nothing, but that still is a unilateral\n> > behaviour change. Can't we come up with a way to make it an opt-in\n> > feature? I've already suggested to allow the \"--recurse-submodules\"\n> > option of \"git clone\" to take an optional parameter (e.g. \"sticky\")\n> > so that the user can request configuration variable to be set, but\n> > you seem to be ignoring or skirting it.\n>\n> The '--recures-submodule' option in 'git clone' already takes an optional\n> argument, which is a pathspec and if given, only submodules matching the given\n> pathspec will be initialized (as opposed to all submodules if the flag is given without\n> an argument). So, it does not seem to be possible to use this\n> flag as a way to also set 'submodule.recurse'.\n>\n\nBecause of the optional pathspec argument, adding a `=sticky` argument\nto the option may be hard to implement. That was my initial hesitation\nto the opt in design.\n\n> When Emily (CC'ed) sent her roadmap for submodule enhancements in [1], the enhancement\n> that Mahi is suggesting was explicitely mentioned:\n>\n> > - git clone\n> ...\n> > What doesn't already work:\n> >\n> > * --recurse-submodules should turn on submodule.recurse=true\n>\n> I don't know if Mahi is part of this effort or just came up with the same idea,\n> but in any case maybe Emily would be able to add more justification for this change.\n>\n\nI am part of the team and am implementing that exact feature from the\nroadmap :)\n\n> > Even though I am not\n> > married to the \"give optional parameter to --recurse-submodules\"\n> > design, unconditionally setting the variable, with or without advice\n> > or warning, is a regression we'd want to avoid.\n> >\n>\n> In my opinion, it would not be a regression; it would a behaviour change that\n> would be a *vast improvement* for the majority of projects that use submodules, at\n> least those that use non-optional submodules (which, I believe, is the vast majority\n> of projects that use submodules, judging by what I've read on the web over the past 3\n> years of my interest in the subject.)\n>\n> As soon as you use submodules in a non-optional way, you really *want* submodule.recurse=true,\n> because if not:\n>\n> 1. 'git checkout' does not recursively check out your submodules, which probably breaks your build.\n> You have to remember to always run 'git checkout --recurse-submodules' or run 'git submdule update'\n> after each checkout, and teach your team to do the same.\n> 2. 'git pull' fetches submodules commits, but does not recursively check out your submodules,\n> which also probably breaks your build. You have to remember to always run 'git pull --recurse-submodules',\n> or run 'git submodule update' after each pull, and also teach your team to do so.\n> 3. If you forget to do 1. or 2., and then use 'git commit -am \"some message\" (as a lot\n> of Git beginners unfortunately do), you regress the submodule commit, creating a lot\n> of problems down the line.\n>\n> These are the main reasons that I think Git should recurse by default. Setting 'submodule.recurse'\n> also brings other niceties, like 'git grep' recursing into submodules.\n>\n\nI completely agree with this! These are a lot of the reasons why the\nfeature was initially suggested. An alternative path forward the team\ndiscussed was testing `submodule.recurse=true` under\n`feature.experimental`. This way we can collect feedback from\ndevelopers before making this the default config value.\n\n> If we can agree that the behaviour *should* change eventually, then at least\n> 'git clone --recurse-submodules' could be changed *right now* to suggest setting\n> 'submodule.recurse' using the advice API, and stating that this will be the default\n> some day.\n>\n> Even if we don't agree that the behaviour should enventually change, I think\n> having this advice would be a strict improvement because\n> it would help user discover the setting, which would already go a long way.\n>\n> Thanks,\n>\n> Philippe.\n>\n>\n> [1] https://lore.kernel.org/git/YHofmWcIAidkvJiD@google.com/\n\nI agree that adding an advice message when a user runs `git clone\n--recurse-submodules` would at least alert users of their options,\ngiving them the choice to set `submodule.recurse` accordingly.\n\nThanks!\n\nBest,\nMahi Kolla\n"},{"id":"432447","messageId":"xmqqbl64bmku.fsf@gitster.g","threadId":"56199","inReplyTo":"xmqqa6lpdu4z.fsf@gitster.g","subject":"Re: [PATCH v4] clone: update submodule.recurse in config when using --recurse-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-11T05:02:41Z","receivedAt":"2021-08-11T05:02:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Mahi Kolla writes:\n>\n>>> Is it possible to avoid changing the behaviour unconditionally and\n>>> potentially breaking existing users by making it an opt-in feature,\n>>> e.g. \"git clone --recurse-submodules\" would work as the current\n>>> users would expect, while \"git clone --recurse-submodules=sticky\"\n>>> would set submodule.recurse to true, or something?\n>>\n>> As mentioned, the `submodule.recurse=true` will only apply to active\n>> submodules specified by the user. Setting this config value when the\n>> user runs their initial `git clone` minimizes the number of times a\n>> developer must use the `--recurse-submodule` option on other commands.\n>>\n>> However, this is a behavior change that may be surprising for\n>> developers. To ensure a smooth rollout and easy adoption, I think\n>> adding a message using an `advice.*` config setting would be useful.\n\nLet me outline some general rules on changing the behaviour of the\nsystem used around here.\n\nFirst of all, if a proposed change of behaviour is a bugfix, the\nfollowing does not apply [*1*].\n\nWhen a new behaviour is made available to those who want to use it,\nit starts as an opt-in feature.\n\n - Existing users will not be surprised by a familiar command\n suddenly changing its behaviour. If users keep using the system\n the same way as they used it before, the system will behave the\n same way, without changing the behaviour.\n\n - Those who want to use the new behaviour need to do something to\n explicitly trigger it (with a command line option, configuration\n variable, a new command, etc.)\n\nOver time, a behaviour that used to be a \"new way\" may just become\n\"one of the two ways available\", and it may even turn out to be a\nmore desirable one between the two. At that point, we may propose\nto flip the default, with a migration plan that is carefully\ndesigned to avoid breaking existing users.\n\nEven if it were an *improvement* to set the configuration variable,\nit is not an excuse to suddenly change the behaviour of the command\nfor users who do not ask. It needs to start as an optional feature,\nand if we really like it and manage to convince majority users to\nalso like the new way, we may even consider making it the default,\nbut it is way too premature to do so.\n\nUnless we can argue that the current behaviour *is* buggy, that is.\n\nThanks.\n\n\n[Footnote]\n\n*1* A change that we have to say \"not all users may be happy with\n this new behaviour\" or \"developers would be surprised by the new\n behaviour\" cannot be a bugfix.\n"},{"id":"432557","messageId":"pull.1006.v5.git.1628736366133.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v4.git.1628536305810.gitgitgadget@gmail.com","subject":"[PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-12T02:46:06Z","receivedAt":"2021-08-12T02:46:12Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nCurrently, 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.\n\nSince V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n\nSigned-off-by: Mahi Kolla \n---\n clone: set submodule.recurse=true if feature.experimental flag enabled\n \n Based on current experience, when running git clone\n --recurse-submodules, developers do not expect other commands such as\n pull or checkout to run recursively into active submodules. However,\n setting submodule.recurse=true at this step could make for a simpler\n workflow by eliminating the need for the --recurse-submodules option in\n subsequent commands. To collect more data on developers' preference in\n regards to making submodule.recurse=true a default config value in the\n future, deploy this feature under the opt in feature.experimental flag.\n \n Since V1: Made this an opt in feature under the experimental flag.\n Updated tests to reflect this design change. Also updated commit\n message.\n \n Signed-off-by: Mahi Kolla mahikolla@google.com\n \n cc: Philippe Blain levraiphilippeblain@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1006\n\nRange-diff vs v4:\n\n 1: 73937d48a53 ! 1: 2c6ffe00736 clone: update submodule.recurse in config when using --recurse-submodule\n @@ Metadata\n Author: Mahi Kolla \n \n ## Commit message ##\n - clone: update submodule.recurse in config when using --recurse-submodule\n + clone: set submodule.recurse=true if user enables feature.experimental flag\n \n - 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'.\n + 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.\n \n - Since V1: Updated test and 'git clone' man page. Also updated commit message.\n + Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n \n Signed-off-by: Mahi Kolla \n \n - ## Documentation/git-clone.txt ##\n -@@ Documentation/git-clone.txt: branch of some repository for search indexing.\n - \tThis option can be given multiple times for pathspecs consisting\n - \tof multiple entries. The resulting clone has `submodule.active` set to\n - \tthe provided pathspec, or \".\" (meaning all submodules) if no\n --\tpathspec is provided.\n -+\tpathspec is provided. In addition, `submodule.recurse` is set to true.\n - +\n - Submodules are initialized and cloned using their default settings. This is\n - equivalent to running\n -\n ## builtin/clone.c ##\n +@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n + \tstruct remote *remote;\n + \tint err = 0, complete_refs_before_fetch = 1;\n + \tint submodule_progress;\n ++\tint experimental_flag;\n + \n + \tstruct transport_ls_refs_options transport_ls_refs_options =\n + \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n -+\t\tstring_list_append(&option_config, \"submodule.recurse=true\");\n ++\t\tif(!git_config_get_bool(\"feature.experimental\", &experimental_flag) &&\n ++\t\t experimental_flag) {\n ++\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n ++\t\t}\n ++\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\n @@ t/t5606-clone-options.sh: test_expect_success 'setup' '\n \n '\n \n -+test_expect_success 'clone --recurse-submodules sets submodule.recurse=true' '\n ++test_expect_success 'feature.experimental flag manipulates submodule.recurse value' '\n ++\n ++\ttest_config_global feature.experimental true &&\n ++\tgit clone --recurse-submodules parent clone_recurse_true &&\n ++\ttest_cmp_config -C clone_recurse_true true submodule.recurse &&\n +\n -+\tgit clone --recurse-submodules parent clone-rec-submodule &&\n -+\ttest_cmp_config -C clone-rec-submodule true submodule.recurse\n ++\ttest_config_global feature.experimental false &&\n ++\tgit clone --recurse-submodules parent clone_recurse_false &&\n ++\ttest_expect_code 1 git -C clone_recurse_false config --get submodule.recurse\n +\n +'\n +\n\n\n builtin/clone.c | 6 ++++++\n t/t5606-clone-options.sh | 12 ++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 66fe66679c8..24f242b4a60 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct remote *remote;\n \tint err = 0, complete_refs_before_fetch = 1;\n \tint submodule_progress;\n+\tint experimental_flag;\n \n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n@@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+\t\tif(!git_config_get_bool(\"feature.experimental\", &experimental_flag) &&\n+\t\t experimental_flag) {\n+\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n+\t\t}\n+\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3a595c0f82c..f20cdfa6fca 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -16,6 +16,18 @@ test_expect_success 'setup' '\n \n '\n \n+test_expect_success 'feature.experimental flag manipulates submodule.recurse value' '\n+\n+\ttest_config_global feature.experimental true &&\n+\tgit clone --recurse-submodules parent clone_recurse_true &&\n+\ttest_cmp_config -C clone_recurse_true true submodule.recurse &&\n+\n+\ttest_config_global feature.experimental false &&\n+\tgit clone --recurse-submodules parent clone_recurse_false &&\n+\ttest_expect_code 1 git -C clone_recurse_false config --get submodule.recurse\n+\n+'\n+\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n\nbase-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\n-- \ngitgitgadget\n"},{"id":"432558","messageId":"xmqqeeaz70ph.fsf@gitster.g","threadId":"56199","inReplyTo":"pull.1006.v5.git.1628736366133.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-12T04:20:58Z","receivedAt":"2021-08-12T04:21:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mahi Kolla via GitGitGadget\" writes:\n\n> From: Mahi Kolla \n>\n> 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.\n\nPlease wrap overlong lines in your proposed log message to say 70 or\nso columns.\n\n>\n> Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n\nThis does not belong to the commit log message proper. Noting the\ndifference between the version being submitted and the pervious one\nthis way is a way to help reviewers and is very much appreciated,\nbut please do so below the three-dash line below your sign-off.\n\n> Signed-off-by: Mahi Kolla \n> ---\n> clone: set submodule.recurse=true if feature.experimental flag enabled\n\nThe proposed approach misuses feature.experimental flag, which was\ndesigned to turn on many new features at once. The features covered\nby the flag share one common trait: they all have gained consensus\nthat in the longer term we would hopefully be able to make it on by\ndefault, and give early adopters an easy way to turn them all on.\n\nI do not think setting submodule.recurse=true upon \"clone --recurse\"\nfalls into that category just yet. If we were to make this opt-in,\nwe'd want a separate flag, so that those early adopters who are\ndogfooding other features that have consensus that they are\nhopefully the way of the future won't have to be forced into this\nseparate feature.\n\nPerhaps a separate (and new) configuration variable (in ~/.gitconfig\nperhaps) can be used as that opt-in flag (I wonder if the existing\nsubmodule.recurse variable can be that opt-in flag, though).\n\n"},{"id":"432618","messageId":"YRW0pGXXWnY7C470@google.com","threadId":"56199","inReplyTo":"xmqqeeaz70ph.fsf@gitster.g","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-08-12T23:54:12Z","receivedAt":"2021-08-12T23:54:24Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Aug 11, 2021 at 09:20:58PM -0700, Junio C Hamano wrote:\n> \n> \"Mahi Kolla via GitGitGadget\" writes:\n> \n> > From: Mahi Kolla \n> >\n> > 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.\n> \n> Please wrap overlong lines in your proposed log message to say 70 or\n> so columns.\n> \n> >\n> > Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n> \n> This does not belong to the commit log message proper. Noting the\n> difference between the version being submitted and the pervious one\n> this way is a way to help reviewers and is very much appreciated,\n> but please do so below the three-dash line below your sign-off.\n> \n> > Signed-off-by: Mahi Kolla \n> > ---\n> > clone: set submodule.recurse=true if feature.experimental flag enabled\n> \n> The proposed approach misuses feature.experimental flag, which was\n> designed to turn on many new features at once. The features covered\n> by the flag share one common trait: they all have gained consensus\n> that in the longer term we would hopefully be able to make it on by\n> default, and give early adopters an easy way to turn them all on.\n> \n> I do not think setting submodule.recurse=true upon \"clone --recurse\"\n> falls into that category just yet. If we were to make this opt-in,\n> we'd want a separate flag, so that those early adopters who are\n> dogfooding other features that have consensus that they are\n> hopefully the way of the future won't have to be forced into this\n> separate feature.\n\nI'd like to open discussions to get said consensus :)\n\nIt seems surprising to me that a user would want to clone with all the\nsubmodules fetched *without* intending to then use\nsuperproject-plus-submodules together recursively. I would like to hear\nmore about the use case you have in mind, Junio.\n\nOne scenario that did come to mind when I discussed this with Mahi is\nthat a user may provide a pathspec to --recurse-submodules (that is,\n\"yes, this repo has submodules a/ and b/, but I only care about the\ncontents of submodule a/\") - and in that case, --recurse-submodules\nseems to do the right thing with or without Mahi's change.\n\nIt seemed to me that trying out this change on feature.experimental flag\nwas the right approach, because users with that flag have already\nvolunteered to be testers for upcoming behavior changes; this seems like\none such that is likely to be welcome. By contrast, turning the behavior\non with a separate config variable reduces the pool of testers\nessentially to \"users who know about this change\" - or, to be more\nreductive, \"a handful of users at Google who we Google Git contributors\nalready know want this change\". I recommended to Mahi that we stick this\nfeature under 'feature.experimental' because I really wanted to hear\nfrom more users than just Googlers.\n\n> \n> Perhaps a separate (and new) configuration variable (in ~/.gitconfig\n> perhaps) can be used as that opt-in flag (I wonder if the existing\n> submodule.recurse variable can be that opt-in flag, though).\n> \n\nDo you mean something like \"git config --global submodule.recurse\nTryTheNewThingPlease\"? I guess it could work - repos that use a pathspec\nin that slot would still have the pathspec configured locally, repos\nthat have submodule.recurse intentionally unset wouldn't know what to do\nwith the junk string, and repos that have submodule.recurse\nintentionally set to true would still have that true override the global\nvalue.\n\nOr else I misunderstood you...\n\n - Emily\n"},{"id":"432620","messageId":"c74a9d75-cd89-d020-dcb3-76509bc95284@gmail.com","threadId":"56199","inReplyTo":"YRW0pGXXWnY7C470@google.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-08-13T03:35:39Z","receivedAt":"2021-08-13T03:35:47Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Emily,\n\nLe 2021-08-12 à 19:54, Emily Shaffer a écrit :\n> On Wed, Aug 11, 2021 at 09:20:58PM -0700, Junio C Hamano wrote:\n>>\n>> \"Mahi Kolla via GitGitGadget\" writes:\n>>\n>>> From: Mahi Kolla \n>>>\n>>> 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.\n>>\n>> Please wrap overlong lines in your proposed log message to say 70 or\n>> so columns.\n>>\n>>>\n>>> Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n>>\n>> This does not belong to the commit log message proper. Noting the\n>> difference between the version being submitted and the pervious one\n>> this way is a way to help reviewers and is very much appreciated,\n>> but please do so below the three-dash line below your sign-off.\n\nMahi, since you're using Gitgitgadget, you would put this \"since v1\"\ncontent in the PR description, and Gitgitgadget will append it under\nthe three-dash line when you /submit :) (Do keep the CC's automatically\nadded by GGG so that your next version is CC'ed to those that participated\nin earlier rounds).\n\n>>\n>>> Signed-off-by: Mahi Kolla \n>>> ---\n>>> clone: set submodule.recurse=true if feature.experimental flag enabled\n>>\n>> The proposed approach misuses feature.experimental flag, which was\n>> designed to turn on many new features at once. The features covered\n>> by the flag share one common trait: they all have gained consensus\n>> that in the longer term we would hopefully be able to make it on by\n>> default, and give early adopters an easy way to turn them all on.\n>>\n>> I do not think setting submodule.recurse=true upon \"clone --recurse\"\n>> falls into that category just yet. If we were to make this opt-in,\n>> we'd want a separate flag, so that those early adopters who are\n>> dogfooding other features that have consensus that they are\n>> hopefully the way of the future won't have to be forced into this\n>> separate feature.\n> \n> I'd like to open discussions to get said consensus :)\n> \n> It seems surprising to me that a user would want to clone with all the\n> submodules fetched *without* intending to then use\n> superproject-plus-submodules together recursively. I would like to hear\n> more about the use case you have in mind, Junio.\n> \n> One scenario that did come to mind when I discussed this with Mahi is\n> that a user may provide a pathspec to --recurse-submodules (that is,\n> \"yes, this repo has submodules a/ and b/, but I only care about the\n> contents of submodule a/\") - and in that case, --recurse-submodules\n> seems to do the right thing with or without Mahi's change.\n\nI'm not sure what you mean by \"the right thing\" here. '--recurse-submodules=a'\nwould set 'submodule.active' to 'a', which means \"when command are asked to recurse into\nsubmodules, I only care about submodules a\", but it does not do anything to\n'submodule.recurse=true', which means \"I do not ever want to type '--recurse-submodules',\nalways use this behaviour for all commands that have that flag, except clone and ls-files.\nUnless I'm missing something :)\n\n> \n> It seemed to me that trying out this change on feature.experimental flag\n> was the right approach, because users with that flag have already\n> volunteered to be testers for upcoming behavior changes; this seems like\n> one such that is likely to be welcome. By contrast, turning the behavior\n> on with a separate config variable reduces the pool of testers\n> essentially to \"users who know about this change\" - or, to be more\n> reductive, \"a handful of users at Google who we Google Git contributors\n> already know want this change\". I recommended to Mahi that we stick this\n> feature under 'feature.experimental' because I really wanted to hear\n> from more users than just Googlers.\n\nI agree that we would not want yet another config variable that users would\nhave to set. If people know about submodule.recurse and want to always use this\nbehaviour, they already have it in their ~/.gitconfig, so they do not need a new\nvariable. If they do not know about submodule.recurse, then they probably won't learn\nabout this new variable either ;) That's why I suggested to Mahi that in any case it would\nbe a good thing that 'git clone --recurse-submodules' would at least inform users, using\nan advice, that they might want to set submodule.recurse.\n\nRegarding feature.experimental, I do not have a strong opinion. I don't think\nthe population of Git users that have this flag set is representative of the total\npopulation of Git users, unfortunately. But I agree it's better than nothing.\n\n> \n>>\n>> Perhaps a separate (and new) configuration variable (in ~/.gitconfig\n>> perhaps) can be used as that opt-in flag (I wonder if the existing\n>> submodule.recurse variable can be that opt-in flag, though).\n>>\n> \n> Do you mean something like \"git config --global submodule.recurse\n> TryTheNewThingPlease\"? I guess it could work - repos that use a pathspec\n> in that slot would still have the pathspec configured locally, \n\nHere I think you are confusing submodule.active (which takes a pathspec)\nand submodule.recurse (which takes a boolean).\n\nCheers,\n\nPhilippe.\n"},{"id":"432621","messageId":"CAN3QUFa3TRh9rvV3xS8XWd-dTvMqS=2j5Vhvp0wP33oPte9_2w@mail.gmail.com","threadId":"56199","inReplyTo":"c74a9d75-cd89-d020-dcb3-76509bc95284@gmail.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Mahi Kolla","fromEmail":"mahikolla@google.com","sentAt":"2021-08-13T04:12:15Z","receivedAt":"2021-08-13T04:12:31Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"Hi all,\n\nThank you all for the great feedback! I'm learning a lot as a\nfirst-time contributor :) I will be wrapping my internship this week\nand will continue contributing externally.\n\n> >>\n> >>>\n> >>> Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n> >>\n> >> This does not belong to the commit log message proper. Noting the\n> >> difference between the version being submitted and the pervious one\n> >> this way is a way to help reviewers and is very much appreciated,\n> >> but please do so below the three-dash line below your sign-off.\n>\n> Mahi, since you're using Gitgitgadget, you would put this \"since v1\"\n> content in the PR description, and Gitgitgadget will append it under\n> the three-dash line when you /submit :) (Do keep the CC's automatically\n> added by GGG so that your next version is CC'ed to those that participated\n> in earlier rounds).\n>\n\nGot it, thank you!\n\n> >\n> > It seemed to me that trying out this change on feature.experimental flag\n> > was the right approach, because users with that flag have already\n> > volunteered to be testers for upcoming behavior changes; this seems like\n> > one such that is likely to be welcome. By contrast, turning the behavior\n> > on with a separate config variable reduces the pool of testers\n> > essentially to \"users who know about this change\" - or, to be more\n> > reductive, \"a handful of users at Google who we Google Git contributors\n> > already know want this change\". I recommended to Mahi that we stick this\n> > feature under 'feature.experimental' because I really wanted to hear\n> > from more users than just Googlers.\n>\n> I agree that we would not want yet another config variable that users would\n> have to set. If people know about submodule.recurse and want to always use this\n> behaviour, they already have it in their ~/.gitconfig, so they do not need a new\n> variable. If they do not know about submodule.recurse, then they probably won't learn\n> about this new variable either ;) That's why I suggested to Mahi that in any case it would\n> be a good thing that 'git clone --recurse-submodules' would at least inform users, using\n> an advice, that they might want to set submodule.recurse.\n>\n\nWhen discussing with the team, we revisited the feature.experimental\ndesign. As we have yet to gain strong consensus on making this a\ndefault config value, we've decided to ship it under a different\nconfig value: submodule.stickyRecursiveClone. Now, if the user sets\nsubmodule.stickyRecursiveClone=true, when they run git clone\n--recurse-submodules, we will set submodule.recurse=true. While this\nmay mean a smaller dataset (only people who know of this flag), we can\nstill collect valuable data.\n\nAs for the advice message, I agree that would be a really useful\nfeature. I'll submit that as a different patch.\n\n> >>\n> >> Perhaps a separate (and new) configuration variable (in ~/.gitconfig\n> >> perhaps) can be used as that opt-in flag (I wonder if the existing\n> >> submodule.recurse variable can be that opt-in flag, though).\n> >>\n\nUnfortunately, the submodule.recurse variable can't be used as the\nopt-in flag because this would cause commands to run recursively even\nif developers don't have submodules in their project (aka don't run\ngit clone --recurse-submodules). That's why the alternate config value\nseems a better choice at the moment.\n\nLet me know what you guys think!\n\nBest,\nMahi Kolla\n"},{"id":"432622","messageId":"xmqqpmuiynbs.fsf@gitster.g","threadId":"56199","inReplyTo":"YRW0pGXXWnY7C470@google.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-13T04:34:47Z","receivedAt":"2021-08-13T04:34:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> It seems surprising to me that a user would want to clone with all the\n> submodules fetched *without* intending to then use\n> superproject-plus-submodules together recursively. I would like to hear\n> more about the use case you have in mind, Junio.\n\nYou may need full forest of submodules with the superproject to\nbuild your ware (i.e. you'd probably want to clone and fetch-update\nthem), but you may only be working on the sources in a small subset\nof submodules and do not need your recursive grep or diff to go\noutside that subset, for example. You'd need to ask the people who\nrecursively clone and not set submodule.recurse to true (I am not\namong them).\n\n> One scenario that did come to mind when I discussed this with Mahi is\n> that a user may provide a pathspec to --recurse-submodules (that is,\n> \"yes, this repo has submodules a/ and b/, but I only care about the\n> contents of submodule a/\") - and in that case, --recurse-submodules\n> seems to do the right thing with or without Mahi's change.\n\nPlease be a bit more specific about \"the right thing\". Do you mean\n\"the submodules that matched the pathspec gets recursed into by\nlater operations\"?\n\nIf so, \"git clone --resurse-submodules=. $from_there\" may perhaps be\nthe \"there is no way to we make this opt-in?\" I have been asking\nabout (not \"asking for\")?\n\n> It seemed to me that trying out this change on feature.experimental flag\n> was the right approach, because users with that flag have already\n> volunteered to be testers for upcoming behavior changes\n\nYes, if we already have a consensus that a proposed change is\nsomething we hope to be desirable, then feature.experimental is a\ngood way to see if early adopters can find problems in their real\nworld use, as these volunteers may include audiences with different\nuse pattern from the original advocates of a particular feature, who\nmight have dogfooded the new feature to gain consensus that it may\nwant to become the default.\n\nBy the way, I am not fundamentally opposed to the feature being\nproposed. I would imagine that such a feature would be liked by\nthose who want to keep things simpler. I however am hesitant to see\nit pushed too hastily without considering if it harms existing users\nwith different preferences.\n\nIOW, I was primarily reacting to the apparent wrong order in which\nthings are being done, first throwing this into feature.experimental\nbefore we have gathered enough confidence that it may be a good\nthing to do by having it in shipped version as an opt-in feature.\n\nThanks.\n"},{"id":"432676","messageId":"xmqqr1exxuew.fsf@gitster.g","threadId":"56199","inReplyTo":"c74a9d75-cd89-d020-dcb3-76509bc95284@gmail.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-13T14:59:19Z","receivedAt":"2021-08-13T14:59:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n> Here I think you are confusing submodule.active (which takes a pathspec)\n> and submodule.recurse (which takes a boolean).\n\nSigh, but I have to agree with you.\n\nI really hoped that in a recursive clone of a repository with two\nsubmodules A and B, when made with --recurse-submodules=A, \"git grep\n.\" would look for things in the superproject and submodule A as\nEmily seemed to have meant by her \"the right thing\", but you are\ncorrect. We only set .active but we do not set .recurse, so \"git\ngrep .\" in the superproject does not descend into neither\nsubmodules without being told.\n\nIf it did \"the right thing\" as Emily said, it would have been much\neasier to justify the change being proposed as a simple fix for the\nbug that --recurse-submodules without pathspec does one thing\n(i.e. setting things up not to recurse for later \"grep\" etc.\") and\nthe same option with \"everything matches\" pathspec \".\" does a\ndifferent thing (i.e. always to recurse).\n\nThe discrepancy would have given us an excuse, an argument for\nchanging the behaviour for the former to match the latter. Some\nusers may have deliberately built their workflow relying on the\ndistinction and the result still may give them a regression, but at\nleast it would have gave us a viable justification:\n\n A command run without pathspec means the entire tree and it is\n the same as running it with pathspec '.' in the rest of Git, but\n the way \"git clone --recurse-submodules\" handles its optional\n pathspec is inconsistent. Treat \"clone --recurse-submodules\"\n without pathspec as if it came with pathspec '.' and give the\n same configuration.\n\nBut unfortunately it does not seem to be the case.\n"},{"id":"432692","messageId":"6116acf7a96f9_dde02087e@natae.notmuch","threadId":"56199","inReplyTo":"pull.1006.v5.git.1628736366133.gitgitgadget@gmail.com","subject":"RE: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-08-13T17:33:43Z","receivedAt":"2021-08-13T17:33:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Mahi Kolla via GitGitGadget wrote:\n> From: Mahi Kolla \n\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tstruct remote *remote;\n> \tint err = 0, complete_refs_before_fetch = 1;\n> \tint submodule_progress;\n> +\tint experimental_flag;\n> \n> \tstruct transport_ls_refs_options transport_ls_refs_options =\n> \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n> @@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t strbuf_detach(&sb, NULL));\n> \t\t}\n> \n> +\t\tif(!git_config_get_bool(\"feature.experimental\", &experimental_flag) &&\n\nYou are missing a space after the 'if'.\n\n> +\t\t experimental_flag) {\n> +\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n> +\t\t}\n> +\n\n-- \nFelipe Contreras\n"},{"id":"432700","messageId":"xmqqeeaxw28z.fsf@gitster.g","threadId":"56199","inReplyTo":"CAN3QUFa3TRh9rvV3xS8XWd-dTvMqS=2j5Vhvp0wP33oPte9_2w@mail.gmail.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-13T19:53:00Z","receivedAt":"2021-08-13T19:53:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mahi Kolla writes:\n\n> Unfortunately, the submodule.recurse variable can't be used as the\n> opt-in flag because this would cause commands to run recursively even\n> if developers don't have submodules in their project (aka don't run\n> git clone --recurse-submodules).\n\nIf you cloned without recurse-submodules and lack submodules,\nwouldn't it be a no-op to have submodule.recurse set to true, so it\nwould not hurt anyway?\n\nIOW, that may already solve the original problem you wanted to\nsolve---those who want their submodules recursively descended into\nby default can just set submodule.recurse to true (in ~/.gitconfig\npresumably) and after \"git clone\" with --recurse-submodules they\nwill get what they want, no? Am I missing something obvious?\n\nThanks.\n\n\n\n\n\n"},{"id":"432702","messageId":"YRbU0uBxqinQ1EnE@google.com","threadId":"56199","inReplyTo":"xmqqpmuiynbs.fsf@gitster.g","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-08-13T20:23:46Z","receivedAt":"2021-08-13T20:23:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Aug 12, 2021 at 09:34:47PM -0700, Junio C Hamano wrote:\n> \n> Emily Shaffer writes:\n> \n> > It seems surprising to me that a user would want to clone with all the\n> > submodules fetched *without* intending to then use\n> > superproject-plus-submodules together recursively. I would like to hear\n> > more about the use case you have in mind, Junio.\n> \n> You may need full forest of submodules with the superproject to\n> build your ware (i.e. you'd probably want to clone and fetch-update\n> them), but you may only be working on the sources in a small subset\n> of submodules and do not need your recursive grep or diff to go\n> outside that subset, for example. You'd need to ask the people who\n> recursively clone and not set submodule.recurse to true (I am not\n> among them).\n> \n> > One scenario that did come to mind when I discussed this with Mahi is\n> > that a user may provide a pathspec to --recurse-submodules (that is,\n> > \"yes, this repo has submodules a/ and b/, but I only care about the\n> > contents of submodule a/\") - and in that case, --recurse-submodules\n> > seems to do the right thing with or without Mahi's change.\n> \n> Please be a bit more specific about \"the right thing\". Do you mean\n> \"the submodules that matched the pathspec gets recursed into by\n> later operations\"?\n> \n> If so, \"git clone --resurse-submodules=. $from_there\" may perhaps be\n> the \"there is no way to we make this opt-in?\" I have been asking\n> about (not \"asking for\")?\n> \n> > It seemed to me that trying out this change on feature.experimental flag\n> > was the right approach, because users with that flag have already\n> > volunteered to be testers for upcoming behavior changes\n> \n> Yes, if we already have a consensus that a proposed change is\n> something we hope to be desirable, then feature.experimental is a\n> good way to see if early adopters can find problems in their real\n> world use, as these volunteers may include audiences with different\n> use pattern from the original advocates of a particular feature, who\n> might have dogfooded the new feature to gain consensus that it may\n> want to become the default.\n> \n> By the way, I am not fundamentally opposed to the feature being\n> proposed. I would imagine that such a feature would be liked by\n> those who want to keep things simpler. I however am hesitant to see\n> it pushed too hastily without considering if it harms existing users\n> with different preferences.\n> \n> IOW, I was primarily reacting to the apparent wrong order in which\n> things are being done, first throwing this into feature.experimental\n> before we have gathered enough confidence that it may be a good\n> thing to do by having it in shipped version as an opt-in feature.\n\nYeah, since writing my reply I was very helpfully reinformed on the\nconvention around 'feature.experimental' by Jonathan N off-list. Thanks\nfor being patient with me.\n\nI think the right move, then, is to explore whether your suggestion in\nhttps://lore.kernel.org/git/xmqqeeaxw28z.fsf%40gitster.g is appropriate\n- I have the sense that it is, but I want to make sure to think it\nthrough before I say so for sure. \n\n - Emily\n"},{"id":"432703","messageId":"xmqqv949uly9.fsf@gitster.g","threadId":"56199","inReplyTo":"YRbU0uBxqinQ1EnE@google.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-13T20:30:22Z","receivedAt":"2021-08-13T20:30:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> I think the right move, then, is to explore whether your suggestion in\n> https://lore.kernel.org/git/xmqqeeaxw28z.fsf%40gitster.g is appropriate\n> - I have the sense that it is, but I want to make sure to think it\n> through before I say so for sure. \n\nNot that one---it was a 40% tongue-in-cheek comment, and does not\ndeserve to be called a suggestion. \n"},{"id":"432704","messageId":"YRbYWR+X8vSq8CYe@google.com","threadId":"56199","inReplyTo":"xmqqv949uly9.fsf@gitster.g","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-08-13T20:38:49Z","receivedAt":"2021-08-13T20:39:00Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Aug 13, 2021 at 01:30:22PM -0700, Junio C Hamano wrote:\n> \n> Emily Shaffer writes:\n> \n> > I think the right move, then, is to explore whether your suggestion in\n> > https://lore.kernel.org/git/xmqqeeaxw28z.fsf%40gitster.g is appropriate\n> > - I have the sense that it is, but I want to make sure to think it\n> > through before I say so for sure. \n> \n> Not that one---it was a 40% tongue-in-cheek comment, and does not\n> deserve to be called a suggestion. \n\nAh well ;)\n\nAnyway, I think it does not make sense, as behavior starts to change for\npeople who already cloned expecting not to recurse (Jonathan N says this\nis the case for his Rust checkout, for example) - and apparently\n'submodule.recurse=true' has some weird edge cases for commands which\nare happy to run out-of-repo.\n\nMahi mentioned wanting to rework her commit to use a config besides\n'feature.experimental' for this same behavior, so hopefully we will see\nthat change come through soon - but today is also the last day of her\ninternship, so we may not be so lucky.\n\n - Emily\n"},{"id":"432705","messageId":"CAN3QUFY5vxb2s0f+5xyt4UQbXwWqqDrOxXz9p0mtQ3qyDfJ0zw@mail.gmail.com","threadId":"56199","inReplyTo":"YRbYWR+X8vSq8CYe@google.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Mahi Kolla","fromEmail":"mahikolla@google.com","sentAt":"2021-08-13T20:48:21Z","receivedAt":"2021-08-13T20:48:37Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"> Mahi mentioned wanting to rework her commit to use a config besides\n> 'feature.experimental' for this same behavior, so hopefully we will see\n> that change come through soon - but today is also the last day of her\n> internship, so we may not be so lucky.\n>\n\nThis change is pretty much good to go! I implemented it under\n`submodule.stickyRecursiveClone`. The commit is actually in the PR.\nJust wanted to hear more from you guys before submitting :)\n"},{"id":"432706","messageId":"xmqqlf55ukx7.fsf@gitster.g","threadId":"56199","inReplyTo":"YRbU0uBxqinQ1EnE@google.com","subject":"Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-13T20:52:36Z","receivedAt":"2021-08-13T20:52:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> Yeah, since writing my reply I was very helpfully reinformed on the\n> convention around 'feature.experimental' by Jonathan N off-list. Thanks\n> for being patient with me.\n\nYou may (or may not) have noticed it, but the entry for the topic in\nthe last \"What's cooking\" report pretends as if we have already\nconcensus that it is a good thing to do and be made default. As it\nseems to be valid use cases to have submodule.recurse not set to\ntrue even when you have submodules (a clone of Rust Jonathan N uses\nyou mentioned in the other message, perhaps?), it probably does make\nsense to initially make this opt-in and also keep opt-in, not as a\ncandidate for future default, but we'll see.\n\nThanks.\n\n\n"},{"id":"432722","messageId":"pull.1006.v6.git.1628903396783.gitgitgadget@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v5.git.1628736366133.gitgitgadget@gmail.com","subject":"[PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Mahi Kolla via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-14T01:09:56Z","receivedAt":"2021-08-14T01:10:02Z","isPatch":true,"sender":{"key":"mahikolla@google.com","avatar":null},"body":"From: Mahi Kolla \n\nBased on current experience, when running git clone --recurse-submodules,\ndevelopers do not expect other commands such as pull or checkout to run\nrecursively into active submodules. However, setting submodule.recurse=true\nat this step could make for a simpler workflow by eliminating the need for\nthe --recurse-submodules option in subsequent commands. To collect more\ndata on developers' preference in regards to making submodule.recurse=true\na default config value in the future, deploy this feature under the opt in\nsubmodule.stickyRecursiveClone flag.\n\nSigned-off-by: Mahi Kolla \n---\n clone: set submodule.recurse=true if submodule.stickyRecursiveClone\n enabled\n \n Based on current experience, when running git clone\n --recurse-submodules, developers do not expect other commands such as\n pull or checkout to run recursively into active submodules. However,\n setting submodule.recurse=true at this step could make for a simpler\n workflow by eliminating the need for the --recurse-submodules option in\n subsequent commands. To collect more data on developers' behavior and\n preferences when making submodule.recurse=true a default, deploy this\n feature under the opt in submodule.stickyRecursiveClone flag.\n \n Signed-off-by: Mahi Kolla mkolla2@illinois.edu\n \n Since V1: Made this an opt in feature under a custom\n submodule.stickyRecursiveClone flag as opposed to feature.experimental.\n Updated tests to reflect this design change. Also updated commit\n message. Additionally, I will be contributing from my personal email\n going forward as opposed to my @google email.\n \n cc: Philippe Blain levraiphilippeblain@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/1006\n\nRange-diff vs v5:\n\n 1: 2c6ffe00736 ! 1: e668fa403cf clone: set submodule.recurse=true if user enables feature.experimental flag\n @@\n ## Metadata ##\n -Author: Mahi Kolla \n +Author: Mahi Kolla \n \n ## Commit message ##\n - clone: set submodule.recurse=true if user enables feature.experimental flag\n + clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled\n \n - 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.\n + Based on current experience, when running git clone --recurse-submodules,\n + developers do not expect other commands such as pull or checkout to run\n + recursively into active submodules. However, setting submodule.recurse=true\n + at this step could make for a simpler workflow by eliminating the need for\n + the --recurse-submodules option in subsequent commands. To collect more\n + data on developers' preference in regards to making submodule.recurse=true\n + a default config value in the future, deploy this feature under the opt in\n + submodule.stickyRecursiveClone flag.\n \n - Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.\n -\n - Signed-off-by: Mahi Kolla \n + Signed-off-by: Mahi Kolla \n \n ## builtin/clone.c ##\n @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct remote *remote;\n \tint err = 0, complete_refs_before_fetch = 1;\n \tint submodule_progress;\n -+\tint experimental_flag;\n ++\tint sticky_recursive_clone;\n \n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n -+\t\tif(!git_config_get_bool(\"feature.experimental\", &experimental_flag) &&\n -+\t\t experimental_flag) {\n ++\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &sticky_recursive_clone)\n ++\t\t && sticky_recursive_clone) {\n +\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n +\t\t}\n +\n @@ t/t5606-clone-options.sh: test_expect_success 'setup' '\n \n '\n \n -+test_expect_success 'feature.experimental flag manipulates submodule.recurse value' '\n ++test_expect_success 'submodule.stickyRecursiveClone flag manipulates submodule.recurse value' '\n +\n -+\ttest_config_global feature.experimental true &&\n ++\ttest_config_global submodule.stickyRecursiveClone true &&\n +\tgit clone --recurse-submodules parent clone_recurse_true &&\n +\ttest_cmp_config -C clone_recurse_true true submodule.recurse &&\n +\n -+\ttest_config_global feature.experimental false &&\n ++\ttest_config_global submodule.stickyRecursiveClone false &&\n +\tgit clone --recurse-submodules parent clone_recurse_false &&\n +\ttest_expect_code 1 git -C clone_recurse_false config --get submodule.recurse\n +\n\n\n builtin/clone.c | 6 ++++++\n t/t5606-clone-options.sh | 12 ++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 66fe66679c8..a08d9012243 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct remote *remote;\n \tint err = 0, complete_refs_before_fetch = 1;\n \tint submodule_progress;\n+\tint sticky_recursive_clone;\n \n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n@@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &sticky_recursive_clone)\n+\t\t && sticky_recursive_clone) {\n+\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n+\t\t}\n+\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex 3a595c0f82c..d822153e4d2 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -16,6 +16,18 @@ test_expect_success 'setup' '\n \n '\n \n+test_expect_success 'submodule.stickyRecursiveClone flag manipulates submodule.recurse value' '\n+\n+\ttest_config_global submodule.stickyRecursiveClone true &&\n+\tgit clone --recurse-submodules parent clone_recurse_true &&\n+\ttest_cmp_config -C clone_recurse_true true submodule.recurse &&\n+\n+\ttest_config_global submodule.stickyRecursiveClone false &&\n+\tgit clone --recurse-submodules parent clone_recurse_false &&\n+\ttest_expect_code 1 git -C clone_recurse_false config --get submodule.recurse\n+\n+'\n+\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n\nbase-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd\n-- \ngitgitgadget\n"},{"id":"432738","messageId":"xmqqy293ucju.fsf@gitster.g","threadId":"56199","inReplyTo":"pull.1006.v6.git.1628903396783.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-14T18:05:41Z","receivedAt":"2021-08-14T18:05:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mahi Kolla via GitGitGadget\" writes:\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 66fe66679c8..a08d9012243 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tstruct remote *remote;\n> \tint err = 0, complete_refs_before_fetch = 1;\n> \tint submodule_progress;\n> +\tint sticky_recursive_clone;\n\nThis variable does not have to be in such a wider scope, I think.\n\n\n> \tstruct transport_ls_refs_options transport_ls_refs_options =\n> \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n> @@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t strbuf_detach(&sb, NULL));\n> \t\t}\n\nJust in this scope, where \"struct string_list_item *item\" and\n\"struct strbuf sb\" are declared, is where an extra int variable,\nwhich receives the configuration value, needs to exist.\n\nAlso, for a variable that is used only to receive value from\ngit_config_get_bool(), immediately to be used and then never used\nagain, we do not need such a long and descriptive name.\n\n> +\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &sticky_recursive_clone)\n> +\t\t && sticky_recursive_clone) {\n> +\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n> +\t\t}\n\nWe do not need {} around a single statement block.\n\nTaken together, perhaps like the attached.\n\nI'll queue the patch posted as-is for now.\n\nThanks.\n\ndiff --git c/builtin/clone.c w/builtin/clone.c\nindex 66fe66679c..c4e02d2f78 100644\n--- c/builtin/clone.c\n+++ w/builtin/clone.c\n@@ -1114,6 +1114,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_recurse_submodules.nr > 0) {\n \t\tstruct string_list_item *item;\n \t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tint val;\n \n \t\t/* remove duplicates */\n \t\tstring_list_sort(&option_recurse_submodules);\n@@ -1130,6 +1131,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n+\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &val) &&\n+\t\t val)\n+\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n+\n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n \t\t\tdie(_(\"clone --recursive is not compatible with \"\n"},{"id":"433038","messageId":"YRw/8tThN7djNE+E@google.com","threadId":"56199","inReplyTo":"xmqqy293ucju.fsf@gitster.g","subject":"Re: [PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-08-17T23:02:10Z","receivedAt":"2021-08-17T23:02:21Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Sat, Aug 14, 2021 at 11:05:41AM -0700, Junio C Hamano wrote:\n> \n> \"Mahi Kolla via GitGitGadget\" writes:\n> \n> > diff --git a/builtin/clone.c b/builtin/clone.c\n> > index 66fe66679c8..a08d9012243 100644\n> > --- a/builtin/clone.c\n> > +++ b/builtin/clone.c\n> > @@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> > \tstruct remote *remote;\n> > \tint err = 0, complete_refs_before_fetch = 1;\n> > \tint submodule_progress;\n> > +\tint sticky_recursive_clone;\n> \n> This variable does not have to be in such a wider scope, I think.\n> \n> \n> > \tstruct transport_ls_refs_options transport_ls_refs_options =\n> > \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n> > @@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> > \t\t\t\t\t strbuf_detach(&sb, NULL));\n> > \t\t}\n> \n> Just in this scope, where \"struct string_list_item *item\" and\n> \"struct strbuf sb\" are declared, is where an extra int variable,\n> which receives the configuration value, needs to exist.\n> \n> Also, for a variable that is used only to receive value from\n> git_config_get_bool(), immediately to be used and then never used\n> again, we do not need such a long and descriptive name.\n> \n> > +\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &sticky_recursive_clone)\n> > +\t\t && sticky_recursive_clone) {\n> > +\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n> > +\t\t}\n> \n> We do not need {} around a single statement block.\n> \n> Taken together, perhaps like the attached.\n> \n> I'll queue the patch posted as-is for now.\n> \n> Thanks.\n> \n> diff --git c/builtin/clone.c w/builtin/clone.c\n> index 66fe66679c..c4e02d2f78 100644\n> --- c/builtin/clone.c\n> +++ w/builtin/clone.c\n> @@ -1114,6 +1114,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tif (option_recurse_submodules.nr > 0) {\n> \t\tstruct string_list_item *item;\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> +\t\tint val;\n> \n> \t\t/* remove duplicates */\n> \t\tstring_list_sort(&option_recurse_submodules);\n> @@ -1130,6 +1131,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t strbuf_detach(&sb, NULL));\n> \t\t}\n> \n> +\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &val) &&\n> +\t\t val)\n> +\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n> +\n> \t\tif (option_required_reference.nr &&\n> \t\t option_optional_reference.nr)\n> \t\t\tdie(_(\"clone --recursive is not compatible with \"\n\nI like the changes you propose here. It also looks to me like you wanted\nto wait for Mahi to send the updates herself, which I appreciate.\n\nMahi's internship ended last week and I believe she told me she's\nplanning to be very busy this week; so if we don't hear from her by this\ntime next week, I'll send a reroll with these changes (unless you want\nto just take them yourself without the list churn).\n\n - Emily\n"},{"id":"433080","messageId":"xmqqk0ki5xwk.fsf@gitster.g","threadId":"56199","inReplyTo":"YRw/8tThN7djNE+E@google.com","subject":"Re: [PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-18T19:57:15Z","receivedAt":"2021-08-18T19:57:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> I like the changes you propose here. It also looks to me like you wanted\n> to wait for Mahi to send the updates herself, which I appreciate.\n>\n> Mahi's internship ended last week and I believe she told me she's\n> planning to be very busy this week; so if we don't hear from her by this\n> time next week, I'll send a reroll with these changes (unless you want\n> to just take them yourself without the list churn).\n\nI'd make a \"fixup!\" out of the message you are responding to, and\nthen queue it on top of what was posted by Mahi.\n\nThen I'll send the fixup to the list as a reply to this message, so\nthat later you and Mahi can ack (or further update) it.\n\nAfter that, I can squash the fixup into the patch and merge it down\n(unless there are objections from other places, that is).\n\nThanks.\n"},{"id":"433081","messageId":"xmqqa6le5x1f.fsf_-_@gitster.g","threadId":"56199","inReplyTo":"xmqqk0ki5xwk.fsf@gitster.g","subject":"[PATCH] fixup! clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-18T20:15:56Z","receivedAt":"2021-08-18T20:16:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Narrow the scope of a temporary variable used only once and\nimmediately die, and rename it to a shorter, throw-away name.\n\nAlso lose a {} around a single statement block.\n\nSigned-off-by: Junio C Hamano \n---\n\n * To be squashed into mk/clone-recurse-submodules topic 1a0e8231\n (clone: set submodule.recurse=true if\n submodule.stickyRecursiveClone enabled, 2021-08-14)\n\n builtin/clone.c | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex a08d901224..9c0c68a8ef 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -986,7 +986,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct remote *remote;\n \tint err = 0, complete_refs_before_fetch = 1;\n \tint submodule_progress;\n-\tint sticky_recursive_clone;\n \n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n@@ -1115,6 +1114,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (option_recurse_submodules.nr > 0) {\n \t\tstruct string_list_item *item;\n \t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tint val;\n \n \t\t/* remove duplicates */\n \t\tstring_list_sort(&option_recurse_submodules);\n@@ -1131,10 +1131,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t strbuf_detach(&sb, NULL));\n \t\t}\n \n-\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &sticky_recursive_clone)\n-\t\t && sticky_recursive_clone) {\n-\t\t string_list_append(&option_config, \"submodule.recurse=true\");\n-\t\t}\n+\t\tif (!git_config_get_bool(\"submodule.stickyRecursiveClone\", &val) &&\n+\t\t val)\n+\t\t\tstring_list_append(&option_config, \"submodule.recurse=true\");\n \n \t\tif (option_required_reference.nr &&\n \t\t option_optional_reference.nr)\n-- \n2.33.0-204-gff69670db4\n\n"},{"id":"433092","messageId":"4103806f-6d88-2756-8853-7e16e56ad425@gmail.com","threadId":"56199","inReplyTo":"pull.1006.v6.git.1628903396783.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-08-18T22:40:58Z","receivedAt":"2021-08-18T22:41:04Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Mahi, Emily and Junio,\n\nLe 2021-08-13 à 21:09, Mahi Kolla via GitGitGadget a écrit :\n> From: Mahi Kolla \n> \n> Based on current experience, when running git clone --recurse-submodules,\n> developers do not expect other commands such as pull or checkout to run\n> recursively into active submodules. However, setting submodule.recurse=true\n> at this step could make for a simpler workflow by eliminating the need for\n> the --recurse-submodules option in subsequent commands. To collect more\n> data on developers' preference in regards to making submodule.recurse=true\n> a default config value in the future, deploy this feature under the opt in\n> submodule.stickyRecursiveClone flag.\n> \n\nAs I mentioned upthread [1], I'm really not sure that we need a new config\nvariable. If people want to have \"--recurse-submodules\" the default behaviour\nfor all commands that know this flag, they can set 'submodule.recurse' in their\n~/.gitconfig, which enables the behaviour for all those commands (except clone\nand ls-files). And orgs shipping Git to their devs can set it in their system\ngitconfig. I've been using this setup for over two years, with almost zero\nadverse effect on repositories that do not contain submodules. The *only* bug that\nI encountered that affects git commands when *no* submodules are at play was:\n\nc56c48dd07 (grep: ignore --recurse-submodules if --no-index is given, 2020-01-30)\n\nI understand that once submodule.recurse is set in the global or system config file,\nthen Git will start recursing in repos that were cloned prior to\nthat config being enabled, as Emily mentions in [2]. Personnally I think it's a positive\npoint. That *might* be a deal-breaker for other people,\nbut in any case it would be good that this alternative - just using 'submodule.recurse'\n- is mentioned in the commit message and that it mentions that caveat, i.e. why we need\na separate config.\n\nLe 2021-08-13 à 16:38, Emily Shaffer a écrit :\n> and apparently\n> 'submodule.recurse=true' has some weird edge cases for commands which\n> are happy to run out-of-repo.\n\nIt would be nice if those known edge cases were documented somewhere (on the list,\non Gitgitgadget's issues list or at https://bugs.chromium.org/p/git). Apart from\nthe 'grep --no-index' glitch that I mentioned above, I did not encounter any\nmyself.\n\n> Signed-off-by: Mahi Kolla \n> ---\n> clone: set submodule.recurse=true if submodule.stickyRecursiveClone\n> enabled\n> \n> Based on current experience, when running git clone\n> --recurse-submodules, developers do not expect other commands such as\n> pull or checkout to run recursively into active submodules. However,\n> setting submodule.recurse=true at this step could make for a simpler\n> workflow by eliminating the need for the --recurse-submodules option in\n> subsequent commands. To collect more data on developers' behavior and\n> preferences when making submodule.recurse=true a default, deploy this\n> feature under the opt in submodule.stickyRecursiveClone flag.\n> \n> Signed-off-by: Mahi Kolla mkolla2@illinois.edu\n\nMahi, you can keep just the \"Since v1\" part in the GGG PR description (and the\nautomatically added CC's). No need to also repeat the commit message.\n\n> \n> Since V1: Made this an opt in feature under a custom\n> submodule.stickyRecursiveClone flag as opposed to feature.experimental.\n> Updated tests to reflect this design change. Also updated commit\n> message. Additionally, I will be contributing from my personal email\n> going forward as opposed to my @google email.\n> \n> cc: Philippe Blain levraiphilippeblain@gmail.com\n\nSmall nit: since you used '/submit' several times on Gitgitgadget, the previous version\nyou sent was actually sent as v5, and this here version is v6. So for next round, you could\nwrite \"Changes since v6\" :)\n\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1006%2F24mahik%2Fmaster-v6\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1006/24mahik/master-v6\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1006\n> \n> Range-diff vs v5:\n> \n> \n> \n> -+test_expect_success 'feature.experimental flag manipulates submodule.recurse value' '\n> ++test_expect_success 'submodule.stickyRecursiveClone flag manipulates submodule.recurse value' '\n> +\n> -+\ttest_config_global feature.experimental true &&\n> ++\ttest_config_global submodule.stickyRecursiveClone true &&\n> +\tgit clone --recurse-submodules parent clone_recurse_true &&\n> +\ttest_cmp_config -C clone_recurse_true true submodule.recurse &&\n> +\n> -+\ttest_config_global feature.experimental false &&\n> ++\ttest_config_global submodule.stickyRecursiveClone false &&\n> +\tgit clone --recurse-submodules parent clone_recurse_false &&\n> +\ttest_expect_code 1 git -C clone_recurse_false config --get submodule.recurse\n> +\n\nOK, so the expectation with 'submodule.stickyRecursiveClone' is that :\n- if it's true, then 'submodule.recurse' is set to true in the clone's local config file.\n That makes sense.\n- if it's false, then 'submodule.recurse' is not set in the clone. This means that\n if 'submodule.recurse' is already set in ~/.gitconfig (or the system config file)\n then the clone will respect the configured value. That also makes sense, I think.\n\n> \n> builtin/clone.c | 6 ++++++\n> t/t5606-clone-options.sh | 12 ++++++++++++\n> 2 files changed, 18 insertions(+)\n\nThe new config variable should be documented in Documentation/config/clone.txt (which\ngets added to text of the git-config(1) man page).\n\n\nCheers,\n\nPhilippe.\n\n[1] https://lore.kernel.org/git/xmqqa6le5x1f.fsf_-_@gitster.g/T/#m1c2e522368ec4c9d458fcf6d83e75afab1632306\n[2] https://lore.kernel.org/git/YRbYWR+X8vSq8CYe@google.com/\n"},{"id":"433137","messageId":"YR6Xr3SMZjlJ1ZaK@google.com","threadId":"56199","inReplyTo":"xmqqa6le5x1f.fsf_-_@gitster.g","subject":"Re: [PATCH] fixup! clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-08-19T17:41:03Z","receivedAt":"2021-08-19T17:41:14Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Aug 18, 2021 at 01:15:56PM -0700, Junio C Hamano wrote:\n> \n> Narrow the scope of a temporary variable used only once and\n> immediately die, and rename it to a shorter, throw-away name.\n> \n> Also lose a {} around a single statement block.\n> \n> Signed-off-by: Junio C Hamano \nAcked-by: Emily Shaffer \n\nThanks.\n"},{"id":"434169","messageId":"YS1Glp/U4WPu6XCp@google.com","threadId":"56199","inReplyTo":"YR6Xr3SMZjlJ1ZaK@google.com","subject":"Re: [PATCH] fixup! clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-08-30T20:59:02Z","receivedAt":"2021-08-30T20:59:12Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Aug 19, 2021 at 10:41:03AM -0700, Emily Shaffer wrote:\n> \n> On Wed, Aug 18, 2021 at 01:15:56PM -0700, Junio C Hamano wrote:\n> > \n> > Narrow the scope of a temporary variable used only once and\n> > immediately die, and rename it to a shorter, throw-away name.\n> > \n> > Also lose a {} around a single statement block.\n> > \n> > Signed-off-by: Junio C Hamano \n> Acked-by: Emily Shaffer \n> \n> Thanks.\n\nHaving not heard from Mahi last week, if you are happy with the\npatch+squash, I think it would be OK to take this without waiting longer\nfor her. Up to you of course.\n\n - Emily\n"},{"id":"434179","messageId":"xmqqzgsy1vas.fsf@gitster.g","threadId":"56199","inReplyTo":"YS1Glp/U4WPu6XCp@google.com","subject":"Re: [PATCH] fixup! clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-30T21:23:23Z","receivedAt":"2021-08-30T21:23:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> On Thu, Aug 19, 2021 at 10:41:03AM -0700, Emily Shaffer wrote:\n>> \n>> On Wed, Aug 18, 2021 at 01:15:56PM -0700, Junio C Hamano wrote:\n>> > \n>> > Narrow the scope of a temporary variable used only once and\n>> > immediately die, and rename it to a shorter, throw-away name.\n>> > \n>> > Also lose a {} around a single statement block.\n>> > \n>> > Signed-off-by: Junio C Hamano \n>> Acked-by: Emily Shaffer \n>> \n>> Thanks.\n>\n> Having not heard from Mahi last week, if you are happy with the\n> patch+squash, I think it would be OK to take this without waiting longer\n> for her. Up to you of course.\n\nThanks for a nudge.\n"}]}