{"thread":{"id":"43031","subject":"[PATCHv3 3/9] submodule--helper module-clone: allow multiple references","startedAt":"2016-08-09T04:08:26Z","lastAt":"2016-08-10T20:39:50Z","messageCount":36,"participants":["Stefan Beller","Jacob Keller","Eric Sunshine","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"293440","messageId":"20160809040811.21408-4-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 3/9] submodule--helper module-clone: allow multiple references","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:03Z","receivedAt":"2016-08-09T04:08:26Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Allow users to pass in multiple references, just as clone accepts multiple\nreferences as well.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6f6d67a..ce9b3e9 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -442,7 +442,7 @@ static int module_name(int argc, const char **argv, const char *prefix)\n }\n \n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n-\t\t\t const char *depth, const char *reference, int quiet)\n+\t\t\t const char *depth, struct string_list *reference, int quiet)\n {\n \tstruct child_process cp;\n \tchild_process_init(&cp);\n@@ -453,8 +453,12 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \t\targv_array_push(&cp.args, \"--quiet\");\n \tif (depth && *depth)\n \t\targv_array_pushl(&cp.args, \"--depth\", depth, NULL);\n-\tif (reference && *reference)\n-\t\targv_array_pushl(&cp.args, \"--reference\", reference, NULL);\n+\tif (reference->nr) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, reference)\n+\t\t\targv_array_pushl(&cp.args, \"--reference\",\n+\t\t\t\t\t item->string, NULL);\n+\t}\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n \n@@ -470,13 +474,13 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \n static int module_clone(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *name = NULL, *url = NULL;\n-\tconst char *reference = NULL, *depth = NULL;\n+\tconst char *name = NULL, *url = NULL, *depth = NULL;\n \tint quiet = 0;\n \tFILE *submodule_dot_git;\n \tchar *p, *path = NULL, *sm_gitdir;\n \tstruct strbuf rel_path = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n+\tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n@@ -491,8 +495,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"url\", &url,\n \t\t\t N_(\"string\"),\n \t\t\t N_(\"url where to clone the submodule from\")),\n-\t\tOPT_STRING(0, \"reference\", &reference,\n-\t\t\t N_(\"string\"),\n+\t\tOPT_STRING_LIST(0, \"reference\", &reference,\n+\t\t\t N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n \t\tOPT_STRING(0, \"depth\", &depth,\n \t\t\t N_(\"string\"),\n@@ -528,7 +532,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tif (!file_exists(sm_gitdir)) {\n \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n \t\t\tdie(_(\"could not create directory '%s'\"), sm_gitdir);\n-\t\tif (clone_submodule(path, sm_gitdir, url, depth, reference, quiet))\n+\t\tif (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet))\n \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n \t\t\t url, path);\n \t} else {\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293441","messageId":"20160809040811.21408-2-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 1/9] t7408: modernize style","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:01Z","receivedAt":"2016-08-09T04:08:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"No functional change intended. This commit only changes formatting\nto the style we recently use, e.g. starting the body of a test with a\nsingle quote on the same line as the header, and then having the test\nindented in the following lines.\n\nWhenever we change directories, we do that in subshells.\n\nSigned-off-by: Stefan Beller \n---\n t/t7408-submodule-reference.sh | 140 +++++++++++++++++++++--------------------\n 1 file changed, 71 insertions(+), 69 deletions(-)\n\ndiff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\nindex eaea19b..beee0bb 100755\n--- a/t/t7408-submodule-reference.sh\n+++ b/t/t7408-submodule-reference.sh\n@@ -8,74 +8,76 @@ test_description='test clone --reference'\n \n base_dir=$(pwd)\n \n-U=$base_dir/UPLOAD_LOG\n-\n-test_expect_success 'preparing first repository' \\\n-'test_create_repo A && cd A &&\n-echo first > file1 &&\n-git add file1 &&\n-git commit -m A-initial'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'preparing second repository' \\\n-'git clone A B && cd B &&\n-echo second > file2 &&\n-git add file2 &&\n-git commit -m B-addition &&\n-git repack -a -d &&\n-git prune'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'preparing superproject' \\\n-'test_create_repo super && cd super &&\n-echo file > file &&\n-git add file &&\n-git commit -m B-super-initial'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'submodule add --reference' \\\n-'cd super && git submodule add --reference ../B \"file://$base_dir/A\" sub &&\n-git commit -m B-super-added'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'after add: existence of info/alternates' \\\n-'test_line_count = 1 super/.git/modules/sub/objects/info/alternates'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'that reference gets used with add' \\\n-'cd super/sub &&\n-echo \"0 objects, 0 kilobytes\" > expected &&\n-git count-objects > current &&\n-diff expected current'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'cloning superproject' \\\n-'git clone super super-clone'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'update with reference' \\\n-'cd super-clone && git submodule update --init --reference ../B'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'after update: existence of info/alternates' \\\n-'test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates'\n-\n-cd \"$base_dir\"\n-\n-test_expect_success 'that reference gets used with update' \\\n-'cd super-clone/sub &&\n-echo \"0 objects, 0 kilobytes\" > expected &&\n-git count-objects > current &&\n-diff expected current'\n-\n-cd \"$base_dir\"\n+test_expect_success 'preparing first repository' '\n+\ttest_create_repo A &&\n+\t(\n+\t\tcd A &&\n+\t\techo first >file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m A-initial\n+\t)\n+'\n+\n+test_expect_success 'preparing second repository' '\n+\tgit clone A B &&\n+\t(\n+\t\tcd B &&\n+\t\techo second >file2 &&\n+\t\tgit add file2 &&\n+\t\tgit commit -m B-addition &&\n+\t\tgit repack -a -d &&\n+\t\tgit prune\n+\t)\n+'\n+\n+test_expect_success 'preparing superproject' '\n+\ttest_create_repo super &&\n+\t(\n+\t\tcd super &&\n+\t\techo file >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m B-super-initial\n+\t)\n+'\n+\n+test_expect_success 'submodule add --reference' '\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule add --reference ../B \"file://$base_dir/A\" sub &&\n+\t\tgit commit -m B-super-added\n+\t)\n+'\n+\n+test_expect_success 'after add: existence of info/alternates' '\n+\ttest_line_count = 1 super/.git/modules/sub/objects/info/alternates\n+'\n+\n+test_expect_success 'that reference gets used with add' '\n+\t(\n+\t\tcd super/sub &&\n+\t\techo \"0 objects, 0 kilobytes\" > expected &&\n+\t\tgit count-objects > current &&\n+\t\tdiff expected current\n+\t)\n+'\n+\n+test_expect_success 'cloning superproject' '\n+\tgit clone super super-clone\n+'\n+\n+test_expect_success 'update with reference' '\n+\tcd super-clone && git submodule update --init --reference ../B\n+'\n+\n+test_expect_success 'after update: existence of info/alternates' '\n+\ttest_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates\n+'\n+\n+test_expect_success 'that reference gets used with update' '\n+\tcd super-clone/sub &&\n+\techo \"0 objects, 0 kilobytes\" > expected &&\n+\tgit count-objects > current &&\n+\tdiff expected current\n+'\n \n test_done\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293442","messageId":"20160809040811.21408-3-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 2/9] t7408: merge short tests, factor out testing method","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:02Z","receivedAt":"2016-08-09T04:08:36Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Tests consisting of one line each can be consolidated to have fewer tests\nto run as well as fewer lines of code.\n\nWhen having just a few git commands, do not create a new shell but\nuse the -C flag in Git to execute in the correct directory.\n\nSigned-off-by: Stefan Beller \n---\n t/t7408-submodule-reference.sh | 57 ++++++++++++++++++------------------------\n 1 file changed, 25 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\nindex beee0bb..4a1b8f0 100755\n--- a/t/t7408-submodule-reference.sh\n+++ b/t/t7408-submodule-reference.sh\n@@ -8,6 +8,15 @@ test_description='test clone --reference'\n \n base_dir=$(pwd)\n \n+test_alternate_is_used () {\n+\talternates_file=\"$1\" &&\n+\tworking_dir=\"$2\" &&\n+\ttest_line_count = 1 \"$alternates_file\" &&\n+\techo \"0 objects, 0 kilobytes\" >expect &&\n+\tgit -C \"$working_dir\" count-objects >actual &&\n+\ttest_cmp expect actual\n+}\n+\n test_expect_success 'preparing first repository' '\n \ttest_create_repo A &&\n \t(\n@@ -40,44 +49,28 @@ test_expect_success 'preparing superproject' '\n \t)\n '\n \n-test_expect_success 'submodule add --reference' '\n+test_expect_success 'submodule add --reference uses alternates' '\n \t(\n \t\tcd super &&\n \t\tgit submodule add --reference ../B \"file://$base_dir/A\" sub &&\n-\t\tgit commit -m B-super-added\n-\t)\n-'\n-\n-test_expect_success 'after add: existence of info/alternates' '\n-\ttest_line_count = 1 super/.git/modules/sub/objects/info/alternates\n+\t\tgit commit -m B-super-added &&\n+\t\tgit repack -ad\n+\t) &&\n+\ttest_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub\n '\n \n-test_expect_success 'that reference gets used with add' '\n-\t(\n-\t\tcd super/sub &&\n-\t\techo \"0 objects, 0 kilobytes\" > expected &&\n-\t\tgit count-objects > current &&\n-\t\tdiff expected current\n-\t)\n-'\n-\n-test_expect_success 'cloning superproject' '\n-\tgit clone super super-clone\n-'\n-\n-test_expect_success 'update with reference' '\n-\tcd super-clone && git submodule update --init --reference ../B\n-'\n-\n-test_expect_success 'after update: existence of info/alternates' '\n-\ttest_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates\n-'\n+# The tests up to this point, and repositories created by them\n+# (A, B, super and super/sub), are about setting up the stage\n+# forsubsequent tests and meant to be kept throughout the\n+# remainder of the test.\n+# Tests from here on, if they create their own test repository,\n+# are expected to clean after themselves.\n \n-test_expect_success 'that reference gets used with update' '\n-\tcd super-clone/sub &&\n-\techo \"0 objects, 0 kilobytes\" > expected &&\n-\tgit count-objects > current &&\n-\tdiff expected current\n+test_expect_success 'updating superproject keeps alternates' '\n+\ttest_when_finished \"rm -rf super-clone\" &&\n+\tgit clone super super-clone &&\n+\tgit -C super-clone submodule update --init --reference ../B &&\n+\ttest_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub\n '\n \n test_done\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293443","messageId":"20160809040811.21408-5-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 4/9] submodule--helper update-clone: allow multiple references","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:04Z","receivedAt":"2016-08-09T04:08:38Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Allow the user to pass in multiple references to update_clone.\nCurrently this is only internal API, but once the shell script is\nreplaced by a C version, this is needed.\n\nThis fixes an API bug between the shell script and the helper.\nCurrently the helper accepts \"--reference\" \"--reference=foo\"\nas a OPT_STRING whose value happens to be \"--reference=foo\", and\nthen uses\n\n if (suc->reference)\n argv_array_push(&child->args, suc->reference)\n\nwhere suc->reference _is_ \"--reference=foo\" when invoking the\nunderlying \"git clone\", it cancels out.\n\nWith this change we omit one of the \"--reference\" arguments when\npassing references from the shell script to the helper.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 14 +++++++++-----\n git-submodule.sh | 2 +-\n 2 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ce9b3e9..7096848 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -584,7 +584,7 @@ struct submodule_update_clone {\n \t/* configuration parameters which are passed on to the children */\n \tint quiet;\n \tint recommend_shallow;\n-\tconst char *reference;\n+\tstruct string_list references;\n \tconst char *depth;\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n@@ -600,7 +600,8 @@ struct submodule_update_clone {\n \tint failed_clones_nr, failed_clones_alloc;\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n-\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \\\n+\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \\\n+\tNULL, NULL, NULL, \\\n \tSTRING_LIST_INIT_DUP, 0, NULL, 0, 0}\n \n \n@@ -710,8 +711,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \targv_array_pushl(&child->args, \"--path\", sub->path, NULL);\n \targv_array_pushl(&child->args, \"--name\", sub->name, NULL);\n \targv_array_pushl(&child->args, \"--url\", url, NULL);\n-\tif (suc->reference)\n-\t\targv_array_push(&child->args, suc->reference);\n+\tif (suc->references.nr) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, &suc->references)\n+\t\t\targv_array_pushl(&child->args, \"--reference\", item->string, NULL);\n+\t}\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n \n@@ -830,7 +834,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"update\", &update,\n \t\t\t N_(\"string\"),\n \t\t\t N_(\"rebase, merge, checkout or none\")),\n-\t\tOPT_STRING(0, \"reference\", &suc.reference, N_(\"repo\"),\n+\t\tOPT_STRING_LIST(0, \"reference\", &suc.references, N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"\",\n \t\t\t N_(\"Create a shallow clone truncated to the \"\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex c90dc33..3b412f5 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -575,7 +575,7 @@ cmd_update()\n \t\t${wt_prefix:+--prefix \"$wt_prefix\"} \\\n \t\t${prefix:+--recursive-prefix \"$prefix\"} \\\n \t\t${update:+--update \"$update\"} \\\n-\t\t${reference:+--reference \"$reference\"} \\\n+\t\t${reference:+\"$reference\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${recommend_shallow:+\"$recommend_shallow\"} \\\n \t\t${jobs:+$jobs} \\\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293444","messageId":"20160809040811.21408-6-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 5/9] clone: clarify option_reference as required","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:05Z","receivedAt":"2016-08-09T04:08:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the next patch we introduce optional references; To better distinguish\nbetween optional and required references we rename the variable.\n\nSigned-off-by: Stefan Beller \n---\n builtin/clone.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f044a8c..052a769 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -50,7 +50,7 @@ static int option_verbosity;\n static int option_progress = -1;\n static enum transport_family family;\n static struct string_list option_config = STRING_LIST_INIT_NODUP;\n-static struct string_list option_reference = STRING_LIST_INIT_NODUP;\n+static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;\n static int option_dissociate;\n static int max_jobs = -1;\n \n@@ -79,7 +79,7 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"number of submodules cloned in parallel\")),\n \tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n \t\t N_(\"directory from which templates will be used\")),\n-\tOPT_STRING_LIST(0, \"reference\", &option_reference, N_(\"repo\"),\n+\tOPT_STRING_LIST(0, \"reference\", &option_required_reference, N_(\"repo\"),\n \t\t\tN_(\"reference repository\")),\n \tOPT_BOOL(0, \"dissociate\", &option_dissociate,\n \t\t N_(\"use --reference only while cloning\")),\n@@ -325,7 +325,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \n static void setup_reference(void)\n {\n-\tfor_each_string_list(&option_reference, add_one_reference, NULL);\n+\tfor_each_string_list(&option_required_reference, add_one_reference, NULL);\n }\n \n static void copy_alternates(struct strbuf *src, struct strbuf *dst,\n@@ -977,7 +977,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tgit_config_set(key.buf, repo);\n \tstrbuf_reset(&key);\n \n-\tif (option_reference.nr)\n+\tif (option_required_reference.nr)\n \t\tsetup_reference();\n \n \tfetch_pattern = value.buf;\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293445","messageId":"20160809040811.21408-7-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv2 5/6] submodule update: add super-reference flag","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:06Z","receivedAt":"2016-08-09T04:08:45Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When we have a another clone of a superproject, we may want to\nmirror the submodules using alternates. Introduce an option\n`--super-reference` that let's a user point to another superproject,\nwhich is assumed to have the same structure as the one they are\nrunning the \"submodule update\" command from and has all submodules\nchecked out to borrow the submodule objects from within the other\nsuperprojects git directory.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-submodule.txt | 8 +++++++-\n builtin/submodule--helper.c | 14 +++++++++++++-\n git-submodule.sh | 10 ++++++++++\n 3 files changed, 30 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex bf3bb37..6f2f873 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] ...)\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t [--[no-]recommend-shallow] [-f|--force] [--rebase|--merge]\n-\t [--reference ] [--depth ] [--recursive]\n+\t [--[super-]reference ] [--depth ] [--recursive]\n \t [--jobs ] [--] [...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]\n \t [commit] [--] [...]\n@@ -370,6 +370,12 @@ the submodule itself.\n \tThis option is only valid for add and update commands. These\n \tcommands sometimes need to clone a remote repository. In this case,\n \tthis option will be passed to the linkgit:git-clone[1] command.\n+\n+--super-reference ::\n+\tThis option is only valid for the update command. When update needs\n+\tto clone a repository, a reference will be passed to the clone command\n+\tthat points at the submodule path inside the reference superproject.\n+\n +\n *NOTE*: Do *not* use this option unless you have read the note\n for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b7710a7..ea6b27c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -584,6 +584,7 @@ struct submodule_update_clone {\n \tint quiet;\n \tint recommend_shallow;\n \tstruct string_list references;\n+\tstruct string_list super_references;\n \tconst char *depth;\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n@@ -600,7 +601,7 @@ struct submodule_update_clone {\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \\\n-\tNULL, NULL, NULL, \\\n+\tSTRING_LIST_INIT_DUP, NULL, NULL, NULL, \\\n \tSTRING_LIST_INIT_DUP, 0, NULL, 0, 0}\n \n \n@@ -715,6 +716,15 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\tfor_each_string_list_item(item, &suc->references)\n \t\t\targv_array_pushl(&child->args, \"--reference\", item->string, NULL);\n \t}\n+\tif (suc->super_references.nr) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, &suc->super_references) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\targv_array_pushf(&child->args, \"--reference=%s/%s\",\n+\t\t\t\t\t relative_path(item->string, suc->prefix, &sb),\n+\t\t\t\t\t sub->path);\n+\t\t}\n+\t}\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n \n@@ -835,6 +845,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"rebase, merge, checkout or none\")),\n \t\tOPT_STRING_LIST(0, \"reference\", &suc.references, N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING_LIST(0, \"super-reference\", &suc.super_references, N_(\"repo\"),\n+\t\t\t N_(\"superproject of a reference repository\")),\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"\",\n \t\t\t N_(\"Create a shallow clone truncated to the \"\n \t\t\t \"specified number of revisions\")),\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3b412f5..17f4ace 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -34,6 +34,7 @@ command=\n branch=\n force=\n reference=\n+superreference=\n cached=\n recursive=\n init=\n@@ -520,6 +521,14 @@ cmd_update()\n \t\t--reference=*)\n \t\t\treference=\"$1\"\n \t\t\t;;\n+\t\t--super-reference)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tsuperreference=\"--super-reference=$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--super-reference=*)\n+\t\t\tsuperreference=\"$1\"\n+\t\t\t;;\n \t\t-m|--merge)\n \t\t\tupdate=\"merge\"\n \t\t\t;;\n@@ -576,6 +585,7 @@ cmd_update()\n \t\t${prefix:+--recursive-prefix \"$prefix\"} \\\n \t\t${update:+--update \"$update\"} \\\n \t\t${reference:+\"$reference\"} \\\n+\t\t${superreference:+\"$superreference\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${recommend_shallow:+\"$recommend_shallow\"} \\\n \t\t${jobs:+$jobs} \\\n-- \n2.9.2.572.g9d9644e.dirty\n\n"},{"id":"293446","messageId":"20160809040811.21408-9-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv2 6/6] clone: reference flag is used for submodules as well","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:08Z","receivedAt":"2016-08-09T04:08:48Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When giving a --reference while also giving --recurse, the alternates\nfor the submodules are assumed to be in the superproject as well.\n\nIn case they are not, we error out when cloning the submodule.\nHowever we error out completely, we did not record the alternates yet,\nso a following update command succeeds as usual (with no alternates).\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-clone.txt | 5 +++++\n builtin/clone.c | 22 ++++++++++++++++++----\n t/t7408-submodule-reference.sh | 31 ++++++++++++++++++++++++++++++-\n 3 files changed, 53 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex ec41d3d..2c6dea6 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -100,6 +100,11 @@ objects from the source repository into a pack in the cloned repository.\n +\n *NOTE*: see the NOTE for the `--shared` option, and also the\n `--dissociate` option.\n++\n+When `--reference` is given together with `--recursive`,\n+the reference repository is assumed to contain the submodules\n+as well and the submodules are setup as alternates of the\n+submodules in the given reference project.\n \n --dissociate::\n \tBorrow the objects from reference repositories specified\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f044a8c..e8272b5 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -51,6 +51,7 @@ static int option_progress = -1;\n static enum transport_family family;\n static struct string_list option_config = STRING_LIST_INIT_NODUP;\n static struct string_list option_reference = STRING_LIST_INIT_NODUP;\n+static struct string_list super_references = STRING_LIST_INIT_DUP;\n static int option_dissociate;\n static int max_jobs = -1;\n \n@@ -280,10 +281,11 @@ static void strip_trailing_slashes(char *dir)\n \t*end = '\\0';\n }\n \n-static int add_one_reference(struct string_list_item *item, void *cb_data)\n+static int add_one_reference(struct string_list_item *item, void *cb_dir)\n {\n \tchar *ref_git;\n \tconst char *repo;\n+\tconst char *dir = cb_dir;\n \tstruct strbuf alternate = STRBUF_INIT;\n \n \t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n@@ -316,6 +318,13 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \tif (!access(mkpath(\"%s/info/grafts\", ref_git), F_OK))\n \t\tdie(_(\"reference repository '%s' is grafted\"), item->string);\n \n+\tif (option_recursive) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tchar *rel = xstrdup(relative_path(item->string, dir, &sb));\n+\t\tstring_list_append(&super_references, rel);\n+\t\tstrbuf_reset(&sb);\n+\t}\n+\n \tstrbuf_addf(&alternate, \"%s/objects\", ref_git);\n \tadd_to_alternates_file(alternate.buf);\n \tstrbuf_release(&alternate);\n@@ -323,9 +332,9 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \treturn 0;\n }\n \n-static void setup_reference(void)\n+static void setup_reference(char *dir)\n {\n-\tfor_each_string_list(&option_reference, add_one_reference, NULL);\n+\tfor_each_string_list(&option_reference, add_one_reference, dir);\n }\n \n static void copy_alternates(struct strbuf *src, struct strbuf *dst,\n@@ -736,6 +745,7 @@ static int checkout(void)\n \n \tif (!err && option_recursive) {\n \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\t\tstatic struct string_list_item *item;\n \t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n \n \t\tif (option_shallow_submodules == 1)\n@@ -744,6 +754,10 @@ static int checkout(void)\n \t\tif (max_jobs != -1)\n \t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n \n+\t\tif (super_references.nr)\n+\t\t\tfor_each_string_list_item(item, &super_references)\n+\t\t\t\targv_array_pushf(&args, \"--super-reference=%s\", item->string);\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\n@@ -978,7 +992,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstrbuf_reset(&key);\n \n \tif (option_reference.nr)\n-\t\tsetup_reference();\n+\t\tsetup_reference(dir);\n \n \tfetch_pattern = value.buf;\n \trefspec = parse_fetch_refspec(1, &fetch_pattern);\ndiff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\nindex 1d9326e..588d53e 100755\n--- a/t/t7408-submodule-reference.sh\n+++ b/t/t7408-submodule-reference.sh\n@@ -53,7 +53,8 @@ test_expect_success 'submodule add --reference uses alternates' '\n \t(\n \t\tcd super &&\n \t\tgit submodule add --reference ../B \"file://$base_dir/A\" sub &&\n-\t\tgit commit -m B-super-added\n+\t\tgit commit -m B-super-added &&\n+\t\tgit repack -ad\n \t) &&\n \ttest_alternate_usage super/.git/modules/sub/objects/info/alternates super/sub\n '\n@@ -65,4 +66,32 @@ test_expect_success 'updating superproject keeps alternates' '\n \ttest_alternate_usage super-clone/.git/modules/sub/objects/info/alternates super-clone/sub\n '\n \n+test_expect_success 'submodules use alternates when cloning a superproject' '\n+\ttest_when_finished \"rm -rf super-clone\" &&\n+\tgit clone --reference super --recursive super super-clone &&\n+\t(\n+\t\tcd super-clone &&\n+\t\t# test superproject has alternates setup correctly\n+\t\ttest_alternate_usage .git/objects/info/alternates . &&\n+\t\t# test submodule has correct setup\n+\t\ttest_alternate_usage .git/modules/sub/objects/info/alternates sub\n+\t)\n+'\n+\n+test_expect_success 'cloning superproject, missing submodule alternates' '\n+\ttest_when_finished \"rm -rf super-clone\" &&\n+\tgit clone super super2 &&\n+\ttest_must_fail git clone --recursive --reference super2 super2 super-clone &&\n+\t(\n+\t\tcd super-clone &&\n+\t\t# test superproject has alternates setup correctly\n+\t\ttest_alternate_usage .git/objects/info/alternates . &&\n+\t\t# update of the submodule succeeds\n+\t\tgit submodule update --init &&\n+\t\t# and we have no alternates:\n+\t\ttest_must_fail test_alternate_usage .git/modules/sub/objects/info/alternates sub &&\n+\t\ttest_path_is_file sub/file1\n+\t)\n+'\n+\n test_done\n-- \n2.9.2.572.g9d9644e.dirty\n\n"},{"id":"293447","messageId":"20160809040811.21408-12-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 9/9] submodule--helper: use parallel processor correctly.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:11Z","receivedAt":"2016-08-09T04:08:49Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When developing the prior patches I had a temporary state in which\ngit-clone would segfault, when prepared the call in\nprepare_to_clone_next_submodule. This lead to the call failing, i.e. in\n`update_clone_task_finished` the task was scheduled to be tried again.\nThe second call to prepare_to_clone_next_submodule would return 0, as the\nsegfaulted clone did create the .git file already, such that was not\nconsidered to need to be cloned again. I was seeing the \"BUG: ce was\na submodule before?\\n\" message, which was the correct behavior at the\ntime as my local code was buggy. When trying to debug this failure, I\ntried to use printing messages into the strbuf that is passed around,\nbut these messages were never printed as the die(..) doesn't\nflush the `err` strbuf.\n\nWhen implementing the die() in 665b35ecc (2016-06-09, \"submodule--helper:\ninitial clone learns retry logic\"), I considered this condition to be\na severe condition, which should lead to an immediate abort as we do not\ntrust ourselves any more. However the queued messages in `err` are valuable\nso let's not toss them out by immediately dieing, but a graceful return.\n\nAnother thing to note: The error message itself was missleading. A return\nvalue of 0 doesn't indicate the passed in `ce` is not a submodule any more,\nbut just that we do not consider cloning it any more.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex fc14843..3e40f99 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -815,8 +815,12 @@ static int update_clone_get_next_task(struct child_process *child,\n \tif (index < suc->failed_clones_nr) {\n \t\tint *p;\n \t\tce = suc->failed_clones[index];\n-\t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err))\n-\t\t\tdie(\"BUG: ce was a submodule before?\");\n+\t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err)) {\n+\t\t\tsuc->current ++;\n+\t\t\tstrbuf_addf(err, \"BUG: submodule considered for cloning,\"\n+\t\t\t\t \"doesn't need cloning any more?\\n\");\n+\t\t\treturn 0;\n+\t\t}\n \t\tp = xmalloc(sizeof(*p));\n \t\t*p = suc->current;\n \t\t*idx_task_cb = p;\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293448","messageId":"20160809040811.21408-11-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 8/9] submodule: try alternates when superproject has an alternate","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:10Z","receivedAt":"2016-08-09T04:08:50Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n Documentation/git-clone.txt | 4 ++++\n builtin/submodule--helper.c | 47 ++++++++++++++++++++++++++++++++++++++++++\n t/t7408-submodule-reference.sh | 29 +++++++++++++++++++++++++-\n 3 files changed, 79 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex e316c4b..cadf138 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -103,6 +103,10 @@ objects from the source repository into a pack in the cloned repository.\n +\n *NOTE*: see the NOTE for the `--shared` option, and also the\n `--dissociate` option.\n++\n+When using --reference any submodule that is cloned\n+sets up a corresponding alternate at $GIT_DIR/modules if such a\n+an alternate exists.\n \n --dissociate::\n \tBorrow the objects from reference repositories specified\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f360473..fc14843 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -635,6 +635,45 @@ static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n \t}\n }\n \n+struct submodule_alternate_setup {\n+\tstruct submodule_update_clone *suc;\n+\tconst char *submodule_name;\n+\tstruct child_process *child;\n+\tstruct strbuf *out;\n+};\n+\n+int add_possible_reference(struct alternate_object_database *alt, void *sas_cb)\n+{\n+\tstruct submodule_alternate_setup *sas = sas_cb;\n+\n+\t/* directory name, minus trailing slash */\n+\tsize_t namelen = alt->name - alt->base - 1;\n+\tstruct strbuf name = STRBUF_INIT;\n+\tstrbuf_add(&name, alt->base, namelen);\n+\n+\t/*\n+\t * If the alternate object store is another repository, try the\n+\t * standard layout with .git/modules//objects\n+\t */\n+\tif (ends_with(name.buf, \".git/objects\")) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tstrbuf_add(&sb, name.buf, name.len - strlen(\"objects\"));\n+\t\t/*\n+\t\t * We need to end the new path with '/' to mark it as a dir,\n+\t\t * otherwise a submodule name containing '/' will be broken\n+\t\t * as the last part of a missing submodule reference would\n+\t\t * be taken as a file name.\n+\t\t */\n+\t\tstrbuf_addf(&sb, \"modules/%s/\", sas->submodule_name);\n+\t\targv_array_pushf(&sas->child->args,\n+\t\t\t\t \"--reference-if-able=%s\", sb.buf);\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n+\tstrbuf_release(&name);\n+\treturn 0;\n+}\n+\n /**\n * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to\n * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.\n@@ -650,6 +689,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tconst char *displaypath = NULL;\n \tchar *url = NULL;\n \tint needs_cloning = 0;\n+\tstruct submodule_alternate_setup sas;\n \n \tif (ce_stage(ce)) {\n \t\tif (suc->recursive_prefix)\n@@ -728,6 +768,13 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\tfor_each_string_list_item(item, &suc->references)\n \t\t\targv_array_pushl(&child->args, \"--reference\", item->string, NULL);\n \t}\n+\n+\tsas.submodule_name = sub->name;\n+\tsas.suc = suc;\n+\tsas.child = child;\n+\tsas.out = out;\n+\tforeach_alt_odb(add_possible_reference, &sas);\n+\n \tif (suc->depth)\n \t\targv_array_push(&child->args, suc->depth);\n \ndiff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\nindex 4a1b8f0..a9b89a3 100755\n--- a/t/t7408-submodule-reference.sh\n+++ b/t/t7408-submodule-reference.sh\n@@ -7,7 +7,6 @@ test_description='test clone --reference'\n . ./test-lib.sh\n \n base_dir=$(pwd)\n-\n test_alternate_is_used () {\n \talternates_file=\"$1\" &&\n \tworking_dir=\"$2\" &&\n@@ -73,4 +72,32 @@ test_expect_success 'updating superproject keeps alternates' '\n \ttest_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub\n '\n \n+test_expect_success 'submodules use alternates when cloning a superproject' '\n+\ttest_when_finished \"rm -rf super-clone\" &&\n+\tgit clone --reference super --recursive super super-clone &&\n+\t(\n+\t\tcd super-clone &&\n+\t\t# test superproject has alternates setup correctly\n+\t\ttest_alternate_is_used .git/objects/info/alternates . &&\n+\t\t# test submodule has correct setup\n+\t\ttest_alternate_is_used .git/modules/sub/objects/info/alternates sub\n+\t)\n+'\n+\n+test_expect_success 'cloning superproject, missing submodule alternates' '\n+\ttest_when_finished \"rm -rf super-clone\" &&\n+\tgit clone super super2 &&\n+\tgit clone --recursive --reference super2 super2 super-clone &&\n+\t(\n+\t\tcd super-clone &&\n+\t\t# test superproject has alternates setup correctly\n+\t\ttest_alternate_is_used .git/objects/info/alternates . &&\n+\t\t# update of the submodule succeeds\n+\t\tgit submodule update --init &&\n+\t\t# and we have no alternates:\n+\t\ttest_must_fail test_alternate_is_used .git/modules/sub/objects/info/alternates sub &&\n+\t\ttest_path_is_file sub/file1\n+\t)\n+'\n+\n test_done\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293449","messageId":"20160809040811.21408-10-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 7/9] submodule helper: pass through --reference-if-able","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:09Z","receivedAt":"2016-08-09T04:13:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we need to pass optional references\nthrough to git clone, so add the capability for it.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 20 ++++++++++++++++----\n 1 file changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 7096848..f360473 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -442,7 +442,8 @@ static int module_name(int argc, const char **argv, const char *prefix)\n }\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 quiet)\n+\t\t\t const char *depth, struct string_list *reference,\n+\t\t\t struct string_list *reference_if_able, int quiet)\n {\n \tstruct child_process cp;\n \tchild_process_init(&cp);\n@@ -459,6 +460,12 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \t\t\targv_array_pushl(&cp.args, \"--reference\",\n \t\t\t\t\t item->string, NULL);\n \t}\n+\tif (reference_if_able->nr) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, reference_if_able)\n+\t\t\targv_array_pushl(&cp.args, \"--reference-if-able\",\n+\t\t\t\t\t item->string, NULL);\n+\t}\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n \n@@ -481,6 +488,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rel_path = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n+\tstruct string_list reference_if_able = STRING_LIST_INIT_NODUP;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n@@ -498,6 +506,9 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"reference\", &reference,\n \t\t\t N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING_LIST(0, \"reference-if-able\", &reference_if_able,\n+\t\t\t N_(\"repo\"),\n+\t\t\t N_(\"reference repository\")),\n \t\tOPT_STRING(0, \"depth\", &depth,\n \t\t\t N_(\"string\"),\n \t\t\t N_(\"depth for shallow clones\")),\n@@ -507,8 +518,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\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 \"--url --path \"),\n+\t\t \"[--reference[-if-able] ] [--name ] \"\n+\t\t \"[--depth ] --url --path \"),\n \t\tNULL\n \t};\n \n@@ -532,7 +543,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tif (!file_exists(sm_gitdir)) {\n \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n \t\t\tdie(_(\"could not create directory '%s'\"), sm_gitdir);\n-\t\tif (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet))\n+\t\tif (clone_submodule(path, sm_gitdir, url, depth, &reference,\n+\t\t &reference_if_able, quiet))\n \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n \t\t\t url, path);\n \t} else {\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293450","messageId":"20160809040811.21408-8-sbeller@google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"[PATCHv3 6/9] clone: implement optional references","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:07Z","receivedAt":"2016-08-09T04:14:09Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we want to try to create alternates for\nall submodules, but they might not exist in the referenced\nsuperproject. So add a way to skip the non existing references\nand report them.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-clone.txt | 5 ++++-\n builtin/clone.c | 32 ++++++++++++++++++++++++++------\n 2 files changed, 30 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex ec41d3d..e316c4b 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -90,13 +90,16 @@ If you want to break the dependency of a repository cloned with `-s` on\n its source repository, you can simply run `git repack -a` to copy all\n objects from the source repository into a pack in the cloned repository.\n \n---reference ::\n+--reference[-if-able] ::\n \tIf the reference repository is on the local machine,\n \tautomatically setup `.git/objects/info/alternates` to\n \tobtain objects from the reference repository. Using\n \tan already existing repository as an alternate will\n \trequire fewer objects to be copied from the repository\n \tbeing cloned, reducing network and local storage costs.\n+\tWhen using the `--reference-if-able`, a non existing\n+\tdirectory is skipped with a warning instead of aborting\n+\tthe clone.\n +\n *NOTE*: see the NOTE for the `--shared` option, and also the\n `--dissociate` option.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 052a769..8f3c4d4 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -51,6 +51,7 @@ static int option_progress = -1;\n static enum transport_family family;\n static struct string_list option_config = STRING_LIST_INIT_NODUP;\n static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;\n+static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;\n static int option_dissociate;\n static int max_jobs = -1;\n \n@@ -81,6 +82,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"directory from which templates will be used\")),\n \tOPT_STRING_LIST(0, \"reference\", &option_required_reference, N_(\"repo\"),\n \t\t\tN_(\"reference repository\")),\n+\tOPT_STRING_LIST(0, \"reference-if-able\", &option_optional_reference,\n+\t\t\tN_(\"repo\"), N_(\"reference repository\")),\n \tOPT_BOOL(0, \"dissociate\", &option_dissociate,\n \t\t N_(\"use --reference only while cloning\")),\n \tOPT_STRING('o', \"origin\", &option_origin, N_(\"name\"),\n@@ -283,11 +286,22 @@ static void strip_trailing_slashes(char *dir)\n static int add_one_reference(struct string_list_item *item, void *cb_data)\n {\n \tchar *ref_git;\n-\tconst char *repo;\n+\tconst char *repo, *ref_git_s;\n+\tint *required = cb_data;\n \tstruct strbuf alternate = STRBUF_INIT;\n \n-\t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n-\tref_git = xstrdup(real_path(item->string));\n+\tref_git_s = *required ?\n+\t\t\treal_path(item->string) :\n+\t\t\treal_path_if_valid(item->string);\n+\tif (!ref_git_s) {\n+\t\twarning(_(\"Not using proposed alternate %s\"), item->string);\n+\t\treturn 0;\n+\t} else\n+\t\t/*\n+\t\t * Beware: read_gitfile(), real_path() and mkpath()\n+\t\t * return static buffer\n+\t\t */\n+\t\tref_git = xstrdup(ref_git_s);\n \n \trepo = read_gitfile(ref_git);\n \tif (!repo)\n@@ -304,7 +318,8 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git))) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\tif (get_common_dir(&sb, ref_git))\n-\t\t\tdie(_(\"reference repository '%s' as a linked checkout is not supported yet.\"),\n+\t\t\tdie(_(\"reference repository '%s' as a \"\n+\t\t\t \"linked checkout is not supported yet.\"),\n \t\t\t item->string);\n \t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n \t\t item->string);\n@@ -325,7 +340,12 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \n static void setup_reference(void)\n {\n-\tfor_each_string_list(&option_required_reference, add_one_reference, NULL);\n+\tint required = 1;\n+\tfor_each_string_list(&option_required_reference,\n+\t\t\t add_one_reference, &required);\n+\trequired = 0;\n+\tfor_each_string_list(&option_optional_reference,\n+\t\t\t add_one_reference, &required);\n }\n \n static void copy_alternates(struct strbuf *src, struct strbuf *dst,\n@@ -977,7 +997,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tgit_config_set(key.buf, repo);\n \tstrbuf_reset(&key);\n \n-\tif (option_required_reference.nr)\n+\tif (option_required_reference.nr || option_optional_reference.nr)\n \t\tsetup_reference();\n \n \tfetch_pattern = value.buf;\n-- \n2.9.2.583.gd6329be.dirty\n\n"},{"id":"293451","messageId":"20160809040811.21408-1-sbeller@google.com","threadId":"43031","inReplyTo":null,"subject":"[PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T04:08:00Z","receivedAt":"2016-08-09T04:15:44Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"\nv3:\n\nThanks to Junios critial questions regarding the design, I took a step back\nto look at the bigger picture. \n\n--super-reference sounds confusing. (what is the super referring to?)\nSo drop that approach.\n\nInstead we'll compute where the reference might be in the superproject scope\nand ask the submodule clone operation to consider an optional reference.\nIf the referenced alternate is not there, we'll just warn about it and\ncarry on.\n\n\n\n* fixed the style in patch 2.\n\n* fixed another bug in the last patch, that is unrelated, but would have helped\n me a lot.\n\nThanks,\nStefan\n\n Documentation/git-clone.txt | 9 ++++++++-\n builtin/clone.c | 36 ++++++++++++++++++++++++++++--------\n builtin/submodule--helper.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------\n git-submodule.sh | 2 +-\n t/t7408-submodule-reference.sh | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------\n 5 files changed, 217 insertions(+), 97 deletions(-)\n\nv2:\n * fixed the p1,2 cleanups\n * added documentation to patches 5,6\n * improved commit message in v4\n \n Thanks,\n Stefan\n \nv1:\n \n Currently when cloning a superproject with --recursive and --reference\n only the superproject learns about its alternates. The submodules are\n cloned independently, which may incur lots of network costs.\n \n Assume that the reference repository has the submodules at the same\n paths as the to-be-cloned submodule and try to setup alternates from\n there.\n \n Some submodules in the referenced superproject may not be there, \n (they are just not initialized/cloned/checked out), which yields\n an error for now. In future work we may want to soften the alternate\n check and not die in the clone when one of the given alternates doesn't\n exist.\n \n patch 1,2 are modernizing style of t7408, \n patches 3,4 are not strictly necessary, but I think it is a good thing\n to not leave the submodule related C code in a crippled state (i.e.\n allowing only one reference). The shell code would also need this update,\n but it looked ugly to me, so I postpone it until more of the submodule code\n is written in C. \n \n Thanks,\n Stefan \n\nStefan Beller (6):\n t7408: modernize style\n t7408: merge short tests, factor out testing method\n submodule--helper module-clone: allow multiple references\n submodule--helper update-clone: allow multiple references\n submodule update: add super-reference flag\n clone: reference flag is used for submodules as well\n\n builtin/clone.c | 22 ++++--\n builtin/submodule--helper.c | 45 ++++++++----\n git-submodule.sh | 12 +++-\n t/t7408-submodule-reference.sh | 153 +++++++++++++++++++++++------------------\n 4 files changed, 147 insertions(+), 85 deletions(-)\n\n-- \n2.9.2.572.g9d9644e.dirty\n\n"},{"id":"293454","messageId":"CA+P7+xpHBkAHU7bAGHN4Hy1SnNS16zAg8XnFjzPY1CBKZV1s1Q@mail.gmail.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-09T05:23:44Z","receivedAt":"2016-08-09T05:24:15Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Aug 8, 2016 at 9:08 PM, Stefan Beller wrote:\n>\n> v3:\n>\n> Thanks to Junios critial questions regarding the design, I took a step back\n> to look at the bigger picture.\n>\n> --super-reference sounds confusing. (what is the super referring to?)\n> So drop that approach.\n>\n> Instead we'll compute where the reference might be in the superproject scope\n> and ask the submodule clone operation to consider an optional reference.\n> If the referenced alternate is not there, we'll just warn about it and\n> carry on.\n>\n\nThis makes a lot more sense to me, and being able to use\n\"reference-if-able\" for regular clones may be a useful addition as\nwell.\n\nI looked over the other patches and didn't see anything obviously\nwrong, but take that with a grain of salt as I didn't spend a lot of\ntime at it.\n\nThanks,\nJake\n"},{"id":"293458","messageId":"CAPig+cRnDVMBZzKOOS-fW+hNaCHhYRVLM+d_akZcB4H5iyqfKA@mail.gmail.com","threadId":"43031","inReplyTo":"20160809040811.21408-2-sbeller@google.com","subject":"Re: [PATCHv3 1/9] t7408: modernize style","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-08-09T06:59:18Z","receivedAt":"2016-08-09T06:59:37Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 9, 2016 at 12:08 AM, Stefan Beller wrote:\n> t7408: modernize style\n>\n> Signed-off-by: Stefan Beller \n> ---\n> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\n> @@ -8,74 +8,76 @@ test_description='test clone --reference'\n> +test_expect_success 'that reference gets used with add' '\n> + (\n> + cd super/sub &&\n> + echo \"0 objects, 0 kilobytes\" > expected &&\n\nSince you're modernizing the style, perhaps drop the space after '>'\nhere and elsewhere.\n\n> + git count-objects > current &&\n> + diff expected current\n\nModern practice is to call this 'actual' rather than 'current', but\nperhaps that's outside the scope of this patch(?).\n\n> + )\n> +'\n"},{"id":"293494","messageId":"xmqqk2fqc583.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"20160809040811.21408-1-sbeller@google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T15:49:32Z","receivedAt":"2016-08-09T15:49:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> v3:\n>\n> Thanks to Junios critial questions regarding the design, I took a step back\n> to look at the bigger picture. \n>\n> --super-reference sounds confusing. (what is the super referring to?)\n> So drop that approach.\n>\n> Instead we'll compute where the reference might be in the superproject scope\n> and ask the submodule clone operation to consider an optional reference.\n> If the referenced alternate is not there, we'll just warn about it and\n> carry on.\n\nI like the general direction, but I hope that the warning comes only\nwhen the user said \"--reference\" on the command line (i.e. \"you\nasked me to use reference, but for this submodule I couldn't find a\nusable one\"). If the implementation allows the same mechanism to\nhelp later \"submodule init && submodule update\" borrow from the\nsubmodule repositories of the superproject the current superproject\nborrows from (i.e. no explicit \"--reference\" on the command line\nwhen doing init/update), I would think the case that needs warning\nis \"you didn't explicitly ask me to borrow objects, but I found one\nwe could, so I did it anyway without being asked\", and it is not a\nwarning-worthy condition if we didn't find a cloned submodule in the\nrepository our superproject borrows from.\n\nThanks.\n"},{"id":"293495","messageId":"xmqqfuqec56x.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAPig+cRnDVMBZzKOOS-fW+hNaCHhYRVLM+d_akZcB4H5iyqfKA@mail.gmail.com","subject":"Re: [PATCHv3 1/9] t7408: modernize style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T15:50:14Z","receivedAt":"2016-08-09T15:50:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Tue, Aug 9, 2016 at 12:08 AM, Stefan Beller wrote:\n>> t7408: modernize style\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\n>> @@ -8,74 +8,76 @@ test_description='test clone --reference'\n>> +test_expect_success 'that reference gets used with add' '\n>> + (\n>> + cd super/sub &&\n>> + echo \"0 objects, 0 kilobytes\" > expected &&\n>\n> Since you're modernizing the style, perhaps drop the space after '>'\n> here and elsewhere.\n>\n>> + git count-objects > current &&\n>> + diff expected current\n>\n> Modern practice is to call this 'actual' rather than 'current', but\n> perhaps that's outside the scope of this patch(?).\n\nNot just that, modern practice is to use \"test_cmp\" not \"diff\".\n"},{"id":"293496","messageId":"xmqqbn12c55e.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"xmqqfuqec56x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 1/9] t7408: modernize style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T15:51:09Z","receivedAt":"2016-08-09T15:51:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Eric Sunshine writes:\n>\n>> On Tue, Aug 9, 2016 at 12:08 AM, Stefan Beller wrote:\n>>> t7408: modernize style\n>>>\n>>> Signed-off-by: Stefan Beller \n>>> ---\n>>> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\n>>> @@ -8,74 +8,76 @@ test_description='test clone --reference'\n>>> +test_expect_success 'that reference gets used with add' '\n>>> + (\n>>> + cd super/sub &&\n>>> + echo \"0 objects, 0 kilobytes\" > expected &&\n>>\n>> Since you're modernizing the style, perhaps drop the space after '>'\n>> here and elsewhere.\n>>\n>>> + git count-objects > current &&\n>>> + diff expected current\n>>\n>> Modern practice is to call this 'actual' rather than 'current', but\n>> perhaps that's outside the scope of this patch(?).\n>\n> Not just that, modern practice is to use \"test_cmp\" not \"diff\".\n\nSorry, hit the \"send\" too soon, before writing \"But all of that\ncomes in the next step.\"\n\n"},{"id":"293497","messageId":"xmqq7fbpdhkw.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"20160809040811.21408-8-sbeller@google.com","subject":"Re: [PATCHv3 6/9] clone: implement optional references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T16:37:19Z","receivedAt":"2016-08-09T16:37:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> @@ -283,11 +286,22 @@ static void strip_trailing_slashes(char *dir)\n> static int add_one_reference(struct string_list_item *item, void *cb_data)\n> {\n> \tchar *ref_git;\n> -\tconst char *repo;\n> +\tconst char *repo, *ref_git_s;\n> +\tint *required = cb_data;\n> \tstruct strbuf alternate = STRBUF_INIT;\n> \n> -\t/* Beware: read_gitfile(), real_path() and mkpath() return static buffer */\n> -\tref_git = xstrdup(real_path(item->string));\n> +\tref_git_s = *required ?\n> +\t\t\treal_path(item->string) :\n> +\t\t\treal_path_if_valid(item->string);\n> +\tif (!ref_git_s) {\n> +\t\twarning(_(\"Not using proposed alternate %s\"), item->string);\n\nIf I am reading the implementation of real_path_internal()\ncorrectly, the most relevant reason that an \"if-able\" repository\ncannot be used causes real_path_if_valid() to return NULL.\n\nLet's say your superproject borrows from ~/w/super, whose submodule\nrepositories (if cloned) are directly in \"~/w/super/.git/modules/\".\nWhen you clone a submodule X for your superproject, you allow it to\nborrow from \"~/w/super/.git/modules/X\" if there is one available.\n\nI think both real_path() and real_path_if_valid() happily returns\nthe real path to \"~/w/super/.git/modules/\" with \"X\" concatenated at\nthe end, as long as that 'modules' directory exists, even when there\nis no X inside.\n\nUsing if_valid() is still necessary to avoid a totally bogus path to\ncause real_path() to barf. I am just saying that this warning is\nnot so interesting, and a real check, \"do we have a repository\nthere? If not, don't add it as an alternate\" must be somewhere\nelse.\n\n> +\t\treturn 0;\n> +\t} else\n> +\t\t/*\n> +\t\t * Beware: read_gitfile(), real_path() and mkpath()\n> +\t\t * return static buffer\n> +\t\t */\n> +\t\tref_git = xstrdup(ref_git_s);\n> \n> \trepo = read_gitfile(ref_git);\n> \tif (!repo)\n> @@ -304,7 +318,8 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n> \t} else if (!is_directory(mkpath(\"%s/objects\", ref_git))) {\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> \t\tif (get_common_dir(&sb, ref_git))\n> -\t\t\tdie(_(\"reference repository '%s' as a linked checkout is not supported yet.\"),\n> +\t\t\tdie(_(\"reference repository '%s' as a \"\n> +\t\t\t \"linked checkout is not supported yet.\"),\n> \t\t\t item->string);\n\nAnd curiously, this is not such a check. This is an unrelated change.\n\n> \t\tdie(_(\"reference repository '%s' is not a local repository.\"),\n> \t\t item->string);\n\nYou need to arrange this die() not to trigger when you are asked to\nborrow from there if there is one to borrow from. I see a few other\ndie() following this check to avoid using shallow repositories and\nrepositories with grafts, but I think they all should turn into a\n\"Nah, this is unusable, and I was asked to borrow _only_ if the\nrepository is usable, so I won't use it.\"\n\n"},{"id":"293498","messageId":"xmqq37mddhf2.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"20160809040811.21408-12-sbeller@google.com","subject":"Re: [PATCHv3 9/9] submodule--helper: use parallel processor correctly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T16:40:49Z","receivedAt":"2016-08-09T16:41:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> When implementing the die() in 665b35ecc (2016-06-09, \"submodule--helper:\n> initial clone learns retry logic\"), I considered this condition to be\n> a severe condition, which should lead to an immediate abort as we do not\n> trust ourselves any more. However the queued messages in `err` are valuable\n> so let's not toss them out by immediately dieing, but a graceful return.\n\nI think you'll be rerolling this series at least once (if only to\ncorrect for 6/9), so perhaps split this fix into a preparatory fix\nthat can go earlier to 'next' and further that the remainder of the\nseries depend on?\n\n>\n> Another thing to note: The error message itself was missleading. A return\n> value of 0 doesn't indicate the passed in `ce` is not a submodule any more,\n> but just that we do not consider cloning it any more.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> builtin/submodule--helper.c | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index fc14843..3e40f99 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -815,8 +815,12 @@ static int update_clone_get_next_task(struct child_process *child,\n> \tif (index < suc->failed_clones_nr) {\n> \t\tint *p;\n> \t\tce = suc->failed_clones[index];\n> -\t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err))\n> -\t\t\tdie(\"BUG: ce was a submodule before?\");\n> +\t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err)) {\n> +\t\t\tsuc->current ++;\n\ns/current /current/;\n\n> +\t\t\tstrbuf_addf(err, \"BUG: submodule considered for cloning,\"\n> +\t\t\t\t \"doesn't need cloning any more?\\n\");\n> +\t\t\treturn 0;\n> +\t\t}\n> \t\tp = xmalloc(sizeof(*p));\n> \t\t*p = suc->current;\n> \t\t*idx_task_cb = p;\n"},{"id":"293499","messageId":"xmqqy445c2sw.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"20160809040811.21408-3-sbeller@google.com","subject":"Re: [PATCHv3 2/9] t7408: merge short tests, factor out testing method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T16:41:51Z","receivedAt":"2016-08-09T16:42:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +# The tests up to this point, and repositories created by them\n> +# (A, B, super and super/sub), are about setting up the stage\n> +# forsubsequent tests and meant to be kept throughout the\n\ns/forsub/for sub/;\n"},{"id":"293503","messageId":"CAGZ79kZKTV5PCAR41O1t1c_y6N18u6gsoWozOfr=EPHic-7wYw@mail.gmail.com","threadId":"43031","inReplyTo":"xmqqk2fqc583.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T17:26:12Z","receivedAt":"2016-08-09T17:26:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 8:49 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> v3:\n>>\n>> Thanks to Junios critial questions regarding the design, I took a step back\n>> to look at the bigger picture.\n>>\n>> --super-reference sounds confusing. (what is the super referring to?)\n>> So drop that approach.\n>>\n>> Instead we'll compute where the reference might be in the superproject scope\n>> and ask the submodule clone operation to consider an optional reference.\n>> If the referenced alternate is not there, we'll just warn about it and\n>> carry on.\n>\n> I like the general direction, but I hope that the warning comes only\n> when the user said \"--reference\" on the command line (i.e. \"you\n> asked me to use reference, but for this submodule I couldn't find a\n> usable one\").\n\nNo it comes all the time now, as there is no difference between\n\"clone --recursive\" and later running \"submodule update\" manually.\n\nAssume a new submodule is added and you forget to update your\nmain alternate all your other clones are borrowing from. If there is\nno warning you create a real clone for the new submodule all the time,\nwhich is what you wanted to avoid in the first place?\n\n> If the implementation allows the same mechanism to\n> help later \"submodule init && submodule update\" borrow from the\n> submodule repositories of the superproject the current superproject\n> borrows from (i.e. no explicit \"--reference\" on the command line\n> when doing init/update), I would think the case that needs warning\n> is \"you didn't explicitly ask me to borrow objects, but I found one\n> we could, so I did it anyway without being asked\", and it is not a\n> warning-worthy condition if we didn't find a cloned submodule in the\n> repository our superproject borrows from.\n\n\"you did ask me to use alternates once and for all when setting up the\nsuperproject: now for this added submodule I don't find the alternate;\nThat is strange?\"\n\nThanks,\nStefan\n"},{"id":"293504","messageId":"CAGZ79kZj=M-kwZugL7Eo4Fk2t7i1AXTshkYSWKN0Rj=-F6qGHg@mail.gmail.com","threadId":"43031","inReplyTo":"xmqqy445c2sw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 2/9] t7408: merge short tests, factor out testing method","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T17:26:33Z","receivedAt":"2016-08-09T17:26:42Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 9:41 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> +# The tests up to this point, and repositories created by them\n>> +# (A, B, super and super/sub), are about setting up the stage\n>> +# forsubsequent tests and meant to be kept throughout the\n>\n> s/forsub/for sub/;\n\nok.\n"},{"id":"293505","messageId":"CAGZ79kZgEF2FBSLQnOm=v-a+jy=6aWyPBoLC9-QU=g-aUaqj3w@mail.gmail.com","threadId":"43031","inReplyTo":"xmqqbn12c55e.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 1/9] t7408: modernize style","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T17:30:47Z","receivedAt":"2016-08-09T17:30:53Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 8:51 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Eric Sunshine writes:\n>>\n\nI originally thought that it may be easier to have 2 patches.\nThis first one is very gentle and \"obviously correct\" as it only changes\nformatting and drops the directory changes.\n\nThe second patch goes for renaming ans subtle style issues,\ncombining some tests, so it is more likely to break.\n\nAfter this review, I consider using just one patch and do it all\nat once to not confuse the readers as otherwise I should reword\nthe commit message with the rationale of doing it in two patches.\n"},{"id":"293506","messageId":"xmqqd1lhc05n.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAGZ79kZgEF2FBSLQnOm=v-a+jy=6aWyPBoLC9-QU=g-aUaqj3w@mail.gmail.com","subject":"Re: [PATCHv3 1/9] t7408: modernize style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T17:39:00Z","receivedAt":"2016-08-09T17:39:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Aug 9, 2016 at 8:51 AM, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>>\n>>> Eric Sunshine writes:\n>>>\n>\n> I originally thought that it may be easier to have 2 patches.\n> This first one is very gentle and \"obviously correct\" as it only changes\n> formatting and drops the directory changes.\n>\n> The second patch goes for renaming ans subtle style issues,\n> combining some tests, so it is more likely to break.\n>\n> After this review, I consider using just one patch and do it all\n> at once to not confuse the readers as otherwise I should reword\n> the commit message with the rationale of doing it in two patches.\n\nFWIW, I would think your split between 1/ and 2/ were very sensible,\nand have a slight preference for keeping them separate.\n\nIf you already have squashed, I do not insist you to split it again;\nit is not a big deal either way.\n\n"},{"id":"293509","messageId":"xmqq8tw5bzs1.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAGZ79kZKTV5PCAR41O1t1c_y6N18u6gsoWozOfr=EPHic-7wYw@mail.gmail.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T17:47:10Z","receivedAt":"2016-08-09T17:47:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> \"you did ask me to use alternates once and for all when setting up the\n> superproject: now for this added submodule I don't find the alternate;\n> That is strange?\"\n\nAbsolutely. I do not think you should expect a user to remember if\ns/he used alternates when getting a copy of the superproject long\ntime ago. Giving \"info: using from ...\" is good; giving \"warning:\nnot using from ...\" is probably irritating, I would think.\n\nStepping back a bit, when the user says --reference-if-able, does it\nwarrant any warning either way? I.e. \"we made it borrow from there,\nso be careful not to trash that one\" may be just as warning-worthy\n(if not even more) as \"you said we can borrow from there if there is\nanything to borrow, but it turns out there isn't any, so the result\nis complete stand-alone.\" It feels as if we can go without any\nwarning at least from \"git clone --reference-if-able\", unless \"-v\"\noption is given.\n\nI have a very strong opinion what should happen when the end-user\nfacing command is \"git submodule\", but if I have to choose, I would\nprefer to see \"git submodule update\" tell what it did with \"info:\"\neither case, i.e. \"info: borrowing from ... because the superproject\nborrows from there\", and \"info: not borrowing from ... even though\nthe superproject borrows from there\".\n\n\n"},{"id":"293511","messageId":"xmqq4m6tbzfd.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"xmqq7fbpdhkw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 6/9] clone: implement optional references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T17:54:46Z","receivedAt":"2016-08-09T17:54:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> If I am reading the implementation of real_path_internal()\n> correctly, the most relevant reason that an \"if-able\" repository\n> cannot be used causes real_path_if_valid() to return NULL.\n\nOops, too many proofreading and rephrasing. Please insert \"I do not\nthink that\" before \"the most relevant\".\n"},{"id":"293512","messageId":"xmqqziolaknw.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"xmqq8tw5bzs1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T17:58:59Z","receivedAt":"2016-08-09T17:59:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Stefan Beller writes:\n>\n>> \"you did ask me to use alternates once and for all when setting up the\n>> superproject: now for this added submodule I don't find the alternate;\n>> That is strange?\"\n>\n> Absolutely. I do not think you should expect a user to remember if\n> s/he used alternates when getting a copy of the superproject long\n> time ago. Giving \"info: using from ...\" is good; giving \"warning:\n> not using from ...\" is probably irritating, I would think.\n>\n> Stepping back a bit, when the user says --reference-if-able, does it\n> warrant any warning either way? I.e. \"we made it borrow from there,\n> so be careful not to trash that one\" may be just as warning-worthy\n> (if not even more) as \"you said we can borrow from there if there is\n> anything to borrow, but it turns out there isn't any, so the result\n> is complete stand-alone.\" It feels as if we can go without any\n> warning at least from \"git clone --reference-if-able\", unless \"-v\"\n> option is given.\n>\n> I have a very strong opinion what should happen when the end-user\n\nToo many proof-reading and rephrasing. \"I DON'T have a very strong\nopinion\" is what I wanted to say. Sorry for the lack of last-time\nproof-reading.\n\n> facing command is \"git submodule\", but if I have to choose, I would\n> prefer to see \"git submodule update\" tell what it did with \"info:\"\n> either case, i.e. \"info: borrowing from ... because the superproject\n> borrows from there\", and \"info: not borrowing from ... even though\n> the superproject borrows from there\".\n"},{"id":"293514","messageId":"CAGZ79kY8EiGaugsh4FxKYp1FxqYr10JfGqsrfsnhULB+OBnFXw@mail.gmail.com","threadId":"43031","inReplyTo":"xmqq8tw5bzs1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T18:09:34Z","receivedAt":"2016-08-09T18:09:43Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 10:47 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> \"you did ask me to use alternates once and for all when setting up the\n>> superproject: now for this added submodule I don't find the alternate;\n>> That is strange?\"\n>\n> Absolutely. I do not think you should expect a user to remember if\n> s/he used alternates when getting a copy of the superproject long\n> time ago. Giving \"info: using from ...\" is good; giving \"warning:\n> not using from ...\" is probably irritating, I would think.\n>\n> Stepping back a bit, when the user says --reference-if-able, does it\n> warrant any warning either way?\n\nStepping another step back, let's discuss the expectations of the\nnew option \"--reference-if-able\".\n\nThe way I understood and implemented it is\n\n here is a path, try to use it as an alternate; if that is not\n an alternate, it's fine too; maybe warn about it, but carry\n on with the operation.\n\nWith such a semantics you can just add the --reference-if-able\nto the submodules that try to borrow from the other superprojects\nsubmodule.\n\n I expected the \"--reference-if-able\" not necessarily be\n used by the end user. It is a convenient way for our scripted\n use, as the -if-able is just a check if the path exists and nothing\n else.\n\n We could check if the git dir exists inthe helper for the\n submodule command, such that we only pass --reference\n as we are certain the alternate exists; we could have a switch\n in the helper --on-missing-submodule-alternate=[die,info,silent]\n\nThe way you write this response I think you have a slightly\ndifferent understanding of what the -if-able ought to do?\n\n When --reference is given, only the superproject should\n borrow and the -if-able, may allow submodules to also borrow?\n\n> I.e. \"we made it borrow from there,\n> so be careful not to trash that one\" may be just as warning-worthy\n> (if not even more) as \"you said we can borrow from there if there is\n> anything to borrow, but it turns out there isn't any, so the result\n> is complete stand-alone.\" It feels as if we can go without any\n> warning at least from \"git clone --reference-if-able\", unless \"-v\"\n> option is given.\n\nBut when git clone is not issueing a warning/info, who is responsible for\nthat? As you noted the superproject may be setup some time ago and\nthe user forgot they used references and want to use references for this\nnew submodule. So the helper would need to do that?\n\n>\n> I have a very strong opinion what should happen when the end-user\n> facing command is \"git submodule\", but if I have to choose, I would\n> prefer to see \"git submodule update\" tell what it did with \"info:\"\n> either case, i.e. \"info: borrowing from ... because the superproject\n> borrows from there\", and \"info: not borrowing from ... even though\n> the superproject borrows from there\".\n\nI see. This way the user is most informed. The current implementation\nof \"submodule update --init\" for start from scratch yields for example:\n\nSubmodule 'foo' () registered for path 'foo'\nSubmodule 'hooks' () registered for path 'hooks'\nCloning into '/home/sbeller/super/foo'...\nCloning into '/home/sbeller/super/hooks'...\nwarning: Not using proposed alternate\n/home/sbeller/super-reference/.git/modules/hooks/\nSubmodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'\nSubmodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'\n\nso before this series we had 3 lines per submodule, and with this series\nwe get a 4th line for the unusual case.\n\nYou would prefer to see always 4 lines per submodule?\nIs one extra line (25% more output) a reasonable tradeoff for the\nreference feature?\n\nI dunno; I guess we could argue either way.\n"},{"id":"293516","messageId":"CAGZ79kbS1_DULjf8gPram+zNLe-xRShETotq+at+r7bmWrUyxA@mail.gmail.com","threadId":"43031","inReplyTo":"xmqq4m6tbzfd.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 6/9] clone: implement optional references","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T18:20:04Z","receivedAt":"2016-08-09T18:20:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 10:54 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> If I am reading the implementation of real_path_internal()\n>> correctly, the most relevant reason that an \"if-able\" repository\n>> cannot be used causes real_path_if_valid() to return NULL.\n>\n> Oops, too many proofreading and rephrasing. Please insert \"I do not\n> think that\" before \"the most relevant\".\n\nSide note:\n It took me a while to mentally process and reread this.\n It would have been easier with just\n s/the most relevant reason/I do not think that the most relevant reason/\n but that is just me being used to s/ expressions by now.\n\nI think our emails nearly crossed. In the other thread I wondered if\nyou had different expectations on the -if-able part than I do.\n\nFor the expected use case I do expect that the missing path is the\nmost relevant thing (the submodule is not checked out).\n\nOf course the submodule may be cloned shallow as it was recommended\nin the .gitmodules file, so we should also care about that use case.\n\n\n> Let's say your superproject borrows from ~/w/super, whose submodule\n> repositories (if cloned) are directly in \"~/w/super/.git/modules/\".\n> When you clone a submodule X for your superproject, you allow it to\n> borrow from \"~/w/super/.git/modules/X\" if there is one available.\n\nwhich is why we need to pass in ..../X/ with an ending slash.\nSee the comment in 8/9.\n\n+ /*\n+ * We need to end the new path with '/' to mark it as a dir,\n+ * otherwise a submodule name containing '/' will be broken\n+ * as the last part of a missing submodule reference would\n+ * be taken as a file name.\n+ */\n+ strbuf_addf(&sb, \"modules/%s/\", sas->submodule_name);\n\nThinking about that we may want to not rely on such a hack,\nbut make it clearer.\n\nThanks,\nStefan\n"},{"id":"293521","messageId":"xmqqd1lhaiys.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAGZ79kbS1_DULjf8gPram+zNLe-xRShETotq+at+r7bmWrUyxA@mail.gmail.com","subject":"Re: [PATCHv3 6/9] clone: implement optional references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T18:35:39Z","receivedAt":"2016-08-09T18:35:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Thinking about that we may want to not rely on such a hack,\n> but make it clearer.\n\nBut that is far from sufficient, isn't it?\n\nYou'd need to bypass \"not a local repository\", \"shallow\" and \"is\ngrafted\" anyway, so in that sense, that hack is not doing much.\nIf modules/X (or modules/X/) does not exist, it won't pass these\nchecks, so burdening the readers with slash trickery is not really\nworth their time.\n\n\n\n"},{"id":"293526","messageId":"xmqq8tw5aijv.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAGZ79kY8EiGaugsh4FxKYp1FxqYr10JfGqsrfsnhULB+OBnFXw@mail.gmail.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T18:44:36Z","receivedAt":"2016-08-09T18:44:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> The way I understood and implemented it is\n>\n> here is a path, try to use it as an alternate; if that is not\n> an alternate, it's fine too; maybe warn about it, but carry\n> on with the operation.\n\nMy expectation is without \"maybe warn about it\". And not adding it\nas an alternate (because it is not usable) is just \"doing as I was\ntold to do\", nothing noteworthy. Because \"do it only if you can\" is\nan explicit instruction to the doer that the caller does not care\nabout the outcome, I'd think there shouldn't be any warning, whether\nit is used or not. At the same time, if the caller wants to know\nthe outcome, and using if-able as a way to say \"I prefer to see it\nhappen, but you do not have to panic if you can't\", I'd think it is\nOK to give \"info:\" to report which of the two possible paths was\ntaken in either case. Throwing a \"warning:\" only when it didn't do\nso does not make much sense to me.\n\n> The way you write this response I think you have a slightly\n> different understanding of what the -if-able ought to do?\n>\n> When --reference is given, only the superproject should\n> borrow and the -if-able, may allow submodules to also borrow?\n\nI have no idea what this sentence means.\n\n>> I have a very strong opinion what should happen when the end-user\n>> facing command is \"git submodule\", but if I have to choose, I would\n>> prefer to see \"git submodule update\" tell what it did with \"info:\"\n>> either case, i.e. \"info: borrowing from ... because the superproject\n>> borrows from there\", and \"info: not borrowing from ... even though\n>> the superproject borrows from there\".\n>\n> I see. This way the user is most informed.\n\nYes, and if we were to go that route, \"clone\" without \"-v\" should\nnot report which of the two it took. It is OK for \"submodule\" to\nlook at what \"clone\" left as the result and do more intelligent\nreporting that is better suited for the context of the command.\n\n> The current implementation\n> of \"submodule update --init\" for start from scratch yields for example:\n>\n> Submodule 'foo' () registered for path 'foo'\n> Submodule 'hooks' () registered for path 'hooks'\n> Cloning into '/home/sbeller/super/foo'...\n> Cloning into '/home/sbeller/super/hooks'...\n> warning: Not using proposed alternate\n> /home/sbeller/super-reference/.git/modules/hooks/\n> Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'\n> Submodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'\n>\n> so before this series we had 3 lines per submodule, and with this series\n> we get a 4th line for the unusual case.\n>\n> You would prefer to see always 4 lines per submodule?\n\nIf the user says \"--recursive --reference\", then the user probably\ndeserves to be notified. As I said (eh, rather, as I wanted to say\nbut failed to say so), I do not have a strong preference either way.\n\n"},{"id":"293539","messageId":"CAGZ79kbPvj6dU1DMaCh7ieJbs0qWimdHSD8xFfUyV98md2pywQ@mail.gmail.com","threadId":"43031","inReplyTo":"xmqq8tw5aijv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T20:31:31Z","receivedAt":"2016-08-09T20:31:52Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 11:44 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> The way I understood and implemented it is\n>>\n>> here is a path, try to use it as an alternate; if that is not\n>> an alternate, it's fine too; maybe warn about it, but carry\n>> on with the operation.\n>\n> My expectation is without \"maybe warn about it\". And not adding it\n> as an alternate (because it is not usable) is just \"doing as I was\n> told to do\", nothing noteworthy. Because \"do it only if you can\" is\n> an explicit instruction to the doer that the caller does not care\n> about the outcome, I'd think there shouldn't be any warning, whether\n> it is used or not. At the same time, if the caller wants to know\n> the outcome, and using if-able as a way to say \"I prefer to see it\n> happen, but you do not have to panic if you can't\", I'd think it is\n> OK to give \"info:\" to report which of the two possible paths was\n> taken in either case. Throwing a \"warning:\" only when it didn't do\n> so does not make much sense to me.\n\nI think this whole thing needs a new config variable to be set.\n\nAt the time of cloning you may run\n\n git clone --recursive --reference \nor\n git clone --recursive --reference-if-able \nor\n git clone --recursive\n\nThe first two should trickle down to the submodules, i.e.\nin the first case we maybe want to run\n\n git config submodule.alternate=required-superproject &&\n git submodule update --init --recurse\n\nIn the second case\n git config submodule.alternate=optional-superproject &&\n git submodule update --init --recurse\n\nAnd in the third case we maybe only want to record\n git config submodule.alternate=none # implicit by not setting it as well\n git submodule update --init --recurse\n\n\nThen later when we run\n git submodule update\nwe have this option to know if a submodule alternate should be treated\nas optional or required referenced as the existence\nof the superprojects alternate (as a boolean indicator) is not enough of\nan indicator what the user later wants.\n\n\n>>\n>> I see. This way the user is most informed.\n>\n> Yes, and if we were to go that route, \"clone\" without \"-v\" should\n> not report which of the two it took. It is OK for \"submodule\" to\n> look at what \"clone\" left as the result and do more intelligent\n> reporting that is better suited for the context of the command.\n>\n>> The current implementation\n>> of \"submodule update --init\" for start from scratch yields for example:\n>>\n>> Submodule 'foo' () registered for path 'foo'\n>> Submodule 'hooks' () registered for path 'hooks'\n>> Cloning into '/home/sbeller/super/foo'...\n>> Cloning into '/home/sbeller/super/hooks'...\n>> warning: Not using proposed alternate\n>> /home/sbeller/super-reference/.git/modules/hooks/\n>> Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'\n>> Submodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'\n>>\n>> so before this series we had 3 lines per submodule, and with this series\n>> we get a 4th line for the unusual case.\n>>\n>> You would prefer to see always 4 lines per submodule?\n>\n> If the user says \"--recursive --reference\", then the user probably\n> deserves to be notified. As I said (eh, rather, as I wanted to say\n> but failed to say so), I do not have a strong preference either way.\n\nWith a new config option we can trigger the notifications based on that as well.\n\n\"required superproject\" would die when the submodule alternate is not there,\nand the \"optional superproject\" would always state if it found an alternate.\n\nThanks,\nStefan\n"},{"id":"293542","messageId":"xmqqeg5x8vl4.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAGZ79kbPvj6dU1DMaCh7ieJbs0qWimdHSD8xFfUyV98md2pywQ@mail.gmail.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-09T21:45:59Z","receivedAt":"2016-08-09T21:46:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> At the time of cloning you may run\n>\n> git clone --recursive --reference \n> or\n> git clone --recursive --reference-if-able \n> or\n> git clone --recursive\n\nThat's an interesting tangent. I never meant \"if-able\" to be an\nend-user visible option [*1*], but now you mention it, I do not see\na reason why \"clone --reference-if-able\" of the top-level project\ncannot be used together with \"--recursive\".\n\n> Then later when we run\n> git submodule update\n> we have this option to know if a submodule alternate should be treated\n> as optional or required referenced as the existence\n> of the superprojects alternate (as a boolean indicator) is not enough of\n> an indicator what the user later wants.\n\nA tangent that comes to my mind after reading this is if letting\n\"if-able\" just skip (with or without warning) and forget about it\nonce a clone is made is what we really want.\n\nSuppose the \"other-super-project-location\" repository did not have a\nclone of a submodule when you create a new clone of the superproject\nusing it as a reference. The submodule will be made a full clone,\nbut after that happens, other-super-project-location can get\ninterested in the submodule and can acquire its own clone.\n\nAt that point, our clone of the submodule _could_ add the submodule\nin the other-super-project-location as its alternate and lose the\nduplicate objects that it could borrow from there by repacking, but\nthe suggested \"clone with if-able, and forget about it after a clone\nis made\" would not allow us to do this. I do not know if a\nreal-world use of submodules want the ability to do so, or it is\nunnecessary. I suspect with the recording of \"you were told that\nborrowing from the same location as the superproject is OK\", this\nbecomes easily doable (i.e. subsequent \"submodule update\" can realize\nthat the submodule does not have alternates but it could borrow from\nthe submodule in the other-super-project-location).\n\n\n[Footnote]\n\n*1* Rather, I meant: clone has a very intimate knowledge on what and\n what cannot be borrowed from and it is not just \"is there a\n directory?\", so \"git submodule update --init\" is not in a good\n position to decide if it wants to add --reference to the\n invocation of \"git clone\" it makes internally, and introducing\n an \"if-able\" variant to \"clone\" is one way to relieve it from\n having to make that decision.\n\n I could have suggested an alternative: because the submodule\n machinery is gettting moved to C the \"update --init\" code that\n drives the internal invocation of \"git clone\" could share the\n the logic in \"git clone --reference\" that determines if a local\n repository can be used as an alternate by small refactoring of\n builtin/clone.c::add_one_reference().\n"},{"id":"293543","messageId":"CAGZ79kYkn6eVUBd_hXd4FSxr+y-iNTSsOfqdcKWBWp-aaVpAkQ@mail.gmail.com","threadId":"43031","inReplyTo":"xmqqeg5x8vl4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T22:05:07Z","receivedAt":"2016-08-09T22:05:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 9, 2016 at 2:45 PM, Junio C Hamano wrote:\n> becomes easily doable (i.e. subsequent \"submodule update\" can realize\n> that the submodule does not have alternates but it could borrow from\n> the submodule in the other-super-project-location).\n\nI would suggest to postpone this to a later time once the need arises.\n\nI rather imagine that once you clone from \"other-super-project-location\"\nand get the warning about no alternates borrowing, you can decide if you want\nto set it up and link the submodule manually to that alternate, or if you just\ndon't care about this one repository being duplicated.\n\nFor now I plan to introduce 2 options, as these are 2 different things, that\ncan be extended independently of each other:\n\nsubmodule.alternateLocation::\n Specifies how the submodules obtain alternates when submodules are\n cloned. Possible values are `no`, `superproject`.\n By default `no` is assumed, which doesn't add references. When the\n value is set to `superproject` the submodule to be cloned computes\n its alternates location relative to the superprojects alternate.\n\nsubmodule.alternateErrorStrategy::\n Specifies how to treat errors with the alternates for a submodule\n as computed via `submodule.alternateLocation`. Possible values are\n `ignore`, `info`, `die`.\n\n> *1* Rather, I meant: clone has a very intimate knowledge on what and\n> what cannot be borrowed from and it is not just \"is there a\n> directory?\", so \"git submodule update --init\" is not in a good\n> position to decide if it wants to add --reference to the\n> invocation of \"git clone\" it makes internally, and introducing\n> an \"if-able\" variant to \"clone\" is one way to relieve it from\n> having to make that decision.\n\nThat is how I first understood the design as well.\n\n>\n> I could have suggested an alternative: because the submodule\n> machinery is gettting moved to C the \"update --init\" code that\n> drives the internal invocation of \"git clone\" could share the\n> the logic in \"git clone --reference\" that determines if a local\n> repository can be used as an alternate by small refactoring of\n> builtin/clone.c::add_one_reference().\n\nI see. I might actually need to do this anyway as the helper is a place\nto act on the submodule.alternateErrorStrategy.\n"},{"id":"293549","messageId":"CAPig+cR+3KWDK5r5YW-Ew0=0jvkGRM7r1SJA3O_7+SDSNmXmeQ@mail.gmail.com","threadId":"43031","inReplyTo":"xmqqd1lhc05n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 1/9] t7408: modernize style","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-08-09T23:06:31Z","receivedAt":"2016-08-09T23:06:36Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 9, 2016 at 1:39 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>> I originally thought that it may be easier to have 2 patches.\n>> This first one is very gentle and \"obviously correct\" as it only changes\n>> formatting and drops the directory changes.\n>>\n>> The second patch goes for renaming ans subtle style issues,\n>> combining some tests, so it is more likely to break.\n>>\n>> After this review, I consider using just one patch and do it all\n>> at once to not confuse the readers as otherwise I should reword\n>> the commit message with the rationale of doing it in two patches.\n>\n> FWIW, I would think your split between 1/ and 2/ were very sensible,\n> and have a slight preference for keeping them separate.\n\nThe review comment about renaming \"current\" to \"actual\" was made\nwithout having looked yet at patch 2. Having now seen patch 2, I agree\nwith Junio that the existing split is preferable.\n\nSo, only the review comment about dropping space after '>' remains relevant.\n"},{"id":"293643","messageId":"xmqqpopg7gyb.fsf@gitster.mtv.corp.google.com","threadId":"43031","inReplyTo":"CAGZ79kYkn6eVUBd_hXd4FSxr+y-iNTSsOfqdcKWBWp-aaVpAkQ@mail.gmail.com","subject":"Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-10T15:59:40Z","receivedAt":"2016-08-10T20:39:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Aug 9, 2016 at 2:45 PM, Junio C Hamano wrote:\n>> becomes easily doable (i.e. subsequent \"submodule update\" can realize\n>> that the submodule does not have alternates but it could borrow from\n>> the submodule in the other-super-project-location).\n>\n> I would suggest to postpone this to a later time once the need arises.\n\nOh, no question about that (did I even sound to be suggesting to do\nit now?). It is just one of the yardsticks I use when gauging if a\nproposed solution is sound to see how it will naturally scale and/or\nextend to other possible scenarios, and I was pointing out that this\nseems to pass the test.\n\n"}]}