{"thread":{"id":"43020","subject":"Re: [PATCHv2 2/6] t7408: merge short tests, factor out testing method","startedAt":"2016-08-06T20:00:36Z","lastAt":"2016-08-08T18:16:32Z","messageCount":12,"participants":["Junio C Hamano","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"293272","messageId":"xmqqshuhj0ea.fsf@gitster.mtv.corp.google.com","threadId":"43020","inReplyTo":"20160806012318.17968-3-sbeller@google.com","subject":"Re: [PATCHv2 2/6] t7408: merge short tests, factor out testing method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-06T17:02:53Z","receivedAt":"2016-08-06T20:00:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +test_alternate_usage() {\n\nAccording to Documentation/CodingGuidelines, this should be:\n\n    test_alternate_usage () {\n\nSomehow the helper name sounds as if it is testing if an alternate\nis used correctly (i.e. the machinery may attempt to use alternate\nbut not in a correct way), not testing if an alternate is correctly\nused (i.e. the machinery incorrectly forgets to use an alternate at\nall), but maybe it is just me.\n\n> +test_expect_success 'updating superproject keeps alternates' '\n> +\ttest_when_finished \"rm -rf super-clone\" &&\n\nThis one is new; we do not remove A, B or super.  According to the\nprevious round of review, this is a deliberate design, that needs to\nbe spelled out by having a comment block before this test so that\nother people who add more tests can understand why they need to\nclean when they follow suit.  Perhaps something like:\n\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 'updating superproject keeps alternates' '\n            test_when_finished \"rm -rf super-clone\" &&\n\n"},{"id":"293274","messageId":"xmqqoa55j0am.fsf@gitster.mtv.corp.google.com","threadId":"43020","inReplyTo":"20160806012318.17968-5-sbeller@google.com","subject":"Re: [PATCHv2 4/6] submodule--helper update-clone: allow multiple references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-06T17:05:05Z","receivedAt":"2016-08-06T20:04:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Allow the user to pass in multiple references to update_clone.\n> Currently this is only internal API, but once the shell script is\n> replaced by a C version, this is needed.\n>\n> This fixes an API bug between the shell script and the helper.\n> Currently the helper accepts \"--reference\" \"--reference=foo\"\n> as a OPT_STRING whose value happens to be \"--reference=foo\", and\n> then uses\n>\n>         if (suc->reference)\n>                 argv_array_push(&child->args, suc->reference)\n>\n> where suc->reference _is_ \"--reference=foo\" when invoking the\n> underlying \"git clone\", it cancels out.\n>\n> with this change we omit one of the \"--reference\" arguments when passing\n\ns/with this change/With this change/;\n\nAfter the \"We won't look at what is queued on 'pu'\", I am debating\nmyself if I should amend this locally.\n"},{"id":"293277","messageId":"xmqqd1lliz5k.fsf@gitster.mtv.corp.google.com","threadId":"43020","inReplyTo":"20160806012318.17968-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-06T17:29:43Z","receivedAt":"2016-08-06T20:05:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\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.\n\nPerhaps you can teach \"git clone --reference\" an new option\n(--reference-if-able) to do this?  Then \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 \nin which \"assumed\" is a horrible wording (leave the reader\nwondering: \"so what happens to my data when the assumption does not\nhold\") can become a lot more reasonable\n\n    When using --reference with --recursive, the --reference is used\n    to specify a repository that has a copy of the superproject.  If\n    that copy has submodules cloned for itself in its $GIT_DIR/modules,\n    they are used as --reference when cloning submodules in the\n    resulting clone.\n\nand readers expectation would match with the reality.  Their\nsubmodules would be cloned in a regular fashion if the central\nmirror does not have it, and would take advantage of it if there is\nalready a clone.\n\nCome to think of it, do we even need --super-reference?  \"git clone\n--reference --recursive\" is a two step process, in that first the\nsuperproject is cloned while creating objects/info/alternates, and\nthen submodules are cloned (via \"update --init\").  Can we make the\nprocedure to clone a submodule always look at the reference of the\nsuperproject (i.e. objects/info/alternates) and try to borrow from\nthe place in it that corresponds to the submodule?  That way, not\njust \"git clone --reference --recursive\" would take advantage of the\nexisting mirrors of submodules, a user who does this:\n\n    $ git clone --reference $URL super\n    $ cd super\n    $ git submodule update --init ...\n\nwould be able to take advantage of the \"what the mirror the\nsuperproject uses already has\" when cloning the submodules, no?\n"},{"id":"293279","messageId":"xmqqh9axizw9.fsf@gitster.mtv.corp.google.com","threadId":"43020","inReplyTo":"20160806012318.17968-6-sbeller@google.com","subject":"Re: [PATCHv2 5/6] submodule update: add super-reference flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-06T17:13:42Z","receivedAt":"2016-08-06T20:10:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> When we have a another clone of a superproject, we may want to\n> mirror the submodules using alternates. Introduce an option\n> `--super-reference` that let's a user point to another superproject,\n> which is assumed to have the same structure as the one they are\n> running the \"submodule update\" command from and has all submodules\n> checked out to borrow the submodule objects from within the other\n> superprojects git directory.\n\nThat's much better than the previous round.\n\nI however have trouble with the \"checked out\", though.  Isn't it\nthat it merely has to be \"init\"ed?  For that matter, as long as\nsuper.git has $GIT_DIR/modules/ populated fully, it does not matter\nit has checkout, and more interestingly and importantly, the\nsuperproject mirror can even be a bare repository!\n\n> +--super-reference <superproject repository>::\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\"points at the submodule path inside\" sounds as if we would look at\n\n    /var/cache/super/libs/xyzzy\n\nin the scenario in <xmqqoa57vvzl.fsf@gitster.mtv.corp.google.com>, \nwhen you give us \"--super-reference=/var/cache/super\".  Can we\nclarify to avoid such a misread?\n\n> +superreference=\n\nPlease don't name a multiple_word field \"multipleword\".\n\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"},{"id":"293282","messageId":"20160806012318.17968-3-sbeller@google.com","threadId":"43020","inReplyTo":"20160806012318.17968-1-sbeller@google.com","subject":"[PATCHv2 2/6] t7408: merge short tests, factor out testing method","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-06T01:23:14Z","receivedAt":"2016-08-06T20:11:41Z","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 <sbeller@google.com>\n---\n t/t7408-submodule-reference.sh | 49 +++++++++++++++---------------------------\n 1 file changed, 17 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\nindex beee0bb..1d9326e 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_usage() {\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,20 @@ 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-'\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+\t) &&\n+\ttest_alternate_usage super/.git/modules/sub/objects/info/alternates super/sub\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+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_usage super-clone/.git/modules/sub/objects/info/alternates super-clone/sub\n '\n \n test_done\n-- \n2.9.2.572.g9d9644e.dirty\n\n"},{"id":"293286","messageId":"20160806012318.17968-1-sbeller@google.com","threadId":"43020","inReplyTo":null,"subject":"[PATCHv2 0/6] git clone: Marry --recursive and --reference","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-06T01:23:12Z","receivedAt":"2016-08-06T20:18:57Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\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":"293288","messageId":"20160806012318.17968-4-sbeller@google.com","threadId":"43020","inReplyTo":"20160806012318.17968-1-sbeller@google.com","subject":"[PATCHv2 3/6] submodule--helper module-clone: allow multiple references","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-06T01:23:15Z","receivedAt":"2016-08-06T20:21:03Z","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 <sbeller@google.com>\n---\n builtin/submodule--helper.c | 19 +++++++++++--------\n 1 file changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6f6d67a..f2b19ea 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,11 @@ 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\", 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 +473,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 +494,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 +531,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.572.g9d9644e.dirty\n\n"},{"id":"293289","messageId":"20160806012318.17968-5-sbeller@google.com","threadId":"43020","inReplyTo":"20160806012318.17968-1-sbeller@google.com","subject":"[PATCHv2 4/6] submodule--helper update-clone: allow multiple references","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-06T01:23:16Z","receivedAt":"2016-08-06T20:21:47Z","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 passing\nreferences from the shell script to the helper.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 f2b19ea..b7710a7 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -583,7 +583,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@@ -599,7 +599,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@@ -709,8 +710,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@@ -829,7 +833,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, \"<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.572.g9d9644e.dirty\n\n"},{"id":"293291","messageId":"20160806012318.17968-2-sbeller@google.com","threadId":"43020","inReplyTo":"20160806012318.17968-1-sbeller@google.com","subject":"[PATCHv2 1/6] t7408: modernize style","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-06T01:23:13Z","receivedAt":"2016-08-06T20:25:13Z","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 <sbeller@google.com>\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.572.g9d9644e.dirty\n\n"},{"id":"293295","messageId":"20160806012318.17968-7-sbeller@google.com","threadId":"43020","inReplyTo":"20160806012318.17968-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-06T01:23:18Z","receivedAt":"2016-08-06T20:40:29Z","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 <sbeller@google.com>\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":"293307","messageId":"20160806012318.17968-6-sbeller@google.com","threadId":"43020","inReplyTo":"20160806012318.17968-1-sbeller@google.com","subject":"[PATCHv2 5/6] submodule update: add super-reference flag","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-06T01:23:17Z","receivedAt":"2016-08-06T22:33:33Z","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 <sbeller@google.com>\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|[--] <path>...)\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t      [--[no-]recommend-shallow] [-f|--force] [--rebase|--merge]\n-\t      [--reference <repository>] [--depth <depth>] [--recursive]\n+\t      [--[super-]reference <repository>] [--depth <depth>] [--recursive]\n \t      [--jobs <n>] [--] [<path>...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]\n \t      [commit] [--] [<path>...]\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 <superproject repository>::\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, \"<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":"293375","messageId":"CAGZ79kYryZkMNj3_OLBU_fq-wmc+6a3ZmNSXrwVBB1jAz8Sc0Q@mail.gmail.com","threadId":"43020","inReplyTo":"xmqqd1lliz5k.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-08T18:16:26Z","receivedAt":"2016-08-08T18:16:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Aug 6, 2016 at 10:29 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\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.\n>\n> Perhaps you can teach \"git clone --reference\" an new option\n> (--reference-if-able) to do this?  Then\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> in which \"assumed\" is a horrible wording (leave the reader\n> wondering: \"so what happens to my data when the assumption does not\n> hold\") can become a lot more reasonable\n>\n>     When using --reference with --recursive, the --reference is used\n>     to specify a repository that has a copy of the superproject.  If\n>     that copy has submodules cloned for itself in its $GIT_DIR/modules,\n>     they are used as --reference when cloning submodules in the\n>     resulting clone.\n>\n> and readers expectation would match with the reality.  Their\n> submodules would be cloned in a regular fashion if the central\n> mirror does not have it, and would take advantage of it if there is\n> already a clone.\n\nThat makes sense.\n\n>\n> Come to think of it, do we even need --super-reference?  \"git clone\n> --reference --recursive\" is a two step process, in that first the\n> superproject is cloned while creating objects/info/alternates, and\n> then submodules are cloned (via \"update --init\").  Can we make the\n> procedure to clone a submodule always look at the reference of the\n> superproject (i.e. objects/info/alternates) and try to borrow from\n> the place in it that corresponds to the submodule?  That way, not\n> just \"git clone --reference --recursive\" would take advantage of the\n> existing mirrors of submodules, a user who does this:\n>\n>     $ git clone --reference $URL super\n>     $ cd super\n>     $ git submodule update --init ...\n>\n> would be able to take advantage of the \"what the mirror the\n> superproject uses already has\" when cloning the submodules, no?\n\nThat would work, too.\n\nI'll implement that.\n\nThanks,\nStefan\n"}]}