{"thread":{"id":"52598","subject":"[PATCH] clone: teach --single-branch and --branch during --recurse","startedAt":"2020-01-08T23:19:07Z","lastAt":"2020-02-22T20:13:43Z","messageCount":22,"participants":["Emily Shaffer","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"389510","messageId":"20200108231900.192476-1-emilyshaffer@google.com","threadId":"52598","inReplyTo":null,"subject":"[PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-08T23:19:00Z","receivedAt":"2020-01-08T23:19:07Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Previously, performing \"git clone --recurse-submodules --single-branch\"\nresulted in submodules cloning all branches even though the superproject\ncloned only one branch. Pipe --single-branch and its friend, --branch,\nthrough the submodule helper framework to make it to 'clone' later on.\n\nSigned-off-by: Emily Shaffer \n---\nNote that 'branch' was already in use in git-submodules.sh, so\n\"submodule branch\" aka 'sm_branch' was used to disambiguate the two.\n\n Documentation/git-submodule.txt | 6 +++++-\n builtin/clone.c | 6 ++++++\n builtin/submodule--helper.c | 28 +++++++++++++++++++++++++---\n git-submodule.sh | 17 ++++++++++++++++-\n t/t5617-clone-submodules-remote.sh | 26 ++++++++++++++++++++++++--\n 5 files changed, 76 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 22425cbc76..8c516a9670 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -133,7 +133,7 @@ If you really want to remove a submodule from the repository and commit\n that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal\n options.\n \n-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--single-branch] [-b|--branch ] [--] [...]::\n +\n --\n Update the registered submodules to match what the superproject\n@@ -430,6 +430,10 @@ options carefully.\n \tClone new submodules in parallel with as many jobs.\n \tDefaults to the `submodule.fetchJobs` option.\n \n+--single-branch::\n+\tThis option is only valid for the update command.\n+\tClone only one branch during update, HEAD or --branch.\n+\n ...::\n \tPaths to submodule(s). When specified this will restrict the command\n \tto only operate on the submodules found at the specified paths.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 6dee265cc9..293cef8b30 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -808,6 +808,12 @@ static int checkout(int submodule_progress)\n \t\t\targv_array_push(&args, \"--no-fetch\");\n \t\t}\n \n+\t\tif (option_single_branch)\n+\t\t\targv_array_push(&args, \"--single-branch\");\n+\n+\t\tif (option_branch)\n+\t\t\targv_array_pushf(&args, \"--branch=%s\", option_branch);\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c72931ecd7..92bd823d38 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1225,7 +1225,8 @@ static int module_deinit(int argc, const char **argv, const char *prefix)\n \n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t const char *depth, struct string_list *reference, int dissociate,\n-\t\t\t int quiet, int progress)\n+\t\t\t int quiet, int progress, int single_branch,\n+\t\t\t const char *branch)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \n@@ -1247,6 +1248,10 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \t\targv_array_push(&cp.args, \"--dissociate\");\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n+\tif (single_branch)\n+\t\targv_array_push(&cp.args, \"--single-branch\");\n+\tif (branch)\n+\t\targv_array_pushl(&cp.args, \"--branch\", branch, NULL);\n \n \targv_array_push(&cp.args, \"--\");\n \targv_array_push(&cp.args, url);\n@@ -1373,6 +1378,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tint dissociate = 0, require_init = 0;\n \tchar *sm_alternate = NULL, *error_strategy = NULL;\n+\tint single_branch = 0;\n+\tchar *branch = NULL;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n@@ -1400,12 +1407,17 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"force cloning progress\")),\n \t\tOPT_BOOL(0, \"require-init\", &require_init,\n \t\t\t N_(\"disallow cloning into non-empty directory\")),\n+\t\tOPT_BOOL(0, \"single-branch\", &single_branch,\n+\t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n+\t\tOPT_STRING('b', \"branch\", &branch, \"\",\n+\t\t\t N_(\"checkout instead of the remote's HEAD\")),\n \t\tOPT_END()\n \t};\n \n \tconst char *const git_submodule_helper_usage[] = {\n \t\tN_(\"git submodule--helper clone [--prefix=] [--quiet] \"\n \t\t \"[--reference ] [--name ] [--depth ] \"\n+\t\t \"[--single-branch] [-b | --branch ] \"\n \t\t \"--url --path \"),\n \t\tNULL\n \t};\n@@ -1438,7 +1450,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tprepare_possible_alternates(name, &reference);\n \n \t\tif (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,\n-\t\t\t\t quiet, progress))\n+\t\t\t\t quiet, progress, single_branch, branch))\n \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n \t\t\t url, path);\n \t} else {\n@@ -1562,6 +1574,8 @@ struct submodule_update_clone {\n \tconst char *depth;\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n+\tint single_branch;\n+\tconst char *branch;\n \n \t/* to be consumed by git-submodule.sh */\n \tstruct update_clone_data *update_clone;\n@@ -1578,7 +1592,7 @@ struct submodule_update_clone {\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n-\tNULL, NULL, NULL, \\\n+\tNULL, NULL, NULL, 0, NULL,\\\n \tNULL, 0, 0, 0, NULL, 0, 0, 1}\n \n \n@@ -1718,6 +1732,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\targv_array_push(&child->args, \"--dissociate\");\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n+\tif (suc->single_branch)\n+\t\targv_array_push(&child->args, \"--single-branch\");\n+\tif (suc->branch)\n+\t\targv_array_pushl(&child->args, \"--branch\", suc->branch, NULL);\n \n cleanup:\n \tstrbuf_reset(&displaypath_sb);\n@@ -1897,6 +1915,10 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"force cloning progress\")),\n \t\tOPT_BOOL(0, \"require-init\", &suc.require_init,\n \t\t\t N_(\"disallow cloning into non-empty directory\")),\n+\t\tOPT_BOOL(0, \"single-branch\", &suc.single_branch,\n+\t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n+\t\tOPT_STRING('b', \"branch\", &suc.branch, \"\",\n+\t\t\t N_(\"checkout instead of the remote's HEAD\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex aaa1809d24..c2eadbb930 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -10,7 +10,7 @@ USAGE=\"[--quiet] [--cached]\n or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]\n or: $dashless [--quiet] init [--] [...]\n or: $dashless [--quiet] deinit [-f|--force] (--all| [--] ...)\n- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--] [...]\n+ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--single-branch] [-b|--branch ] [--] [...]\n or: $dashless [--quiet] set-branch (--default|--branch ) [--] \n or: $dashless [--quiet] set-url [--] \n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n@@ -47,6 +47,8 @@ custom_name=\n depth=\n progress=\n dissociate=\n+single_branch=\n+sm_branch=\n \n die_if_unmatched ()\n {\n@@ -526,6 +528,17 @@ cmd_update()\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n+\t\t--single-branch)\n+\t\t\tsingle_branch=1\n+\t\t\t;;\n+\t\t-b|--branch)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tsm_branch=\"--branch=$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t-b=*|--branch=*)\n+\t\t\tsm_branch=$1\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -555,6 +568,8 @@ cmd_update()\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${require_init:+--require-init} \\\n+\t\t${single_branch:+--single-branch} \\\n+\t\t${sm_branch:+\"$sm_branch\"} \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n \t\t-- \\\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nindex 37fcce9c40..390645897d 100755\n--- a/t/t5617-clone-submodules-remote.sh\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -14,14 +14,16 @@ test_expect_success 'setup' '\n \t\tcd sub &&\n \t\tgit init &&\n \t\ttest_commit subcommit1 &&\n-\t\tgit tag sub_when_added_to_super\n+\t\tgit tag sub_when_added_to_super &&\n+\t\tgit branch other\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+\t) &&\n+\tgit branch other\n '\n \n test_expect_success 'clone with --no-remote-submodules' '\n@@ -51,4 +53,24 @@ test_expect_success 'check the default is --no-remote-submodules' '\n \t)\n '\n \n+test_expect_success 'clone with --single-branch' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --single-branch \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit branch -a >branches &&\n+\t\ttest_must_fail grep other branches\n+\t)\n+'\n+\n+test_expect_success 'clone with --single-branch and --branch' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --single-branch --branch other \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit branch -a >branches &&\n+\t\ttest_must_fail grep master branches\n+\t)\n+'\n+\n test_done\n-- \n2.25.0.rc1.283.g88dfdc4193-goog\n\n"},{"id":"389511","messageId":"20200108233905.GE181522@google.com","threadId":"52598","inReplyTo":"20200108231900.192476-1-emilyshaffer@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-08T23:39:05Z","receivedAt":"2020-01-08T23:39:12Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Jan 08, 2020 at 03:19:00PM -0800, Emily Shaffer wrote:\n\nBah, in my attempt to keep the subject brief I was aiming to write\n\"--recursive\" but instead just wrote \"--recurse\" which is wrong. I can\npush another version with the fix if desired.\n\n> Previously, performing \"git clone --recurse-submodules --single-branch\"\n> resulted in submodules cloning all branches even though the superproject\n> cloned only one branch. Pipe --single-branch and its friend, --branch,\n> through the submodule helper framework to make it to 'clone' later on.\n> \n> Signed-off-by: Emily Shaffer \n> ---\n> Note that 'branch' was already in use in git-submodules.sh, so\n> \"submodule branch\" aka 'sm_branch' was used to disambiguate the two.\n> \n> Documentation/git-submodule.txt | 6 +++++-\n> builtin/clone.c | 6 ++++++\n> builtin/submodule--helper.c | 28 +++++++++++++++++++++++++---\n> git-submodule.sh | 17 ++++++++++++++++-\n> t/t5617-clone-submodules-remote.sh | 26 ++++++++++++++++++++++++--\n> 5 files changed, 76 insertions(+), 7 deletions(-)\n"},{"id":"389521","messageId":"20200109081150.GC3978837@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200108231900.192476-1-emilyshaffer@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-09T08:11:50Z","receivedAt":"2020-01-09T08:11:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 08, 2020 at 03:19:00PM -0800, Emily Shaffer wrote:\n\n> Subject: clone: teach --single-branch and --branch during --recurse\n\nA minor nit, but this subject confused me for a moment. I think we'd\nusually say \"teach\" for a new feature being implemented, and this is\nreally just about passing along the existing features. Something like:\n\n clone: pass --single-branch and --branch when recursing submodules\n\nwould have been a bit more obvious (to me anyway).\n\n> Previously, performing \"git clone --recurse-submodules --single-branch\"\n> resulted in submodules cloning all branches even though the superproject\n> cloned only one branch. Pipe --single-branch and its friend, --branch,\n> through the submodule helper framework to make it to 'clone' later on.\n\nSince I don't really use submodules, I don't have much data or even\nintuition to go on. But could this be a regression for some situations?\nE.g., imagine I have a repo \"parent\" whose branch \"foo\" has a submodule\n\"child\", but \"child\" only has a branch \"bar\". What happens now if I \"git\nclone --recurse-submodules --single-branch -b foo parent\", and what will\nhappen after this patch?\n\nI think it works before, but doesn't now.\n\nSetting up like this:\n\n git init child\n (\n \tcd child\n \tgit checkout -b bar\n \techo whatever >file\n \tgit add file\n \tgit commit -m 'child commit'\n )\n \n git init parent\n cd parent\n git checkout -b foo\n git submodule add $PWD/../child\n git commit -m 'add submodule'\n\nif I use the current tip of master, I get:\n\n $ git clone --recurse-submodules --single-branch -b foo parent cur\n Cloning into 'cur'...\n done.\n Submodule 'child' (/home/peff/tmp/parent/../child) registered for path 'child'\n Cloning into '/home/peff/tmp/cur/child'...\n done.\n Submodule path 'child': checked out 'b5cbfcc9fec3b7d67e305468624fed2ba1aa4758'\n\n $ git -C cur/child log -1 --oneline | cat\n b5cbfcc child commit\n\nwith your patch, I get:\n\n $ git.compile clone --recurse-submodules --single-branch -b foo parent new\n Cloning into 'new'...\n done.\n Submodule 'child' (/home/peff/tmp/parent/../child) registered for path 'child'\n Cloning into '/home/peff/tmp/new/child'...\n warning: Could not find remote branch foo to clone.\n fatal: Remote branch foo not found in upstream origin\n fatal: clone of '/home/peff/tmp/parent/../child' into submodule path '/home/peff/tmp/new/child' failed\n Failed to clone 'child'. Retry scheduled\n Cloning into '/home/peff/tmp/new/child'...\n warning: Could not find remote branch foo to clone.\n fatal: Remote branch foo not found in upstream origin\n fatal: clone of '/home/peff/tmp/parent/../child' into submodule path '/home/peff/tmp/new/child' failed\n Failed to clone 'child' a second time, aborting\n\n $ git -C new/child log -1 --oneline | cat\n 11acb3a add submodule\n\n(there's nothing checked out in the submodule).\n\nI have no idea how common this kind of thing would be, and I expect in\nmost cases your patch would do what people want. But we might need to be\nbetter about retrying without those options when the first clone fails.\n\n-Peff\n"},{"id":"389947","messageId":"20200116223800.GM181522@google.com","threadId":"52598","inReplyTo":"20200109081150.GC3978837@coredump.intra.peff.net","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-16T22:38:00Z","receivedAt":"2020-01-16T22:38:07Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jan 09, 2020 at 03:11:50AM -0500, Jeff King wrote:\n> On Wed, Jan 08, 2020 at 03:19:00PM -0800, Emily Shaffer wrote:\n> \n> > Subject: clone: teach --single-branch and --branch during --recurse\n> \n> A minor nit, but this subject confused me for a moment. I think we'd\n> usually say \"teach\" for a new feature being implemented, and this is\n> really just about passing along the existing features. Something like:\n> \n> clone: pass --single-branch and --branch when recursing submodules\n> \n> would have been a bit more obvious (to me anyway).\n> \n> > Previously, performing \"git clone --recurse-submodules --single-branch\"\n> > resulted in submodules cloning all branches even though the superproject\n> > cloned only one branch. Pipe --single-branch and its friend, --branch,\n> > through the submodule helper framework to make it to 'clone' later on.\n> \n> Since I don't really use submodules, I don't have much data or even\n> intuition to go on. But could this be a regression for some situations?\n> E.g., imagine I have a repo \"parent\" whose branch \"foo\" has a submodule\n> \"child\", but \"child\" only has a branch \"bar\". What happens now if I \"git\n> clone --recurse-submodules --single-branch -b foo parent\", and what will\n> happen after this patch?\n> \n> I think it works before, but doesn't now.\n> \n> Setting up like this:\n> \n> git init child\n> (\n> \tcd child\n> \tgit checkout -b bar\n> \techo whatever >file\n> \tgit add file\n> \tgit commit -m 'child commit'\n> )\n> \n> git init parent\n> cd parent\n> git checkout -b foo\n> git submodule add $PWD/../child\n> git commit -m 'add submodule'\n> \n> if I use the current tip of master, I get:\n> \n> $ git clone --recurse-submodules --single-branch -b foo parent cur\n> Cloning into 'cur'...\n> done.\n> Submodule 'child' (/home/peff/tmp/parent/../child) registered for path 'child'\n> Cloning into '/home/peff/tmp/cur/child'...\n> done.\n> Submodule path 'child': checked out 'b5cbfcc9fec3b7d67e305468624fed2ba1aa4758'\n> \n> $ git -C cur/child log -1 --oneline | cat\n> b5cbfcc child commit\n> \n> with your patch, I get:\n> \n> $ git.compile clone --recurse-submodules --single-branch -b foo parent new\n> Cloning into 'new'...\n> done.\n> Submodule 'child' (/home/peff/tmp/parent/../child) registered for path 'child'\n> Cloning into '/home/peff/tmp/new/child'...\n> warning: Could not find remote branch foo to clone.\n> fatal: Remote branch foo not found in upstream origin\n> fatal: clone of '/home/peff/tmp/parent/../child' into submodule path '/home/peff/tmp/new/child' failed\n> Failed to clone 'child'. Retry scheduled\n> Cloning into '/home/peff/tmp/new/child'...\n> warning: Could not find remote branch foo to clone.\n> fatal: Remote branch foo not found in upstream origin\n> fatal: clone of '/home/peff/tmp/parent/../child' into submodule path '/home/peff/tmp/new/child' failed\n> Failed to clone 'child' a second time, aborting\n> \n> $ git -C new/child log -1 --oneline | cat\n> 11acb3a add submodule\n> \n> (there's nothing checked out in the submodule).\n> \n> I have no idea how common this kind of thing would be, and I expect in\n> most cases your patch would do what people want. But we might need to be\n> better about retrying without those options when the first clone fails.\n\nYeah, that's interesting. A retry sounds like a pretty solid approach,\nalthough if someone's being cautious and using --single-branch I wonder\nif that's really something they want (since that's avoiding grabbing\nextraneous branches).\n\nI suppose at the very least, --single-branch without --branch should\nbecome recursive. Whether --branch should become recursive, I'm not\ntotally sure.\n\nThe two scenarios I see in conflict are this:\n- Superproject branch \"foo\"\n - submodule A branch \"foo\"\n - submodule B branch \"foo\"\nand\n- Superproject branch \"foo\"\n - submodule A branch \"bar\"\n - submodule B branch \"baz\"\n\nIf we propagate --branch, the first scenario Just Works, and the second\nscenario requires something like:\n\n git clone --recurse-submodules=no --single-branch --branch foo https://superproject\n git submodule update --init --single-branch --branch bar A/\n git submodule update --init --single-branch --branch baz B/\n\n(I guess if the superproject knows what branch it wants for all the submodules,\nyou could also just do \"git submodule update --init --single-branch\" and\nhave it go and ask for all of them.)\n\nIf we don't propagate --branch, the second scenario Just Works, and the\nfirst scenario requires something like:\n\n git clone --recurse-submodules=no --single-branch --branch foo https://superproject\n git submodule update --init --single-branch --branch foo\n\n(That is, I think as long as 'update' takes --branch, even if it's not\npassed along by 'clone', it should still work OK when delegating to\neveryone.)\n\nLet me know if I misunderstood what you were worried about.\n\nI don't use submodules heavily for myself either. I'll try and ask\naround a little to see what folks want, at least here. The ergonomics\nseem pretty similar, so I guess it comes down to having explicit\ndocumentation.\n\n> \n> -Peff\n"},{"id":"390019","messageId":"20200117210319.GA15460@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200116223800.GM181522@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-17T21:03:19Z","receivedAt":"2020-01-17T21:03:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 16, 2020 at 02:38:00PM -0800, Emily Shaffer wrote:\n\n> > I have no idea how common this kind of thing would be, and I expect in\n> > most cases your patch would do what people want. But we might need to be\n> > better about retrying without those options when the first clone fails.\n> \n> Yeah, that's interesting. A retry sounds like a pretty solid approach,\n> although if someone's being cautious and using --single-branch I wonder\n> if that's really something they want (since that's avoiding grabbing\n> extraneous branches).\n> \n> I suppose at the very least, --single-branch without --branch should\n> become recursive. Whether --branch should become recursive, I'm not\n> totally sure.\n\nYeah, I think it may be worth separating out how to think about the two\noptions. It's a lot more plausible to me that --single-branch would want\nto recurse than --branch.\n\n> The two scenarios I see in conflict are this:\n> - Superproject branch \"foo\"\n> - submodule A branch \"foo\"\n> - submodule B branch \"foo\"\n> and\n> - Superproject branch \"foo\"\n> - submodule A branch \"bar\"\n> - submodule B branch \"baz\"\n> \n> If we propagate --branch, the first scenario Just Works, and the second\n> scenario requires something like:\n> \n> git clone --recurse-submodules=no --single-branch --branch foo https://superproject\n> git submodule update --init --single-branch --branch bar A/\n> git submodule update --init --single-branch --branch baz B/\n> \n> (I guess if the superproject knows what branch it wants for all the submodules,\n> you could also just do \"git submodule update --init --single-branch\" and\n> have it go and ask for all of them.)\n> \n> If we don't propagate --branch, the second scenario Just Works, and the\n> first scenario requires something like:\n> \n> git clone --recurse-submodules=no --single-branch --branch foo https://superproject\n> git submodule update --init --single-branch --branch foo\n> \n> (That is, I think as long as 'update' takes --branch, even if it's not\n> passed along by 'clone', it should still work OK when delegating to\n> everyone.)\n> \n> Let me know if I misunderstood what you were worried about.\n\nRight, that's exactly my concern. You're making one case work but\nbreaking the other.\n\nIt really seems like there's no particular reason to assume that the\nsuperproject branch corresponds to the submodule branch (or even\nsubmodules of submodules). I imagine it would in some cases (like trying\nto replace the use of \"repo\" in Android), but that's just one model.\n\nIt would make more sense to me to either (or both):\n\n - make sure that .gitmodules has enough information about which branch\n to use for each submodule\n\n - offer an extra option for the default branch to use for any\n submodules. This is still not general enough to cover all situations\n (e.g., the bar/baz you showed above), but it at least makes it\n relatively easy to cover the simple cases, without breaking any\n existing ones.\n\n-Peff\n"},{"id":"390585","messageId":"20200127222019.GA233139@google.com","threadId":"52598","inReplyTo":"20200117210319.GA15460@coredump.intra.peff.net","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-27T22:20:19Z","receivedAt":"2020-01-27T22:20:27Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Jan 17, 2020 at 04:03:19PM -0500, Jeff King wrote:\n\n> (like trying to replace the use of \"repo\" in Android)\n\nOops, you saw right through us ;)\n\n> It would make more sense to me to either (or both):\n> \n> - make sure that .gitmodules has enough information about which branch\n> to use for each submodule\n\nHum. I don't work with them day to day, but aren't we already in that\nstate? Is that not what the 'branch' option for each submodule means?\n\n> \n> - offer an extra option for the default branch to use for any\n> submodules. This is still not general enough to cover all situations\n> (e.g., the bar/baz you showed above), but it at least makes it\n> relatively easy to cover the simple cases, without breaking any\n> existing ones.\n\nYeah, this is sort of the direction my mind went too - \"not\n--branch recursively, but --submodule-branch\". But that breaks down when you've\ngot a nontrivial number of submodules, at which point you're gonna have\na hard time unless you've got the .gitmodules configured correctly.\n\n\nWell, as for this patch, let me try it with just --single-branch and see\nwhether that works for the case the user reported. I can head back to\nthe drawing board if not.\n\n - Emily\n"},{"id":"390588","messageId":"20200127224914.GB233139@google.com","threadId":"52598","inReplyTo":"20200127222019.GA233139@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-27T22:49:14Z","receivedAt":"2020-01-27T22:49:21Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Jan 27, 2020 at 02:20:19PM -0800, Emily Shaffer wrote:\n> On Fri, Jan 17, 2020 at 04:03:19PM -0500, Jeff King wrote:\n> \n> > (like trying to replace the use of \"repo\" in Android)\n> \n> Oops, you saw right through us ;)\n> \n> > It would make more sense to me to either (or both):\n> > \n> > - make sure that .gitmodules has enough information about which branch\n> > to use for each submodule\n> \n> Hum. I don't work with them day to day, but aren't we already in that\n> state? Is that not what the 'branch' option for each submodule means?\n\nI've been corrected off-list that the 'branch' in .gitmodules is used\nduring 'git submodule update --remote', but not during 'git submodule\ninit' or 'git clone --recurse-submodules'. Then, for the problem in\ndiscussion for this thread, it seems like a better choice is something\nlike 'git clone --recurse-submdoules --use-gitmodules' or whatever we\nwant to call it - e.g., rather than fetching the branch where the server\nknows HEAD, ask the .gitmodules to figure out which branch?\n\nIt seems like that ought to live separately from --single-branch. In the\ncase where you very strictly only want to fetch one branch (not two\nbranches) I suppose you'd want something like 'git clone\n--recurse-submodules --single-branch --branch=mysuperprojectbranch\n--use-gitmodules' to make sure that only one branch per repo comes down.\n\nWith n submodules of various naming schemas, provenance, etc., I don't\nthink there's a good case for recursing --branch one way or another; it\nseems like filling out some config is the way to go.\n\nI guess we could also teach it to take some input like\n--submodule-branch-spec=foo.txt, and/or a multiply provided\n--submodule-branch foo=foobranch --submodule-branch bar/baz=bazbranch.\n\n [foo.txt]\n foo=foobranch\n bar/baz=bazbranch\n\nWith that approach, then someone gets a little more flexibility than\nrelying on what the .gitmodules has set up.\n\n> > - offer an extra option for the default branch to use for any\n> > submodules. This is still not general enough to cover all situations\n> > (e.g., the bar/baz you showed above), but it at least makes it\n> > relatively easy to cover the simple cases, without breaking any\n> > existing ones.\n> \n> Yeah, this is sort of the direction my mind went too - \"not\n> --branch recursively, but --submodule-branch\". But that breaks down when you've\n> got a nontrivial number of submodules, at which point you're gonna have\n> a hard time unless you've got the .gitmodules configured correctly.\n> \n> \n> Well, as for this patch, let me try it with just --single-branch and see\n> whether that works for the case the user reported. I can head back to\n> the drawing board if not.\n\nWith only half the rework of my patch done, I'm starting to convince\nmyself it's not actually going to work :) Well, I'll still try and see.\n\n - Emily\n"},{"id":"390590","messageId":"20200127230039.GA19360@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200127222019.GA233139@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-27T23:00:39Z","receivedAt":"2020-01-27T23:00:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 27, 2020 at 02:20:19PM -0800, Emily Shaffer wrote:\n\n> > It would make more sense to me to either (or both):\n> > \n> > - make sure that .gitmodules has enough information about which branch\n> > to use for each submodule\n> \n> Hum. I don't work with them day to day, but aren't we already in that\n> state? Is that not what the 'branch' option for each submodule means?\n\nProbably? :) I've never used the feature myself, but it does seem like\nthe right thing (and should definitely take precedence over any \"-b\"\noption passed to the superproject).\n\n> > - offer an extra option for the default branch to use for any\n> > submodules. This is still not general enough to cover all situations\n> > (e.g., the bar/baz you showed above), but it at least makes it\n> > relatively easy to cover the simple cases, without breaking any\n> > existing ones.\n> \n> Yeah, this is sort of the direction my mind went too - \"not\n> --branch recursively, but --submodule-branch\". But that breaks down when you've\n> got a nontrivial number of submodules, at which point you're gonna have\n> a hard time unless you've got the .gitmodules configured correctly.\n\nRight. Probably the right answer for that bar/baz case is to complain to\nthe superproject owner that they didn't put branch fields into their\n.gitmodules. So...\n\n> Well, as for this patch, let me try it with just --single-branch and see\n> whether that works for the case the user reported. I can head back to\n> the drawing board if not.\n\nYeah, that makes perfect sense to me.\n\n-Peff\n"},{"id":"390592","messageId":"20200127231007.GB19360@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200127224914.GB233139@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-27T23:10:07Z","receivedAt":"2020-01-27T23:10:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 27, 2020 at 02:49:14PM -0800, Emily Shaffer wrote:\n\n> > > - make sure that .gitmodules has enough information about which branch\n> > > to use for each submodule\n> > \n> > Hum. I don't work with them day to day, but aren't we already in that\n> > state? Is that not what the 'branch' option for each submodule means?\n> \n> I've been corrected off-list that the 'branch' in .gitmodules is used\n> during 'git submodule update --remote', but not during 'git submodule\n> init' or 'git clone --recurse-submodules'. Then, for the problem in\n> discussion for this thread, it seems like a better choice is something\n> like 'git clone --recurse-submdoules --use-gitmodules' or whatever we\n> want to call it - e.g., rather than fetching the branch where the server\n> knows HEAD, ask the .gitmodules to figure out which branch?\n\nOof, I should have read this message before responding to the other one. ;)\n\n> It seems like that ought to live separately from --single-branch. In the\n> case where you very strictly only want to fetch one branch (not two\n> branches) I suppose you'd want something like 'git clone\n> --recurse-submodules --single-branch --branch=mysuperprojectbranch\n> --use-gitmodules' to make sure that only one branch per repo comes down.\n> \n> With n submodules of various naming schemas, provenance, etc., I don't\n> think there's a good case for recursing --branch one way or another; it\n> seems like filling out some config is the way to go.\n\nYeah, I do still think that it makes sense for clone to pass along\n--single-branch, regardless, and then deal with branch selection problem\nseparately on top.\n\n> I guess we could also teach it to take some input like\n> --submodule-branch-spec=foo.txt, and/or a multiply provided\n> --submodule-branch foo=foobranch --submodule-branch bar/baz=bazbranch.\n> \n> [foo.txt]\n> foo=foobranch\n> bar/baz=bazbranch\n> \n> With that approach, then someone gets a little more flexibility than\n> relying on what the .gitmodules has set up.\n\nYeah, I agree that the most general form is being able to specify the\nmapping for each individually. At first I wondered why you'd ever _not_\nwant to just use the branches specified in .gitmodules. But I suppose\nthat gets embedded in the superproject history, which gets awkward as\nthose commits move between branches. E.g., for an android-like project,\nyou don't want to make a commit that says \"use branch devel for all\nsubmodules\" on the devel branch of your superproject. Eventually that\nwill get merged to master, and you'd have to remember to switch it back\nto \"master\".\n\nSo for the simple case, you probably do want to be able to say \"use this\nbranch for cloning all submodules\".\n\nFor the complex cases, you'd need that full mapping. But I think it may\nbe worth it to punt on that for now. Even if we eventually added such a\nfeature, I think we'd still want the simpler version anyway (because\nwhen you _can_ use it, it's going to be much easier). So there's nothing\nlost by doing the minimal thing now and waiting to see if more complex\nuse cases even show up.\n\nAnother thing occurs to me, though: should the binding of this submodule\ndefault branch be written to disk (e.g., a config option)? I'm thinking\nspecifically that if you do:\n\n git clone --submodule-branch=devel -b devel superproject\n\nand then later, you \"git fetch\" and find that somebody has added a new\nsubmodule, you'd presumably want the devel branch of that, too?\n\n-Peff\n"},{"id":"390600","messageId":"20200128010841.GD233139@google.com","threadId":"52598","inReplyTo":"20200127231007.GB19360@coredump.intra.peff.net","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-28T01:08:41Z","receivedAt":"2020-01-28T01:08:49Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Jan 27, 2020 at 06:10:07PM -0500, Jeff King wrote:\n> On Mon, Jan 27, 2020 at 02:49:14PM -0800, Emily Shaffer wrote:\n> \n> > > > - make sure that .gitmodules has enough information about which branch\n> > > > to use for each submodule\n> > > \n> > > Hum. I don't work with them day to day, but aren't we already in that\n> > > state? Is that not what the 'branch' option for each submodule means?\n> > \n> > I've been corrected off-list that the 'branch' in .gitmodules is used\n> > during 'git submodule update --remote', but not during 'git submodule\n> > init' or 'git clone --recurse-submodules'. Then, for the problem in\n> > discussion for this thread, it seems like a better choice is something\n> > like 'git clone --recurse-submdoules --use-gitmodules' or whatever we\n> > want to call it - e.g., rather than fetching the branch where the server\n> > knows HEAD, ask the .gitmodules to figure out which branch?\n> \n> Oof, I should have read this message before responding to the other one. ;)\n> \n> > It seems like that ought to live separately from --single-branch. In the\n> > case where you very strictly only want to fetch one branch (not two\n> > branches) I suppose you'd want something like 'git clone\n> > --recurse-submodules --single-branch --branch=mysuperprojectbranch\n> > --use-gitmodules' to make sure that only one branch per repo comes down.\n> > \n> > With n submodules of various naming schemas, provenance, etc., I don't\n> > think there's a good case for recursing --branch one way or another; it\n> > seems like filling out some config is the way to go.\n> \n> Yeah, I do still think that it makes sense for clone to pass along\n> --single-branch, regardless, and then deal with branch selection problem\n> separately on top.\n\nSure; I've got that ready to send shortly. It seems to grab HEAD of the\nremote for each submodule, and then checkout the specific commit ID the\nsuperproject wants - in my test case, that commit ID was a direct\nancestor of 'master', so the single branch only got 'master'. I'm not\nsure how it would work with a commit ID which doesn't exist in the\nsingle branch that was fetched; I'll write a test and have a look.\n\n> \n> > I guess we could also teach it to take some input like\n> > --submodule-branch-spec=foo.txt, and/or a multiply provided\n> > --submodule-branch foo=foobranch --submodule-branch bar/baz=bazbranch.\n> > \n> > [foo.txt]\n> > foo=foobranch\n> > bar/baz=bazbranch\n> > \n> > With that approach, then someone gets a little more flexibility than\n> > relying on what the .gitmodules has set up.\n> \n> Yeah, I agree that the most general form is being able to specify the\n> mapping for each individually. At first I wondered why you'd ever _not_\n> want to just use the branches specified in .gitmodules. But I suppose\n> that gets embedded in the superproject history, which gets awkward as\n> those commits move between branches. E.g., for an android-like project,\n> you don't want to make a commit that says \"use branch devel for all\n> submodules\" on the devel branch of your superproject. Eventually that\n> will get merged to master, and you'd have to remember to switch it back\n> to \"master\".\n\nYeah, or I suppose I might be doing something weird, like wanting to run\nintegration tests for the whole project on changes in just one\nsubmodule, or something.\n\n> So for the simple case, you probably do want to be able to say \"use this\n> branch for cloning all submodules\".\n\nI think it still makes sense to call this out explicitly, yes? Or do you\nthink that should just be the default?\n\n> \n> For the complex cases, you'd need that full mapping. But I think it may\n> be worth it to punt on that for now. Even if we eventually added such a\n> feature, I think we'd still want the simpler version anyway (because\n> when you _can_ use it, it's going to be much easier). So there's nothing\n> lost by doing the minimal thing now and waiting to see if more complex\n> use cases even show up.\n> \n> Another thing occurs to me, though: should the binding of this submodule\n> default branch be written to disk (e.g., a config option)? I'm thinking\n> specifically that if you do:\n> \n> git clone --submodule-branch=devel -b devel superproject\n> \n> and then later, you \"git fetch\" and find that somebody has added a new\n> submodule, you'd presumably want the devel branch of that, too?\n\nThis made me think - I wonder if it makes sense to take\n--submodule-branch as a wildcarded spec instead. So in your case, I\ncould say,\n\n git clone --submodule-branch *=devel -b devel superproject\n\nAnd then I don't need to do anything differently for 'git fetch' later.\nThis also opens the door for some repos getting special treatment:\n\n git clone --submodule-branch-file=foo.txt -b dev example\n\n foo.txt:\n curl=stable-1.2.3\n nlohmann=v2.28\n example-*=dev\n *=master\n\n(specifying specific versions for some source dependencies, dev branches\nfor submodules which are associated with with 'example' superproject and\nmight be getting active development, and a wild guess for everything\nelse)\n\nI think that also tends to match the glob-expansion configs we use for\nother things. One thing sticking out to me about the idea of providing\n--submodule-branch is that you need to know what's in the repo before\nyou clone it the first time, which being able to use globbing like this\nkind of helps with. But then, I suppose if you don't know what you're\nlooking for, you're not also looking for a very precise filter on your\nclone ;)\n\n - Emily\n"},{"id":"390602","messageId":"20200128013139.GA38223@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200128010841.GD233139@google.com","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-28T01:31:39Z","receivedAt":"2020-01-28T01:31:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 27, 2020 at 05:08:41PM -0800, Emily Shaffer wrote:\n\n> > Yeah, I do still think that it makes sense for clone to pass along\n> > --single-branch, regardless, and then deal with branch selection problem\n> > separately on top.\n> \n> Sure; I've got that ready to send shortly. It seems to grab HEAD of the\n> remote for each submodule, and then checkout the specific commit ID the\n> superproject wants - in my test case, that commit ID was a direct\n> ancestor of 'master', so the single branch only got 'master'. I'm not\n> sure how it would work with a commit ID which doesn't exist in the\n> single branch that was fetched; I'll write a test and have a look.\n\nYeah, it's definitely worth exploring how that works. I thought we had\nsome kind of fallback for when we didn't manage to fetch the object. But\nmaybe I am confusing it with the fallback for \"we tried to fetch this\nspecific object, but the other side doesn't allow that, so we grabbed a\nbranch instead\".\n\n> > So for the simple case, you probably do want to be able to say \"use this\n> > branch for cloning all submodules\".\n> \n> I think it still makes sense to call this out explicitly, yes? Or do you\n> think that should just be the default?\n\nYes, I think it should be a separate option from \"--branch\".\n\n> This made me think - I wonder if it makes sense to take\n> --submodule-branch as a wildcarded spec instead. So in your case, I\n> could say,\n> \n> git clone --submodule-branch *=devel -b devel superproject\n> \n> And then I don't need to do anything differently for 'git fetch' later.\n> This also opens the door for some repos getting special treatment:\n> \n> git clone --submodule-branch-file=foo.txt -b dev example\n> \n> foo.txt:\n> curl=stable-1.2.3\n> nlohmann=v2.28\n> example-*=dev\n> *=master\n\nIf we write it all as config, I think things may get simpler. IIRC,\nthere is already submodule.*.foo config in .git/config (that can mirror\nand override what's in .gitmodules).\n\nSo if we had some config option for \"clone this branch for the submodule\ninstead of HEAD\", then that means you can do:\n\n git clone -c submodule.foo.clonehead=devel ...\n\nand the result would be used by the submodule code, but also saved for\nfuture invocations. Likewise, if there's no \"clonehead\" config for a\nparticular submodule, if we fall back to submodule.defaultclonehead,\nthen you could do:\n\n git clone -c submodule.defaultclonehead=devel ...\n\nand it would also be saved as the default for future submodules. And\nall without having to invent a new submodule-branch-file format.\n\nThe name \"clonehead\" isn't great. I'm not sure if this ought to be\nsubmodule.*.branch (since I don't quite know what that's used for). I\nthink you'll have to explore that a bit.\n\n> I think that also tends to match the glob-expansion configs we use for\n> other things. One thing sticking out to me about the idea of providing\n> --submodule-branch is that you need to know what's in the repo before\n> you clone it the first time, which being able to use globbing like this\n> kind of helps with. But then, I suppose if you don't know what you're\n> looking for, you're not also looking for a very precise filter on your\n> clone ;)\n\nYeah; the scheme I outlined above only allow specifying the value for\none submodule, or the fallback default. It wouldn't allow arbitrary\nglobbing. But I also suspect nobody wants that. If you know what the\nsubmodules are, then you can set up config for each. If you don't, then\n\"everything\" is the only glob that makes sense.\n\n-Peff\n"},{"id":"390604","messageId":"20200128021016.GE233139@google.com","threadId":"52598","inReplyTo":"20200128013139.GA38223@coredump.intra.peff.net","subject":"Re: [PATCH] clone: teach --single-branch and --branch during --recurse","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-28T02:10:16Z","receivedAt":"2020-01-28T02:10:24Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Jan 27, 2020 at 08:31:39PM -0500, Jeff King wrote:\n> On Mon, Jan 27, 2020 at 05:08:41PM -0800, Emily Shaffer wrote:\n> \n> > > Yeah, I do still think that it makes sense for clone to pass along\n> > > --single-branch, regardless, and then deal with branch selection problem\n> > > separately on top.\n> > \n> > Sure; I've got that ready to send shortly. It seems to grab HEAD of the\n> > remote for each submodule, and then checkout the specific commit ID the\n> > superproject wants - in my test case, that commit ID was a direct\n> > ancestor of 'master', so the single branch only got 'master'. I'm not\n> > sure how it would work with a commit ID which doesn't exist in the\n> > single branch that was fetched; I'll write a test and have a look.\n> \n> Yeah, it's definitely worth exploring how that works. I thought we had\n> some kind of fallback for when we didn't manage to fetch the object. But\n> maybe I am confusing it with the fallback for \"we tried to fetch this\n> specific object, but the other side doesn't allow that, so we grabbed a\n> branch instead\".\n\nOk, so I gave it a try. Some well-trimmed trace output:\n\n1) git clone --recurse-submodules --single-branch (the branch in\nquestion is remote's HEAD)\n - Normal clone of superproject\n - git submodule--helper update-clone --progress --require-init\n --single-branch --\n - ultimately...\n - git clone --no-checkout --progress --separate-git-dir\n '/.../super_clone/.git/modules/sub'\n --single-branch --\n '/path/to/submodule/source'\n '/path/to/submodule/destination'\n - git checkout -f -q \n\n2) git clone --recurse-submodules --single-branch --branch other \n 'other' points to a commit of 'sub' which is not an ancestor of 'sub''s\n current HEAD.\n - Normal clone of superproject identical to 1)\n - git submodule--helper update-clone --progress --require-init\n --single-branch --\n - ultimately...\n - git clone --no-checkout --progress --separate-git-dir\n '/.../super_clone/.git/modules/sub'\n --single-branch --\n '/path/to/submodule/source'\n '/path/to/submodule/destination'\n - git fetch origin \n - git checkout -f -q \n\nSo, somewhere in the submodule machinery, it looks like we check if we\nhave the commit in question, and if not, we do another fetch. So in this\ncase, we reach to the server twice per submodule.\n\nOn the bright side, it doesn't fall over; on the dim side, I'd think\nwe could ask for this ref up front along with whatever branch HEAD is.\nI thought there was a way we could tell the server we want 'master' as\nwell as '58c34ed'?\n\n> \n> > > So for the simple case, you probably do want to be able to say \"use this\n> > > branch for cloning all submodules\".\n> > \n> > I think it still makes sense to call this out explicitly, yes? Or do you\n> > think that should just be the default?\n> \n> Yes, I think it should be a separate option from \"--branch\".\n> \n> > This made me think - I wonder if it makes sense to take\n> > --submodule-branch as a wildcarded spec instead. So in your case, I\n> > could say,\n> > \n> > git clone --submodule-branch *=devel -b devel superproject\n> > \n> > And then I don't need to do anything differently for 'git fetch' later.\n> > This also opens the door for some repos getting special treatment:\n> > \n> > git clone --submodule-branch-file=foo.txt -b dev example\n> > \n> > foo.txt:\n> > curl=stable-1.2.3\n> > nlohmann=v2.28\n> > example-*=dev\n> > *=master\n> \n> If we write it all as config, I think things may get simpler. IIRC,\n> there is already submodule.*.foo config in .git/config (that can mirror\n> and override what's in .gitmodules).\n\nHm. But at clone time, there is no .git/config yet, which is why I\nproposed a file passed in at the command line. Although it does seem to\nmake sense to write down those preferences in the .git/config after.\n\nI guess you could pass in configs at the command line, though, and then\nyou don't have to massage it to write your config after fetch.\n\n> So if we had some config option for \"clone this branch for the submodule\n> instead of HEAD\", then that means you can do:\n> \n> git clone -c submodule.foo.clonehead=devel ...\n> \n> and the result would be used by the submodule code, but also saved for\n> future invocations. Likewise, if there's no \"clonehead\" config for a\n> particular submodule, if we fall back to submodule.defaultclonehead,\n> then you could do:\n> \n> git clone -c submodule.defaultclonehead=devel ...\n> \n> and it would also be saved as the default for future submodules. And\n> all without having to invent a new submodule-branch-file format.\n> \n> The name \"clonehead\" isn't great.\n\nAu contraire - it might be my new go-to insult. ;)\n\n> I'm not sure if this ought to be submodule.*.branch (since I don't\n> quite know what that's used for). I think you'll have to explore that\n> a bit.\n> \n> > I think that also tends to match the glob-expansion configs we use for\n> > other things. One thing sticking out to me about the idea of providing\n> > --submodule-branch is that you need to know what's in the repo before\n> > you clone it the first time, which being able to use globbing like this\n> > kind of helps with. But then, I suppose if you don't know what you're\n> > looking for, you're not also looking for a very precise filter on your\n> > clone ;)\n> \n> Yeah; the scheme I outlined above only allow specifying the value for\n> one submodule, or the fallback default. It wouldn't allow arbitrary\n> globbing. But I also suspect nobody wants that. If you know what the\n> submodules are, then you can set up config for each. If you don't, then\n> \"everything\" is the only glob that makes sense.\n\nYeah, I suspect you're right and this fancy globbing falls under YAGNI.\n\n - Emily\n"},{"id":"390673","messageId":"20200128221736.9217-1-emilyshaffer@google.com","threadId":"52598","inReplyTo":"20200108231900.192476-1-emilyshaffer@google.com","subject":"[PATCH v2] clone: pass --single-branch during --recurse-submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-28T22:17:36Z","receivedAt":"2020-01-28T22:17:44Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Previously, performing \"git clone --recurse-submodules --single-branch\"\nresulted in submodules cloning all branches even though the superproject\ncloned only one branch. Pipe --single-branch through the submodule\nhelper framework to make it to 'clone' later on.\n\nSigned-off-by: Emily Shaffer \n---\nRemoved references to --branch since v1.\n\nAs discussed in the thread for v1 of this patch, in cases when the\nsubmodule commit referenced by the specified superproject branch isn't\nthe same as the HEAD of the submodule repo known by the server side,\nthis still works in kind of a non-obvious way. In these cases, first we\nfetch the single branch that is the ancestor of the server's HEAD; then\nwe fetch the commit needed by the superproject (and its ancestry). So\nwhile this change prevents us from fetching *all* branches on clone, it\ndoesn't necessarily limit us to a single branch as described.\n\nTo limit submodules to a single branch on clone, we need to know which\nbranch we want for each submodule; unfortunately it seems that just\npropagating --branch doesn't necessarily help as there's not much\nguarantee that the superproject branch name matches the submodule branch\nname, or that all submodule branch names are the same. So, we need to\ngive a little more thought there.\n\n - Emily\n\n Documentation/git-submodule.txt | 6 +++++-\n builtin/clone.c | 3 +++\n builtin/submodule--helper.c | 17 ++++++++++++++---\n git-submodule.sh | 7 ++++++-\n t/t5617-clone-submodules-remote.sh | 13 ++++++++++++-\n 5 files changed, 40 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 5232407f68..10c42b752a 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -133,7 +133,7 @@ If you really want to remove a submodule from the repository and commit\n that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal\n options.\n \n-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--single-branch] [--] [...]::\n +\n --\n Update the registered submodules to match what the superproject\n@@ -430,6 +430,10 @@ options carefully.\n \tClone new submodules in parallel with as many jobs.\n \tDefaults to the `submodule.fetchJobs` option.\n \n+--single-branch::\n+\tThis option is only valid for the update command.\n+\tClone only one branch during update, HEAD or --branch.\n+\n ...::\n \tPaths to submodule(s). When specified this will restrict the command\n \tto only operate on the submodules found at the specified paths.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 0fc89ae2b9..ec749a86c1 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -832,6 +832,9 @@ static int checkout(int submodule_progress)\n \t\t\targv_array_push(&args, \"--no-fetch\");\n \t\t}\n \n+\t\tif (option_single_branch)\n+\t\t\targv_array_push(&args, \"--single-branch\");\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c72931ecd7..50c9e23db1 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1225,7 +1225,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)\n \n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t const char *depth, struct string_list *reference, int dissociate,\n-\t\t\t int quiet, int progress)\n+\t\t\t int quiet, int progress, int single_branch)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \n@@ -1247,6 +1247,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \t\targv_array_push(&cp.args, \"--dissociate\");\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n+\tif (single_branch)\n+\t\targv_array_push(&cp.args, \"--single-branch\");\n \n \targv_array_push(&cp.args, \"--\");\n \targv_array_push(&cp.args, url);\n@@ -1373,6 +1375,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tint dissociate = 0, require_init = 0;\n \tchar *sm_alternate = NULL, *error_strategy = NULL;\n+\tint single_branch = 0;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n@@ -1400,12 +1403,15 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"force cloning progress\")),\n \t\tOPT_BOOL(0, \"require-init\", &require_init,\n \t\t\t N_(\"disallow cloning into non-empty directory\")),\n+\t\tOPT_BOOL(0, \"single-branch\", &single_branch,\n+\t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n \t\tOPT_END()\n \t};\n \n \tconst char *const git_submodule_helper_usage[] = {\n \t\tN_(\"git submodule--helper clone [--prefix=] [--quiet] \"\n \t\t \"[--reference ] [--name ] [--depth ] \"\n+\t\t \"[--single-branch] \"\n \t\t \"--url --path \"),\n \t\tNULL\n \t};\n@@ -1438,7 +1444,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tprepare_possible_alternates(name, &reference);\n \n \t\tif (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,\n-\t\t\t\t quiet, progress))\n+\t\t\t\t quiet, progress, single_branch))\n \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n \t\t\t url, path);\n \t} else {\n@@ -1562,6 +1568,7 @@ struct submodule_update_clone {\n \tconst char *depth;\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n+\tint single_branch;\n \n \t/* to be consumed by git-submodule.sh */\n \tstruct update_clone_data *update_clone;\n@@ -1578,7 +1585,7 @@ struct submodule_update_clone {\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n-\tNULL, NULL, NULL, \\\n+\tNULL, NULL, NULL, 0,\\\n \tNULL, 0, 0, 0, NULL, 0, 0, 1}\n \n \n@@ -1718,6 +1725,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\targv_array_push(&child->args, \"--dissociate\");\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n+\tif (suc->single_branch)\n+\t\targv_array_push(&child->args, \"--single-branch\");\n \n cleanup:\n \tstrbuf_reset(&displaypath_sb);\n@@ -1897,6 +1906,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"force cloning progress\")),\n \t\tOPT_BOOL(0, \"require-init\", &suc.require_init,\n \t\t\t N_(\"disallow cloning into non-empty directory\")),\n+\t\tOPT_BOOL(0, \"single-branch\", &suc.single_branch,\n+\t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex aaa1809d24..95f73ab775 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -10,7 +10,7 @@ USAGE=\"[--quiet] [--cached]\n or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]\n or: $dashless [--quiet] init [--] [...]\n or: $dashless [--quiet] deinit [-f|--force] (--all| [--] ...)\n- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--] [...]\n+ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--single-branch] [--] [...]\n or: $dashless [--quiet] set-branch (--default|--branch ) [--] \n or: $dashless [--quiet] set-url [--] \n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n@@ -47,6 +47,7 @@ custom_name=\n depth=\n progress=\n dissociate=\n+single_branch=\n \n die_if_unmatched ()\n {\n@@ -526,6 +527,9 @@ cmd_update()\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n+\t\t--single-branch)\n+\t\t\tsingle_branch=1\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -555,6 +559,7 @@ cmd_update()\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${require_init:+--require-init} \\\n+\t\t${single_branch:+--single-branch} \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n \t\t-- \\\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nindex 37fcce9c40..67f2b48659 100755\n--- a/t/t5617-clone-submodules-remote.sh\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -14,7 +14,8 @@ test_expect_success 'setup' '\n \t\tcd sub &&\n \t\tgit init &&\n \t\ttest_commit subcommit1 &&\n-\t\tgit tag sub_when_added_to_super\n+\t\tgit tag sub_when_added_to_super &&\n+\t\tgit branch other\n \t) &&\n \tgit submodule add \"file://$pwd/sub\" sub &&\n \tgit commit -m \"add submodule\" &&\n@@ -51,4 +52,14 @@ test_expect_success 'check the default is --no-remote-submodules' '\n \t)\n '\n \n+test_expect_success 'clone with --single-branch' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --single-branch \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit branch -a >branches &&\n+\t\ttest_must_fail grep other branches\n+\t)\n+'\n+\n test_done\n-- \n2.25.0.341.g760bfbb309-goog\n\n"},{"id":"390780","messageId":"20200130102303.GD840531@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200128221736.9217-1-emilyshaffer@google.com","subject":"Re: [PATCH v2] clone: pass --single-branch during --recurse-submodules","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-30T10:23:03Z","receivedAt":"2020-01-30T10:23:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 28, 2020 at 02:17:36PM -0800, Emily Shaffer wrote:\n\n> Previously, performing \"git clone --recurse-submodules --single-branch\"\n> resulted in submodules cloning all branches even though the superproject\n> cloned only one branch. Pipe --single-branch through the submodule\n> helper framework to make it to 'clone' later on.\n\nThis makes sense to me, bearing in mind that I'm not at all a good\nperson to point out subtleties with submodules that could bite us.\n\n> As discussed in the thread for v1 of this patch, in cases when the\n> submodule commit referenced by the specified superproject branch isn't\n> the same as the HEAD of the submodule repo known by the server side,\n> this still works in kind of a non-obvious way. In these cases, first we\n> fetch the single branch that is the ancestor of the server's HEAD; then\n> we fetch the commit needed by the superproject (and its ancestry). So\n> while this change prevents us from fetching *all* branches on clone, it\n> doesn't necessarily limit us to a single branch as described.\n\nIs it worth adding a test that we do the right thing here? Not so much\nto prove that it works now, but to protect us against future changes. It\nseems like the sort of thing that could get subtly broken.\n\nThe patch looks mostly good to me (my, that was a lot of plumbing\nthrough that option); here are a few minor comments:\n\n> -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n> +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--single-branch] [--] [...]::\n\nThis line is horrendously long. Not new in your patch, but I wonder if\nthe time might have come to break it up.\n\n> +--single-branch::\n> +\tThis option is only valid for the update command.\n> +\tClone only one branch during update, HEAD or --branch.\n\nFor some reason my brain insists on parsing this second sentence as a\n3-item list without an Oxford comma. I wonder if it would be more clear\nas:\n\n Clone only one branch during update: HEAD or one specified by\n --branch.\n\nor similar.\n\n> #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n> \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n> -\tNULL, NULL, NULL, \\\n> +\tNULL, NULL, NULL, 0,\\\n> \tNULL, 0, 0, 0, NULL, 0, 0, 1}\n\nWow. Also not new in your patch, but I think we're moving towards the\nuse of C99 named initializers, which would make this a bit less\ndaunting (all of the NULL/0 items could be omitted!).\n\n> +test_expect_success 'clone with --single-branch' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules --single-branch \"file://$pwd/.\" super_clone &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit branch -a >branches &&\n> +\t\ttest_must_fail grep other branches\n> +\t)\n> +'\n\nDon't use test_must_fail with non-Git commands; you can just say \"! grep\".\n\nWe usually try to avoid scripting around git-branch output (although I\nfind it pretty unlikely that future changes would break this particular\ncase). git-for-each-ref would be a better pick, but I wonder if:\n\n git rev-parse --verify origin/master &&\n test_must_fail git rev-parse --verify origin/other\n\nmight express the expectation more clearly.\n\n-Peff\n"},{"id":"392242","messageId":"20200221025306.GK2447@google.com","threadId":"52598","inReplyTo":"20200130102303.GD840531@coredump.intra.peff.net","subject":"Re: [PATCH v2] clone: pass --single-branch during --recurse-submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-02-21T02:53:06Z","receivedAt":"2020-02-21T02:53:15Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jan 30, 2020 at 05:23:03AM -0500, Jeff King wrote:\n> On Tue, Jan 28, 2020 at 02:17:36PM -0800, Emily Shaffer wrote:\n\nOuch. I forgot about this review for some time. Sorry :)\n\n> \n> > Previously, performing \"git clone --recurse-submodules --single-branch\"\n> > resulted in submodules cloning all branches even though the superproject\n> > cloned only one branch. Pipe --single-branch through the submodule\n> > helper framework to make it to 'clone' later on.\n> \n> This makes sense to me, bearing in mind that I'm not at all a good\n> person to point out subtleties with submodules that could bite us.\n\nI wonder if it makes sense to ship this to our submodule-using masses\nhere for a little while and see how it works / whether anybody yells?\nThis might be too small for that kind of thing.\n\n> \n> > As discussed in the thread for v1 of this patch, in cases when the\n> > submodule commit referenced by the specified superproject branch isn't\n> > the same as the HEAD of the submodule repo known by the server side,\n> > this still works in kind of a non-obvious way. In these cases, first we\n> > fetch the single branch that is the ancestor of the server's HEAD; then\n> > we fetch the commit needed by the superproject (and its ancestry). So\n> > while this change prevents us from fetching *all* branches on clone, it\n> > doesn't necessarily limit us to a single branch as described.\n> \n> Is it worth adding a test that we do the right thing here? Not so much\n> to prove that it works now, but to protect us against future changes. It\n> seems like the sort of thing that could get subtly broken.\n\nWhat did you have in mind beyond the test I added already?\n\n> \n> The patch looks mostly good to me (my, that was a lot of plumbing\n> through that option); here are a few minor comments:\n> \n> > -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n> > +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--single-branch] [--] [...]::\n> \n> This line is horrendously long. Not new in your patch, but I wonder if\n> the time might have come to break it up.\n\nI dug around in Asciidoc doc and couldn't find a good way to do so. The\ntrailing :: means this command listing is done as a \"definition list\",\nand I just didn't see any way to multiline an entry for such a thing. :(\n\n> \n> > +--single-branch::\n> > +\tThis option is only valid for the update command.\n> > +\tClone only one branch during update, HEAD or --branch.\n> \n> For some reason my brain insists on parsing this second sentence as a\n> 3-item list without an Oxford comma. I wonder if it would be more clear\n> as:\n> \n> Clone only one branch during update: HEAD or one specified by\n> --branch.\n> \n> or similar.\n\nTook it verbatim, I agree.\n\n> \n> > #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n> > \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n> > -\tNULL, NULL, NULL, \\\n> > +\tNULL, NULL, NULL, 0,\\\n> > \tNULL, 0, 0, 0, NULL, 0, 0, 1}\n> \n> Wow. Also not new in your patch, but I think we're moving towards the\n> use of C99 named initializers, which would make this a bit less\n> daunting (all of the NULL/0 items could be omitted!).\n\nHrm. Yeah, I'll add a quick patch before this one to do so. It's pretty\ngross :)\n\n> > +test_expect_success 'clone with --single-branch' '\n> > +\ttest_when_finished \"rm -rf super_clone\" &&\n> > +\tgit clone --recurse-submodules --single-branch \"file://$pwd/.\" super_clone &&\n> > +\t(\n> > +\t\tcd super_clone/sub &&\n> > +\t\tgit branch -a >branches &&\n> > +\t\ttest_must_fail grep other branches\n> > +\t)\n> > +'\n> \n> Don't use test_must_fail with non-Git commands; you can just say \"! grep\".\n> \n> We usually try to avoid scripting around git-branch output (although I\n\nooof\n\n> find it pretty unlikely that future changes would break this particular\n> case). git-for-each-ref would be a better pick, but I wonder if:\n> \n> git rev-parse --verify origin/master &&\n> test_must_fail git rev-parse --verify origin/other\n> \n> might express the expectation more clearly.\n\nSure, done.\n\nSorry again for the long wait, and thanks for the effort on the review.\nNew revision coming momentarily.\n\n - Emily\n"},{"id":"392245","messageId":"20200221031027.204486-1-emilyshaffer@google.com","threadId":"52598","inReplyTo":"20200128221736.9217-1-emilyshaffer@google.com","subject":"[PATCH v3 0/2] propagate --single-branch during clone --recurse-submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-02-21T03:10:25Z","receivedAt":"2020-02-21T03:10:39Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Since v2, I replied to Peff's comments, and I also added support to\npropagate --no-single-branch. I'm pretty sure I followed the idiom right\nto to treat options as \"yes, no, unset\" but not sure of the best way to\ntest to feel assured that they're being propagated correctly.\n\n - Emily\n\nEmily Shaffer (2):\n submodule--helper: use C99 named initializer\n clone: pass --single-branch during --recurse-submodules\n\n Documentation/git-submodule.txt | 6 +++++-\n builtin/clone.c | 5 +++++\n builtin/submodule--helper.c | 32 +++++++++++++++++++++++-------\n git-submodule.sh | 10 +++++++++-\n t/t5617-clone-submodules-remote.sh | 13 +++++++++++-\n 5 files changed, 56 insertions(+), 10 deletions(-)\n\n\n-: ---------- > 1: d1d69cc9ce submodule--helper: use C99 named initializer\n1: e064e805da ! 2: adfe55b18f clone: pass --single-branch during --recurse-submodules\n @@ Documentation/git-submodule.txt: If you really want to remove a submodule from t\n options.\n \n -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n -+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--single-branch] [--] [...]::\n ++update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--] [...]::\n +\n --\n Update the registered submodules to match what the superproject\n @@ Documentation/git-submodule.txt: options carefully.\n \tClone new submodules in parallel with as many jobs.\n \tDefaults to the `submodule.fetchJobs` option.\n \n -+--single-branch::\n ++--[no-]single-branch::\n +\tThis option is only valid for the update command.\n -+\tClone only one branch during update, HEAD or --branch.\n ++\tClone only one branch during update: HEAD or one specified by --branch.\n +\n ...::\n \tPaths to submodule(s). When specified this will restrict the command\n @@ builtin/clone.c: static int checkout(int submodule_progress)\n \t\t\targv_array_push(&args, \"--no-fetch\");\n \t\t}\n \n -+\t\tif (option_single_branch)\n -+\t\t\targv_array_push(&args, \"--single-branch\");\n ++\t\tif (option_single_branch >= 0)\n ++\t\t\targv_array_push(&args, option_single_branch ?\n ++\t\t\t\t\t \"--single-branch\" :\n ++\t\t\t\t\t \"--no-single-branch\");\n +\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n @@ builtin/submodule--helper.c: static int clone_submodule(const char *path, const\n \t\targv_array_push(&cp.args, \"--dissociate\");\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n -+\tif (single_branch)\n -+\t\targv_array_push(&cp.args, \"--single-branch\");\n ++\tif (single_branch >= 0)\n ++\t\targv_array_push(&cp.args, single_branch ?\n ++\t\t\t\t\t \"--single-branch\" :\n ++\t\t\t\t\t \"--no-single-branch\");\n \n \targv_array_push(&cp.args, \"--\");\n \targv_array_push(&cp.args, url);\n @@ builtin/submodule--helper.c: static int module_clone(int argc, const char **argv\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tint dissociate = 0, require_init = 0;\n \tchar *sm_alternate = NULL, *error_strategy = NULL;\n -+\tint single_branch = 0;\n ++\tint single_branch = -1;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n @@ builtin/submodule--helper.c: struct submodule_update_clone {\n \t/* to be consumed by git-submodule.sh */\n \tstruct update_clone_data *update_clone;\n @@ builtin/submodule--helper.c: struct submodule_update_clone {\n - };\n - #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n - \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n --\tNULL, NULL, NULL, \\\n -+\tNULL, NULL, NULL, 0,\\\n - \tNULL, 0, 0, 0, NULL, 0, 0, 1}\n - \n - \n + \t.update = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n + \t.recommend_shallow = -1, \\\n + \t.references = STRING_LIST_INIT_DUP, \\\n ++\t.single_branch = -1, \\\n + \t.max_jobs = 1 \\\n + }\n + \n +-\n + static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n + \t\tstruct strbuf *out, const char *displaypath)\n + {\n @@ builtin/submodule--helper.c: static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\targv_array_push(&child->args, \"--dissociate\");\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n -+\tif (suc->single_branch)\n -+\t\targv_array_push(&child->args, \"--single-branch\");\n ++\tif (suc->single_branch >= 0)\n ++\t\targv_array_push(&child->args, suc->single_branch ?\n ++\t\t\t\t\t \"--single-branch\" :\n ++\t\t\t\t\t \"--no-single-branch\");\n \n cleanup:\n \tstrbuf_reset(&displaypath_sb);\n @@ git-submodule.sh: USAGE=\"[--quiet] [--cached]\n or: $dashless [--quiet] init [--] [...]\n or: $dashless [--quiet] deinit [-f|--force] (--all| [--] ...)\n - or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--] [...]\n -+ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--single-branch] [--] [...]\n ++ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--[no-]single-branch] [--] [...]\n or: $dashless [--quiet] set-branch (--default|--branch ) [--] \n or: $dashless [--quiet] set-url [--] \n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n @@ git-submodule.sh: cmd_update()\n \t\t\tjobs=$1\n \t\t\t;;\n +\t\t--single-branch)\n -+\t\t\tsingle_branch=1\n ++\t\t\tsingle_branch=\"--single-branch\"\n ++\t\t\t;;\n ++\t\t--no-single-branch)\n ++\t\t\tsingle_branch=\"--no-single-branch\"\n +\t\t\t;;\n \t\t--)\n \t\t\tshift\n @@ git-submodule.sh: cmd_update()\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${require_init:+--require-init} \\\n -+\t\t${single_branch:+--single-branch} \\\n ++\t\t$single_branch \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n \t\t-- \\\n @@ t/t5617-clone-submodules-remote.sh: test_expect_success 'check the default is --\n +\tgit clone --recurse-submodules --single-branch \"file://$pwd/.\" super_clone &&\n +\t(\n +\t\tcd super_clone/sub &&\n -+\t\tgit branch -a >branches &&\n -+\t\ttest_must_fail grep other branches\n ++\t\tgit rev-parse --verify origin/master &&\n ++\t\ttest_must_fail git rev-parse --verify origin/other\n +\t)\n +'\n +\n\n-- \n2.25.0.265.gbab2e86ba0-goog\n\n"},{"id":"392246","messageId":"20200221031027.204486-2-emilyshaffer@google.com","threadId":"52598","inReplyTo":"20200221031027.204486-1-emilyshaffer@google.com","subject":"[PATCH v3 1/2] submodule--helper: use C99 named initializer","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-02-21T03:10:26Z","receivedAt":"2020-02-21T03:10:43Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Start using a named initializer list for SUBMODULE_UPDATE_CLONE_INIT, as\nthe struct is becoming cumbersome for a typical struct initializer list.\n\nSigned-off-by: Emily Shaffer \n---\n builtin/submodule--helper.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b6e4c79968..1737a91890 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1586,10 +1586,13 @@ struct submodule_update_clone {\n \n \tint max_jobs;\n };\n-#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n-\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n-\tNULL, NULL, NULL, \\\n-\tNULL, 0, 0, 0, NULL, 0, 0, 1}\n+#define SUBMODULE_UPDATE_CLONE_INIT { \\\n+\t.list = MODULE_LIST_INIT, \\\n+\t.update = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n+\t.recommend_shallow = -1, \\\n+\t.references = STRING_LIST_INIT_DUP, \\\n+\t.max_jobs = 1 \\\n+}\n \n \n static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n-- \n2.25.0.265.gbab2e86ba0-goog\n\n"},{"id":"392247","messageId":"20200221031027.204486-3-emilyshaffer@google.com","threadId":"52598","inReplyTo":"20200221031027.204486-1-emilyshaffer@google.com","subject":"[PATCH v3 2/2] clone: pass --single-branch during --recurse-submodules","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-02-21T03:10:27Z","receivedAt":"2020-02-21T03:10:45Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Previously, performing \"git clone --recurse-submodules --single-branch\"\nresulted in submodules cloning all branches even though the superproject\ncloned only one branch. Pipe --single-branch through the submodule\nhelper framework to make it to 'clone' later on.\n\nSigned-off-by: Emily Shaffer \n---\n Documentation/git-submodule.txt | 6 +++++-\n builtin/clone.c | 5 +++++\n builtin/submodule--helper.c | 21 ++++++++++++++++++---\n git-submodule.sh | 10 +++++++++-\n t/t5617-clone-submodules-remote.sh | 13 ++++++++++++-\n 5 files changed, 49 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 218942acd1..c9ed2bf3d5 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -133,7 +133,7 @@ If you really want to remove a submodule from the repository and commit\n that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal\n options.\n \n-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--] [...]::\n +\n --\n Update the registered submodules to match what the superproject\n@@ -430,6 +430,10 @@ options carefully.\n \tClone new submodules in parallel with as many jobs.\n \tDefaults to the `submodule.fetchJobs` option.\n \n+--[no-]single-branch::\n+\tThis option is only valid for the update command.\n+\tClone only one branch during update: HEAD or one specified by --branch.\n+\n ...::\n \tPaths to submodule(s). When specified this will restrict the command\n \tto only operate on the submodules found at the specified paths.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 4f6150c55c..1ad26f4d8c 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -833,6 +833,11 @@ static int checkout(int submodule_progress)\n \t\t\targv_array_push(&args, \"--no-fetch\");\n \t\t}\n \n+\t\tif (option_single_branch >= 0)\n+\t\t\targv_array_push(&args, option_single_branch ?\n+\t\t\t\t\t \"--single-branch\" :\n+\t\t\t\t\t \"--no-single-branch\");\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 1737a91890..11c38369bd 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1235,7 +1235,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)\n \n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t const char *depth, struct string_list *reference, int dissociate,\n-\t\t\t int quiet, int progress)\n+\t\t\t int quiet, int progress, int single_branch)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \n@@ -1257,6 +1257,10 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \t\targv_array_push(&cp.args, \"--dissociate\");\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n+\tif (single_branch >= 0)\n+\t\targv_array_push(&cp.args, single_branch ?\n+\t\t\t\t\t \"--single-branch\" :\n+\t\t\t\t\t \"--no-single-branch\");\n \n \targv_array_push(&cp.args, \"--\");\n \targv_array_push(&cp.args, url);\n@@ -1383,6 +1387,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tint dissociate = 0, require_init = 0;\n \tchar *sm_alternate = NULL, *error_strategy = NULL;\n+\tint single_branch = -1;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n@@ -1410,12 +1415,15 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"force cloning progress\")),\n \t\tOPT_BOOL(0, \"require-init\", &require_init,\n \t\t\t N_(\"disallow cloning into non-empty directory\")),\n+\t\tOPT_BOOL(0, \"single-branch\", &single_branch,\n+\t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n \t\tOPT_END()\n \t};\n \n \tconst char *const git_submodule_helper_usage[] = {\n \t\tN_(\"git submodule--helper clone [--prefix=] [--quiet] \"\n \t\t \"[--reference ] [--name ] [--depth ] \"\n+\t\t \"[--single-branch] \"\n \t\t \"--url --path \"),\n \t\tNULL\n \t};\n@@ -1448,7 +1456,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tprepare_possible_alternates(name, &reference);\n \n \t\tif (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,\n-\t\t\t\t quiet, progress))\n+\t\t\t\t quiet, progress, single_branch))\n \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n \t\t\t url, path);\n \t} else {\n@@ -1572,6 +1580,7 @@ struct submodule_update_clone {\n \tconst char *depth;\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n+\tint single_branch;\n \n \t/* to be consumed by git-submodule.sh */\n \tstruct update_clone_data *update_clone;\n@@ -1591,10 +1600,10 @@ struct submodule_update_clone {\n \t.update = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n \t.recommend_shallow = -1, \\\n \t.references = STRING_LIST_INIT_DUP, \\\n+\t.single_branch = -1, \\\n \t.max_jobs = 1 \\\n }\n \n-\n static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n \t\tstruct strbuf *out, const char *displaypath)\n {\n@@ -1731,6 +1740,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\targv_array_push(&child->args, \"--dissociate\");\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n+\tif (suc->single_branch >= 0)\n+\t\targv_array_push(&child->args, suc->single_branch ?\n+\t\t\t\t\t \"--single-branch\" :\n+\t\t\t\t\t \"--no-single-branch\");\n \n cleanup:\n \tstrbuf_reset(&displaypath_sb);\n@@ -1910,6 +1923,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"force cloning progress\")),\n \t\tOPT_BOOL(0, \"require-init\", &suc.require_init,\n \t\t\t N_(\"disallow cloning into non-empty directory\")),\n+\t\tOPT_BOOL(0, \"single-branch\", &suc.single_branch,\n+\t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex afcb4c0948..89f915cae9 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -10,7 +10,7 @@ USAGE=\"[--quiet] [--cached]\n or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]\n or: $dashless [--quiet] init [--] [...]\n or: $dashless [--quiet] deinit [-f|--force] (--all| [--] ...)\n- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--] [...]\n+ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference ] [--recursive] [--[no-]single-branch] [--] [...]\n or: $dashless [--quiet] set-branch (--default|--branch ) [--] \n or: $dashless [--quiet] set-url [--] \n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n@@ -47,6 +47,7 @@ custom_name=\n depth=\n progress=\n dissociate=\n+single_branch=\n \n die_if_unmatched ()\n {\n@@ -528,6 +529,12 @@ cmd_update()\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n+\t\t--single-branch)\n+\t\t\tsingle_branch=\"--single-branch\"\n+\t\t\t;;\n+\t\t--no-single-branch)\n+\t\t\tsingle_branch=\"--no-single-branch\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -557,6 +564,7 @@ cmd_update()\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${require_init:+--require-init} \\\n+\t\t$single_branch \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n \t\t-- \\\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nindex 37fcce9c40..1a041df10b 100755\n--- a/t/t5617-clone-submodules-remote.sh\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -14,7 +14,8 @@ test_expect_success 'setup' '\n \t\tcd sub &&\n \t\tgit init &&\n \t\ttest_commit subcommit1 &&\n-\t\tgit tag sub_when_added_to_super\n+\t\tgit tag sub_when_added_to_super &&\n+\t\tgit branch other\n \t) &&\n \tgit submodule add \"file://$pwd/sub\" sub &&\n \tgit commit -m \"add submodule\" &&\n@@ -51,4 +52,14 @@ test_expect_success 'check the default is --no-remote-submodules' '\n \t)\n '\n \n+test_expect_success 'clone with --single-branch' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --single-branch \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit rev-parse --verify origin/master &&\n+\t\ttest_must_fail git rev-parse --verify origin/other\n+\t)\n+'\n+\n test_done\n-- \n2.25.0.265.gbab2e86ba0-goog\n\n"},{"id":"392248","messageId":"20200221031651.GA1278019@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200221025306.GK2447@google.com","subject":"Re: [PATCH v2] clone: pass --single-branch during --recurse-submodules","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-21T03:16:51Z","receivedAt":"2020-02-21T03:16:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 20, 2020 at 06:53:06PM -0800, Emily Shaffer wrote:\n\n> On Thu, Jan 30, 2020 at 05:23:03AM -0500, Jeff King wrote:\n> > On Tue, Jan 28, 2020 at 02:17:36PM -0800, Emily Shaffer wrote:\n> \n> Ouch. I forgot about this review for some time. Sorry :)\n\nThat's OK. I forgot about, too. :)\n\n> > This makes sense to me, bearing in mind that I'm not at all a good\n> > person to point out subtleties with submodules that could bite us.\n> \n> I wonder if it makes sense to ship this to our submodule-using masses\n> here for a little while and see how it works / whether anybody yells?\n> This might be too small for that kind of thing.\n\nCertainly putting it in front of more users gives us more confidence.\nBut I have a feeling the gotcha here would be that it does what _some_\nset of users wants, but not so much for others. And your audience may\nnot be a representative sample. So certainly it doesn't hurt, but I'll\nleave it you whether you think it's worthwhile.\n\nI'm having trouble even coming up with a devil's advocate example for\nthe \"others\" in this case, though. Getting general exposure in \"next\"\nand then \"master\" leading up to the release might also help, but I get\nthe impression that those audiences aren't necessarily comprehensive,\neither.\n\n> > > As discussed in the thread for v1 of this patch, in cases when the\n> > > submodule commit referenced by the specified superproject branch isn't\n> > > the same as the HEAD of the submodule repo known by the server side,\n> > > this still works in kind of a non-obvious way. In these cases, first we\n> > > fetch the single branch that is the ancestor of the server's HEAD; then\n> > > we fetch the commit needed by the superproject (and its ancestry). So\n> > > while this change prevents us from fetching *all* branches on clone, it\n> > > doesn't necessarily limit us to a single branch as described.\n> > \n> > Is it worth adding a test that we do the right thing here? Not so much\n> > to prove that it works now, but to protect us against future changes. It\n> > seems like the sort of thing that could get subtly broken.\n> \n> What did you have in mind beyond the test I added already?\n\nI think your test just covers the \"happy path\" that what we want to\nfetch is on the tip of that single branch. But do we test a case where\nthe commit we need for a submodule isn't on the HEAD branch of that\nsubmodule, and we \"clone --single-branch --recurse-submodules\" the\nsuperproject and it works? I.e., from what you describe above it should\nwork, but it might be nice to make sure it continues to do so.\n\n> > > -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]::\n> > > +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--single-branch] [--] [...]::\n> > \n> > This line is horrendously long. Not new in your patch, but I wonder if\n> > the time might have come to break it up.\n> \n> I dug around in Asciidoc doc and couldn't find a good way to do so. The\n> trailing :: means this command listing is done as a \"definition list\",\n> and I just didn't see any way to multiline an entry for such a thing. :(\n\nHmm, yeah. Personally I think it would be better as:\n\n update [options] [--] [...]::\n ...\n The supported options are:\n\n --init::\n ...here's what init does...\n\nbut I guess some of them are repeated between commands, which would need\naddressing. Anyway, it's clear this is way beyond the scope of your\npatch, so let's forget about it for now.\n\n-Peff\n"},{"id":"392252","messageId":"20200221052633.GA1280313@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200221031027.204486-2-emilyshaffer@google.com","subject":"Re: [PATCH v3 1/2] submodule--helper: use C99 named initializer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-21T05:26:33Z","receivedAt":"2020-02-21T05:26:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 20, 2020 at 07:10:26PM -0800, Emily Shaffer wrote:\n\n> Start using a named initializer list for SUBMODULE_UPDATE_CLONE_INIT, as\n> the struct is becoming cumbersome for a typical struct initializer list.\n\nMuch nicer. :) I manually compared the struct declaration to the\nexisting initializer to double-check that you counted the offsets\ncorrectly, and all looks good.\n\n-Peff\n"},{"id":"392253","messageId":"20200221052818.GB1280313@coredump.intra.peff.net","threadId":"52598","inReplyTo":"20200221031027.204486-3-emilyshaffer@google.com","subject":"Re: [PATCH v3 2/2] clone: pass --single-branch during --recurse-submodules","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-21T05:28:18Z","receivedAt":"2020-02-21T05:28:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 20, 2020 at 07:10:27PM -0800, Emily Shaffer wrote:\n\n> Previously, performing \"git clone --recurse-submodules --single-branch\"\n> resulted in submodules cloning all branches even though the superproject\n> cloned only one branch. Pipe --single-branch through the submodule\n> helper framework to make it to 'clone' later on.\n\nThis looks good to me, with the exception of the open question I had\nearlier about testing the case where the needed commit isn't covered by\nthe single branch.\n\n-Peff\n"},{"id":"392309","messageId":"xmqqlfounzwy.fsf@gitster-ct.c.googlers.com","threadId":"52598","inReplyTo":"20200221031027.204486-2-emilyshaffer@google.com","subject":"Re: [PATCH v3 1/2] submodule--helper: use C99 named initializer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-22T20:13:33Z","receivedAt":"2020-02-22T20:13:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> Start using a named initializer list for SUBMODULE_UPDATE_CLONE_INIT, as\n\nThe feature has its own name. Instead of coming up with your own\nname for it, let's say \"designated initializers\".\n\n> -#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n> -\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \\\n> -\tNULL, NULL, NULL, \\\n> -\tNULL, 0, 0, 0, NULL, 0, 0, 1}\n> +#define SUBMODULE_UPDATE_CLONE_INIT { \\\n> +\t.list = MODULE_LIST_INIT, \\\n> +\t.update = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n> +\t.recommend_shallow = -1, \\\n> +\t.references = STRING_LIST_INIT_DUP, \\\n> +\t.max_jobs = 1 \\\n> +}\n\nThis does make it read more easily. If you ended the last field\nwith trailing comma, that would have been even better as a future\nproofing ;-)\n\n"}]}