{"thread":{"id":"51081","subject":"[PATCH] clone: add `--remote-submodules` flag","startedAt":"2019-05-13T18:15:22Z","lastAt":"2019-05-19T17:27:29Z","messageCount":7,"participants":["Ben Avison","Ævar Arnfjörð Bjarmason","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"375414","messageId":"20190513175903.126079-1-bavison@riscosopen.org","threadId":"51081","inReplyTo":null,"subject":"[PATCH] clone: add `--remote-submodules` flag","fromName":"Ben Avison","fromEmail":"bavison@riscosopen.org","sentAt":"2019-05-13T17:59:03Z","receivedAt":"2019-05-13T18:15:22Z","isPatch":true,"sender":{"key":"bavison@riscosopen.org","avatar":"https://avatars.githubusercontent.com/u/3324657?v=4"},"body":"When using `git clone --recurse-submodules` there was previously no way to\npass a `--remote` switch to the implicit `git submodule update` command for\nany use case where you want the submodules to be checked out on their\nremote-tracking branch rather than with the SHA-1 recorded in the superproject.\n\nThis patch rectifies this situation. It actually passes `--no-fetch` to\n`git submodule update` as well on the grounds they the submodule has only just\nbeen cloned, so fetching from the remote again only serves to slow things down.\n\nSigned-off-by: Ben Avison <bavison@riscosopen.org>\n---\n Documentation/git-clone.txt        |  9 ++++-\n builtin/clone.c                    |  8 +++++\n t/t5617-clone-submodules-remote.sh | 54 ++++++++++++++++++++++++++++++\n 3 files changed, 70 insertions(+), 1 deletion(-)\n create mode 100755 t/t5617-clone-submodules-remote.sh\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex a0f14b51f2..5fc97f14de 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -15,7 +15,8 @@ SYNOPSIS\n \t  [--dissociate] [--separate-git-dir <git dir>]\n \t  [--depth <depth>] [--[no-]single-branch] [--no-tags]\n \t  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]\n-\t  [--jobs <n>] [--] <repository> [<directory>]\n+\t  [--[no-]remote-submodules] [--jobs <n>] [--] <repository>\n+\t  [<directory>]\n \n DESCRIPTION\n -----------\n@@ -260,6 +261,12 @@ or `--mirror` is given)\n --[no-]shallow-submodules::\n \tAll submodules which are cloned will be shallow with a depth of 1.\n \n+--[no-]remote-submodules::\n+\tAll submodules which are cloned will use the status of the submodule’s\n+\tremote-tracking branch to update the submodule, rather than the\n+\tsuperproject’s recorded SHA-1. Equivalent to passing `--remote` to\n+\t`git submodule update`.\n+\n --separate-git-dir=<git dir>::\n \tInstead of placing the cloned repository where it is supposed\n \tto be, place the cloned repository at the specified directory,\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 31a47d190a..9e5752ee41 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -67,6 +67,7 @@ static int max_jobs = -1;\n static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\n static struct string_list server_options = STRING_LIST_INIT_NODUP;\n+static int option_remote_submodules;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -145,6 +146,8 @@ static struct option builtin_clone_options[] = {\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV6),\n \tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n+\tOPT_BOOL(0, \"remote-submodules\", &option_remote_submodules,\n+\t\t    N_(\"any cloned submodules will use their remote-tracking branch\")),\n \tOPT_END()\n };\n \n@@ -792,6 +795,11 @@ static int checkout(int submodule_progress)\n \t\tif (option_verbosity < 0)\n \t\t\targv_array_push(&args, \"--quiet\");\n \n+\t\tif (option_remote_submodules == 1) {\n+\t\t\targv_array_push(&args, \"--remote\");\n+\t\t\targv_array_push(&args, \"--no-fetch\");\n+\t\t}\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nnew file mode 100755\nindex 0000000000..3ba9a55341\n--- /dev/null\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -0,0 +1,54 @@\n+#!/bin/sh\n+\n+test_description='Test cloning repos with submodules using remote-tracking branches'\n+\n+. ./test-lib.sh\n+\n+pwd=$(pwd)\n+\n+test_expect_success 'setup' '\n+\tgit checkout -b master &&\n+\ttest_commit commit1 &&\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit init &&\n+\t\ttest_commit subcommit1 &&\n+\t\tgit tag sub_when_added_to_super\n+\t) &&\n+\tgit submodule add \"file://$pwd/sub\" sub &&\n+\tgit commit -m \"add submodule\" &&\n+\t(\n+\t\tcd sub &&\n+\t\ttest_commit subcommit2\n+\t)\n+'\n+\n+test_expect_success 'clone with --no-remote-submodules' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --no-remote-submodules \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit diff --exit-code sub_when_added_to_super\n+\t)\n+'\n+\n+test_expect_success 'clone with --remote-submodules' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --remote-submodules \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit diff --exit-code remotes/origin/master\n+\t)\n+'\n+\n+test_expect_success 'check the default is --no-remote-submodules' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --no-remote-submodules \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit diff --exit-code sub_when_added_to_super\n+\t)\n+'\n+\n+test_done\n-- \n2.21.0.896.g6a6c0f10a7.dirty\n\n"},{"id":"375418","messageId":"87sgtivz2m.fsf@evledraar.gmail.com","threadId":"51081","inReplyTo":"20190513175903.126079-1-bavison@riscosopen.org","subject":"Re: [PATCH] clone: add `--remote-submodules` flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-13T21:12:17Z","receivedAt":"2019-05-13T21:12:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, May 13 2019, Ben Avison wrote:\n\n> @@ -792,6 +795,11 @@ static int checkout(int submodule_progress)\n>  \t\tif (option_verbosity < 0)\n>  \t\t\targv_array_push(&args, \"--quiet\");\n>\n> +\t\tif (option_remote_submodules == 1) {\n\nI see you copied this from code above the context, but to check a bool\nvariable just use \"if (var)\" not \"if (var == 1)\".\n\n> +test_expect_success 'check the default is --no-remote-submodules' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules --no-remote-submodules \"file://$pwd/.\" super_clone &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit diff --exit-code sub_when_added_to_super\n> +\t)\n> +'\n\nThis isn't testing the default, it just tests --no-remote-submodules,\ni.e. if you change the \"static int\" assignment to \"1\" (to make this new\noption the default) all these tests will still pass.\n\nJust fixing that means we finally test for this (the entire test suite\nwould pass before with this on, showing a concerning lack of test\ncoverage in the submodule tests...), but with just\ns/--no-remote-submodules // here we'll test for it.\n\nYou might want to have another test to explicitly check what happens\nwith --no-remote-submodules, but I don't think it's worth the effort, at\nthat point you're just testing the getopt machinery.\n"},{"id":"375564","messageId":"be6f4de9-ee29-f690-3184-1ff7429a0181@riscosopen.org","threadId":"51081","inReplyTo":"87sgtivz2m.fsf@evledraar.gmail.com","subject":"Re: [PATCH] clone: add `--remote-submodules` flag","fromName":"Ben Avison","fromEmail":"bavison@riscosopen.org","sentAt":"2019-05-14T18:38:58Z","receivedAt":"2019-05-14T18:55:15Z","isPatch":true,"sender":{"key":"bavison@riscosopen.org","avatar":"https://avatars.githubusercontent.com/u/3324657?v=4"},"body":"On 13/05/2019 22:12, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, May 13 2019, Ben Avison wrote:\n> \n>> +\t\tif (option_remote_submodules == 1) {\n> \n> I see you copied this from code above the context, but to check a bool\n> variable just use \"if (var)\" not \"if (var == 1)\".\n\nOK. Would you prefer I edit the line above it as well? I simply assumed \nthat was the way it was preferred to perform the test for some reason.\n\n>> +test_expect_success 'check the default is --no-remote-submodules' '\n>> +\ttest_when_finished \"rm -rf super_clone\" &&\n>> +\tgit clone --recurse-submodules --no-remote-submodules \"file://$pwd/.\" super_clone &&\n> \n> This isn't testing the default, it just tests --no-remote-submodules,\n> i.e. if you change the \"static int\" assignment to \"1\" (to make this new\n> option the default) all these tests will still pass.\n\nThat was a cut-and-paste error - I meant to omit both new options as you \nsuggest! Good spot.\n\nBen\n"},{"id":"375716","messageId":"CACsJy8B589nOPNt6143BNQNojRYn4pyQCMNZKLRn+EGyWD4-mw@mail.gmail.com","threadId":"51081","inReplyTo":"20190513175903.126079-1-bavison@riscosopen.org","subject":"Re: [PATCH] clone: add `--remote-submodules` flag","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-16T11:31:53Z","receivedAt":"2019-05-16T11:32:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 14, 2019 at 2:46 AM Ben Avison <bavison@riscosopen.org> wrote:\n>\n> When using `git clone --recurse-submodules` there was previously no way to\n> pass a `--remote` switch to the implicit `git submodule update` command for\n> any use case where you want the submodules to be checked out on their\n> remote-tracking branch rather than with the SHA-1 recorded in the superproject.\n\nAre there any other submodule options that could be useful passing\nfrom git-clone as well? What I'm getting at is, if there are multiple\nof them, perhaps we need a different/generic approach than introducing\na bunch of --<something>-submodules. But of course if --remote is the\nonly useful one left, then it's moot.\n-- \nDuy\n"},{"id":"375730","messageId":"1376dce1-6ca1-88f2-97c9-c8dd2ac683a1@riscosopen.org","threadId":"51081","inReplyTo":"CACsJy8B589nOPNt6143BNQNojRYn4pyQCMNZKLRn+EGyWD4-mw@mail.gmail.com","subject":"Re: [PATCH] clone: add `--remote-submodules` flag","fromName":"Ben Avison","fromEmail":"bavison@riscosopen.org","sentAt":"2019-05-16T17:50:45Z","receivedAt":"2019-05-16T18:07:25Z","isPatch":true,"sender":{"key":"bavison@riscosopen.org","avatar":"https://avatars.githubusercontent.com/u/3324657?v=4"},"body":"On 16/05/2019 12:31, Duy Nguyen wrote:\n> On Tue, May 14, 2019 at 2:46 AM Ben Avison <bavison@riscosopen.org> wrote:\n>>\n>> When using `git clone --recurse-submodules` there was previously no way to\n>> pass a `--remote` switch to the implicit `git submodule update` command for\n>> any use case where you want the submodules to be checked out on their\n>> remote-tracking branch rather than with the SHA-1 recorded in the superproject.\n> \n> Are there any other submodule options that could be useful passing\n> from git-clone as well? What I'm getting at is, if there are multiple\n> of them, perhaps we need a different/generic approach than introducing\n> a bunch of --<something>-submodules. But of course if --remote is the\n> only useful one left, then it's moot.\n\nThat's an interesting point. However, for many of the switches, it only \nmakes sense to set them one way when you're calling `git submodule \nupdate` within `git clone --recurse-submodules`.\n\n\n--quiet: already inherited from `git clone`\n\n--progress: already inherited from `git clone`\n\n--force: wouldn't have any effect since there cannot be any local \nchanges to override yet\n\n--remote: this is what my patch is adding support for\n\n--no-fetch: since any submodules will be freshly cloned, there is no \nneed to fetch from them again already, so you'd always want this on (as \nfar as I'm aware, this only has effect when you also use --remote, so \nI've made it conditional on that)\n\n--checkout/--rebase/--merge: since there cannot be any local changes to \nrebased or merged yet, these wouldn't have any effect, and it's fine to \nleave them as the default (--checkout)\n\n--init: you always want this in this case\n\n--reference/--dissociate: I suppose you might want these in theory? \nHowever, as far as I understand, it's only really useful for `git \nsubmodule update` if the superproject only contains a single submodule, \nsince each submodule would require a different reference repository.\n\n--recursive: at present this is applied unconditionally. I suppose you \nmight only want to recurse to one level, but I'd think that's rare?\n\n--depth: at present you only get to set this to a depth of 1, by passing \n`--shallow-submodules` to `git clone`. I suppose you might occasionally \nwant a different depth, but again I'd expect that to be rare.\n\n--no-recommend-shallow: there may be an argument for letting you set \nthis one, and you can't at present\n\n--jobs: already inherited from `git clone`\n\n\nIn summary, the most significant omission is --remote IMHO, though there \nmay be an argument for adding a small number of others.\n\nBen\n"},{"id":"375848","messageId":"CACsJy8BypLygJGW67mtomL=VLQLVuPSsYTx-3+Dp_Zg-hkSVpA@mail.gmail.com","threadId":"51081","inReplyTo":"1376dce1-6ca1-88f2-97c9-c8dd2ac683a1@riscosopen.org","subject":"Re: [PATCH] clone: add `--remote-submodules` flag","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-18T11:40:55Z","receivedAt":"2019-05-18T11:41:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, May 17, 2019 at 12:50 AM Ben Avison <bavison@riscosopen.org> wrote:\n>\n> On 16/05/2019 12:31, Duy Nguyen wrote:\n> > On Tue, May 14, 2019 at 2:46 AM Ben Avison <bavison@riscosopen.org> wrote:\n> >>\n> >> When using `git clone --recurse-submodules` there was previously no way to\n> >> pass a `--remote` switch to the implicit `git submodule update` command for\n> >> any use case where you want the submodules to be checked out on their\n> >> remote-tracking branch rather than with the SHA-1 recorded in the superproject.\n> >\n> > Are there any other submodule options that could be useful passing\n> > from git-clone as well? What I'm getting at is, if there are multiple\n> > of them, perhaps we need a different/generic approach than introducing\n> > a bunch of --<something>-submodules. But of course if --remote is the\n> > only useful one left, then it's moot.\n>\n> That's an interesting point. However, for many of the switches, it only\n> makes sense to set them one way when you're calling `git submodule\n> update` within `git clone --recurse-submodules`.\n>\n> ...\n>\n>\n> In summary, the most significant omission is --remote IMHO, though there\n> may be an argument for adding a small number of others.\n\nGreat! Thanks for checking.\n-- \nDuy>>\n"},{"id":"375878","messageId":"20190519142649.15600-1-bavison@riscosopen.org","threadId":"51081","inReplyTo":"87sgtivz2m.fsf@evledraar.gmail.com","subject":"[PATCH v2] clone: add `--remote-submodules` flag","fromName":"Ben Avison","fromEmail":"bavison@riscosopen.org","sentAt":"2019-05-19T14:26:49Z","receivedAt":"2019-05-19T17:27:29Z","isPatch":true,"sender":{"key":"bavison@riscosopen.org","avatar":"https://avatars.githubusercontent.com/u/3324657?v=4"},"body":"When using `git clone --recurse-submodules` there was previously no way to\npass a `--remote` switch to the implicit `git submodule update` command for\nany use case where you want the submodules to be checked out on their\nremote-tracking branch rather than with the SHA-1 recorded in the superproject.\n\nThis patch rectifies this situation. It actually passes `--no-fetch` to\n`git submodule update` as well on the grounds they the submodule has only just\nbeen cloned, so fetching from the remote again only serves to slow things down.\n\nSigned-off-by: Ben Avison <bavison@riscosopen.org>\n---\n Documentation/git-clone.txt        |  9 ++++-\n builtin/clone.c                    |  8 +++++\n t/t5617-clone-submodules-remote.sh | 54 ++++++++++++++++++++++++++++++\n 3 files changed, 70 insertions(+), 1 deletion(-)\n create mode 100755 t/t5617-clone-submodules-remote.sh\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex a0f14b51f2..5fc97f14de 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -15,7 +15,8 @@ SYNOPSIS\n \t  [--dissociate] [--separate-git-dir <git dir>]\n \t  [--depth <depth>] [--[no-]single-branch] [--no-tags]\n \t  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]\n-\t  [--jobs <n>] [--] <repository> [<directory>]\n+\t  [--[no-]remote-submodules] [--jobs <n>] [--] <repository>\n+\t  [<directory>]\n \n DESCRIPTION\n -----------\n@@ -260,6 +261,12 @@ or `--mirror` is given)\n --[no-]shallow-submodules::\n \tAll submodules which are cloned will be shallow with a depth of 1.\n \n+--[no-]remote-submodules::\n+\tAll submodules which are cloned will use the status of the submodule’s\n+\tremote-tracking branch to update the submodule, rather than the\n+\tsuperproject’s recorded SHA-1. Equivalent to passing `--remote` to\n+\t`git submodule update`.\n+\n --separate-git-dir=<git dir>::\n \tInstead of placing the cloned repository where it is supposed\n \tto be, place the cloned repository at the specified directory,\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 31a47d190a..b1ae087427 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -67,6 +67,7 @@ static int max_jobs = -1;\n static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\n static struct string_list server_options = STRING_LIST_INIT_NODUP;\n+static int option_remote_submodules;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -145,6 +146,8 @@ static struct option builtin_clone_options[] = {\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV6),\n \tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n+\tOPT_BOOL(0, \"remote-submodules\", &option_remote_submodules,\n+\t\t    N_(\"any cloned submodules will use their remote-tracking branch\")),\n \tOPT_END()\n };\n \n@@ -792,6 +795,11 @@ static int checkout(int submodule_progress)\n \t\tif (option_verbosity < 0)\n \t\t\targv_array_push(&args, \"--quiet\");\n \n+\t\tif (option_remote_submodules) {\n+\t\t\targv_array_push(&args, \"--remote\");\n+\t\t\targv_array_push(&args, \"--no-fetch\");\n+\t\t}\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nnew file mode 100755\nindex 0000000000..37fcce9c40\n--- /dev/null\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -0,0 +1,54 @@\n+#!/bin/sh\n+\n+test_description='Test cloning repos with submodules using remote-tracking branches'\n+\n+. ./test-lib.sh\n+\n+pwd=$(pwd)\n+\n+test_expect_success 'setup' '\n+\tgit checkout -b master &&\n+\ttest_commit commit1 &&\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit init &&\n+\t\ttest_commit subcommit1 &&\n+\t\tgit tag sub_when_added_to_super\n+\t) &&\n+\tgit submodule add \"file://$pwd/sub\" sub &&\n+\tgit commit -m \"add submodule\" &&\n+\t(\n+\t\tcd sub &&\n+\t\ttest_commit subcommit2\n+\t)\n+'\n+\n+test_expect_success 'clone with --no-remote-submodules' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --no-remote-submodules \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit diff --exit-code sub_when_added_to_super\n+\t)\n+'\n+\n+test_expect_success 'clone with --remote-submodules' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --remote-submodules \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit diff --exit-code remotes/origin/master\n+\t)\n+'\n+\n+test_expect_success 'check the default is --no-remote-submodules' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit diff --exit-code sub_when_added_to_super\n+\t)\n+'\n+\n+test_done\n-- \n2.21.0.896.g6a6c0f10a7.dirty\n\n"}]}