{"thread":{"id":"42009","subject":"[PATCH 2/3] clone: add `--shallow-submodules` flag","startedAt":"2016-04-12T23:48:46Z","lastAt":"2016-04-25T17:41:48Z","messageCount":9,"participants":["Stefan Beller","Jacob Keller","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"283255","messageId":"1460504929-19208-1-git-send-email-sbeller@google.com","threadId":"42009","inReplyTo":null,"subject":"[PATCH 0/3] clone --shallow-submodules flag","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-12T23:48:46Z","receivedAt":"2016-04-12T23:48:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"(This is a resend of a series from March 15th, titled \n\"Towards sane shallow clones with submodules\", this series\napplies on top of sb/submodule-parallel-update,\nit replaces sb/clone-shallow-passthru)\n\nWhen creating a shallow clone of a repository with submodules, the depth\nargument does not influence the submodules, i.e. the submodules are done\nas non-shallow clones. It is unclear what the best default is for the\ndepth of submodules of a shallow clone, so we need to have the possibility\nto do all kinds of combinations:\n\n* shallow super project with shallow submodules\n e.g. build bots starting always from scratch. They want to transmit\n the least amount of network data as well as using the least amount\n of space on their hard drive.\n* shallow super project with unshallow submodules\n e.g. The superproject is just there to track a collection of repositories\n and it is not important to have the relationship between the repositories\n intact. However the history of the individual submodules matter.\n* unshallow super project with shallow submodules\n e.g. The superproject is the actual project and the submodule is a\n library which is rarely touched.\n\nThe new switch to select submodules to be shallow or unshallow supports\nall of these three cases.\n\nChanges to the previous series:\n * Reordered patch 1 and 2, as it sounds like a more natural ordering.\n \nAny comments welcome!\nThanks,\nStefan\n\nStefan Beller (3):\n clone: add `--shallow-submodules` flag\n submodule clone: pass along `local` option\n clone: Add t5614 to test cloning submodules with shallowness involved\n\n Documentation/git-clone.txt | 13 +++++--\n builtin/clone.c | 21 ++++++++++++\n builtin/submodule--helper.c | 22 ++++++++++--\n git-submodule.sh | 7 ++++\n t/t5614-clone-submodules.sh | 82 +++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 139 insertions(+), 6 deletions(-)\n create mode 100755 t/t5614-clone-submodules.sh\n\n-- \n2.5.0.264.gc776916.dirty\n"},{"id":"283253","messageId":"1460504929-19208-2-git-send-email-sbeller@google.com","threadId":"42009","inReplyTo":"1460504929-19208-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/3] submodule clone: pass along `local` option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-12T23:48:47Z","receivedAt":"2016-04-12T23:48:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When cloning a local repository, the user may choose to use an optimization\nsuch that the transfer uses a Git agnostic protocol. Propagate the users\nchoice to submodules or if they don't choose, propagate nothing.\nA test will be added in a later patch.\n\nSigned-off-by: Stefan Beller \n---\n builtin/clone.c | 14 ++++++++++++++\n builtin/submodule--helper.c | 22 +++++++++++++++++++---\n git-submodule.sh | 7 +++++++\n 3 files changed, 40 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b004fb4..7453244 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -727,6 +727,20 @@ static int checkout(void)\n \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n \t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n \n+\t\tswitch (option_local) {\n+\t\tcase 1:\n+\t\t\targv_array_push(&args, \"--local\");\n+\t\t\tbreak;\n+\t\tcase 0:\n+\t\t\targv_array_push(&args, \"--no-local\");\n+\t\t\tbreak;\n+\t\tcase -1:\n+\t\t\t/* pass nothing */\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tdie(\"BUG: option_local out of range\");\n+\t\t}\n+\n \t\tif (max_jobs != -1)\n \t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a484945..822ec69 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -125,7 +125,8 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\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, const char *reference, int quiet,\n+\t\t\t int local)\n {\n \tstruct child_process cp;\n \tchild_process_init(&cp);\n@@ -140,6 +141,10 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \t\targv_array_pushl(&cp.args, \"--reference\", reference, NULL);\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n+\tif (local == 1)\n+\t\targv_array_push(&cp.args, \"--local\");\n+\telse if (!local)\n+\t\targv_array_push(&cp.args, \"--no-local\");\n \n \targv_array_push(&cp.args, url);\n \targv_array_push(&cp.args, path);\n@@ -156,6 +161,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tconst char *path = NULL, *name = NULL, *url = NULL;\n \tconst char *reference = NULL, *depth = NULL;\n \tint quiet = 0;\n+\tint local = -1;\n \tFILE *submodule_dot_git;\n \tchar *sm_gitdir, *cwd, *p;\n \tstruct strbuf rel_path = STRBUF_INIT;\n@@ -180,6 +186,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &depth,\n \t\t\t N_(\"string\"),\n \t\t\t N_(\"depth for shallow clones\")),\n+\t\tOPT_BOOL(0, \"local\", &local,\n+\t\t\t N_(\"to clone from a local repository\")),\n \t\tOPT__QUIET(&quiet, \"Suppress output for cloning a submodule\"),\n \t\tOPT_END()\n \t};\n@@ -200,7 +208,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\t\t quiet, local))\n \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n \t\t\t url, path);\n \t} else {\n@@ -266,6 +275,7 @@ struct submodule_update_clone {\n \n \t/* configuration parameters which are passed on to the children */\n \tint quiet;\n+\tint local;\n \tconst char *reference;\n \tconst char *depth;\n \tconst char *recursive_prefix;\n@@ -278,7 +288,7 @@ struct submodule_update_clone {\n \tunsigned quickstop : 1;\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n-\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n+\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \\\n \tSTRING_LIST_INIT_DUP, 0}\n \n /**\n@@ -367,6 +377,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tchild->err = -1;\n \targv_array_push(&child->args, \"submodule--helper\");\n \targv_array_push(&child->args, \"clone\");\n+\tif (suc->local == 1)\n+\t\targv_array_push(&child->args, \"--local\");\n+\telse if (!suc->local)\n+\t\targv_array_push(&child->args, \"--no-local\");\n \tif (suc->quiet)\n \t\targv_array_push(&child->args, \"--quiet\");\n \tif (suc->prefix)\n@@ -451,6 +465,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\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\")),\n+\t\tOPT_BOOL(0, \"local\", &suc.local,\n+\t\t\t N_(\"to clone from a local repository\")),\n \t\tOPT_INTEGER('j', \"jobs\", &max_jobs,\n \t\t\t N_(\"parallel jobs\")),\n \t\tOPT__QUIET(&suc.quiet, N_(\"don't print cloning progress\")),\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 86018ee..4d5e8c7 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -653,6 +653,12 @@ cmd_update()\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n+\t\t--local)\n+\t\t\toption_local=\"--local\"\n+\t\t\t;;\n+\t\t--no-local)\n+\t\t\toption_local=\"--no-local\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -680,6 +686,7 @@ cmd_update()\n \t\t${reference:+--reference \"$reference\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n \t\t${jobs:+$jobs} \\\n+\t\t${option_local:+$option_local} \\\n \t\t\"$@\" || echo \"#unmatched\"\n \t} | {\n \terr=\n-- \n2.5.0.264.gc776916.dirty\n"},{"id":"283252","messageId":"1460504929-19208-3-git-send-email-sbeller@google.com","threadId":"42009","inReplyTo":"1460504929-19208-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/3] clone: add `--shallow-submodules` flag","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-12T23:48:48Z","receivedAt":"2016-04-12T23:48:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When creating a shallow clone of a repository with submodules, the depth\nargument does not influence the submodules, i.e. the submodules are done\nas non-shallow clones. It is unclear what the best default is for the\ndepth of submodules of a shallow clone, so we need to have the possibility\nto do all kinds of combinations:\n\n* shallow super project with shallow submodules\n e.g. build bots starting always from scratch. They want to transmit\n the least amount of network data as well as using the least amount\n of space on their hard drive.\n* shallow super project with unshallow submodules\n e.g. The superproject is just there to track a collection of repositories\n and it is not important to have the relationship between the repositories\n intact. However the history of the individual submodules matter.\n* unshallow super project with shallow submodules\n e.g. The superproject is the actual project and the submodule is a\n library which is rarely touched.\n\nThe new switch to select submodules to be shallow or unshallow supports\nall of these three cases.\n\nIt is easy to transition from the first to the second case by just\nunshallowing the submodules (`git submodule foreach git fetch\n--unshallow`), but it is not possible to transition from the second to the\nfirst case (as we wouldd have already transmitted the non shallow over\nthe network). That is why we want to make the first case the default in\ncase of a shallow super project. This leads to the inconvenience in the\nsecond case with the shallow super project and unshallow submodules,\nas you need to pass `--no-shallow-submodules`.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-clone.txt | 13 ++++++++++---\n builtin/clone.c | 7 +++++++\n 2 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 6db7b6d..20a4577 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -14,8 +14,8 @@ SYNOPSIS\n \t [-o ] [-b ] [-u ] [--reference ]\n \t [--dissociate] [--separate-git-dir ]\n \t [--depth ] [--[no-]single-branch]\n-\t [--recursive | --recurse-submodules] [--jobs ] [--] \n-\t []\n+\t [--recursive | --recurse-submodules] [--[no-]shallow-submodules]\n+\t [--jobs ] [--] []\n \n DESCRIPTION\n -----------\n@@ -190,7 +190,11 @@ objects from the source repository into a pack in the cloned repository.\n \n --depth ::\n \tCreate a 'shallow' clone with a history truncated to the\n-\tspecified number of revisions.\n+\tspecified number of revisions. Implies `--single-branch` unless\n+\t`--no-single-branch` is given to fetch the histories near the\n+\ttips of all branches. This implies `--shallow-submodules`. If\n+\tyou want to have a shallow clone, but full submodules, also pass\n+\t`--no-shallow-submodules`.\n \n --[no-]single-branch::\n \tClone only the history leading to the tip of a single branch,\n@@ -214,6 +218,9 @@ objects from the source repository into a pack in the cloned repository.\n \trepository does not have a worktree/checkout (i.e. if any of\n \t`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)\n \n+--shallow-submodules::\n+\tAll submodules which are cloned, will be shallow.\n+\n --separate-git-dir=::\n \tInstead of placing the cloned repository where it is supposed\n \tto be, place the cloned repository at the specified directory,\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 7453244..bd1d598 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -40,6 +40,7 @@ static const char * const builtin_clone_usage[] = {\n \n static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n+static int option_shallow_submodules = -1;\n static char *option_template, *option_depth;\n static char *option_origin = NULL;\n static char *option_branch = NULL;\n@@ -91,6 +92,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"create a shallow clone of that depth\")),\n \tOPT_BOOL(0, \"single-branch\", &option_single_branch,\n \t\t N_(\"clone only one branch, HEAD or --branch\")),\n+\tOPT_BOOL(0, \"shallow-submodules\", &option_shallow_submodules,\n+\t\t N_(\"any cloned submodules will be shallow\")),\n \tOPT_STRING(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n \t\t N_(\"separate git dir from working tree\")),\n \tOPT_STRING_LIST('c', \"config\", &option_config, N_(\"key=value\"),\n@@ -741,6 +744,10 @@ static int checkout(void)\n \t\t\tdie(\"BUG: option_local out of range\");\n \t\t}\n \n+\t\tif (option_shallow_submodules == 1\n+\t\t || (option_shallow_submodules == -1 && option_depth))\n+\t\t\targv_array_push(&args, \"--depth=1\");\n+\n \t\tif (max_jobs != -1)\n \t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n \n-- \n2.5.0.264.gc776916.dirty\n"},{"id":"283254","messageId":"1460504929-19208-4-git-send-email-sbeller@google.com","threadId":"42009","inReplyTo":"1460504929-19208-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/3] clone: add t5614 to test cloning submodules with shallowness involved","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-12T23:48:49Z","receivedAt":"2016-04-12T23:48:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"There are some inherent issues with shallow clones and submodules, such\nas having not having a commit available the superproject may point to\nin the submodule due to being shallow. Use the new file t5614 to document\nand test expectations in this area.\n\nSigned-off-by: Stefan Beller \n---\n t/t5614-clone-submodules.sh | 82 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 82 insertions(+)\n create mode 100755 t/t5614-clone-submodules.sh\n\ndiff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh\nnew file mode 100755\nindex 0000000..a66c2db\n--- /dev/null\n+++ b/t/t5614-clone-submodules.sh\n@@ -0,0 +1,82 @@\n+#!/bin/sh\n+\n+test_description='Test shallow cloning of repos with submodules'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tgit checkout -b master &&\n+\ttest_commit commit1 &&\n+\ttest_commit commit2 &&\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit init &&\n+\t\ttest_commit subcommit1 &&\n+\t\ttest_commit subcommit2\n+\t) &&\n+\tgit submodule add ./sub &&\n+\tgit commit -m \"add submodule\"\n+'\n+\n+test_expect_success 'nonshallow clone implies nonshallow submodule' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules . super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 3 lines\n+\t) &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 2 lines\n+\t)\n+'\n+\n+test_expect_success 'shallow clone implies shallow submodule' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --no-local --depth 1 . super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 1 lines\n+\t) &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 1 lines\n+\t)\n+'\n+\n+test_expect_success 'shallow clone with non shallow submodule' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --no-local --depth 1 --no-shallow-submodules . super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 1 lines\n+\t) &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 2 lines\n+\t)\n+'\n+\n+test_expect_success 'non shallow clone with shallow submodule' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --no-local --shallow-submodules . super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 3 lines\n+\t) &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 1 lines\n+\t)\n+'\n+\n+test_done\n-- \n2.5.0.264.gc776916.dirty\n"},{"id":"283299","messageId":"CA+P7+xppaK9d77h74j-4K=oWftdkeJE7048+PMzLS6gBS3E_uQ@mail.gmail.com","threadId":"42009","inReplyTo":"1460504929-19208-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/3] clone: add t5614 to test cloning submodules with shallowness involved","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-13T06:52:32Z","receivedAt":"2016-04-13T06:52:32Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Apr 12, 2016 at 4:48 PM, Stefan Beller wrote:\n> There are some inherent issues with shallow clones and submodules, such\n> as having not having a commit available the superproject may point to\n> in the submodule due to being shallow. Use the new file t5614 to document\n> and test expectations in this area.\n>\n\nThe description seems to imply that there will be\n\"test_expect_failure\" tests to indicate what needs to be improved...\nMaybe I am just mis-reading it?\n\nThanks,\nJake\n\n> Signed-off-by: Stefan Beller \n> ---\n> t/t5614-clone-submodules.sh | 82 +++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 82 insertions(+)\n> create mode 100755 t/t5614-clone-submodules.sh\n>\n> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh\n> new file mode 100755\n> index 0000000..a66c2db\n> --- /dev/null\n> +++ b/t/t5614-clone-submodules.sh\n> @@ -0,0 +1,82 @@\n> +#!/bin/sh\n> +\n> +test_description='Test shallow cloning of repos with submodules'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> + git checkout -b master &&\n> + test_commit commit1 &&\n> + test_commit commit2 &&\n> + mkdir sub &&\n> + (\n> + cd sub &&\n> + git init &&\n> + test_commit subcommit1 &&\n> + test_commit subcommit2\n> + ) &&\n> + git submodule add ./sub &&\n> + git commit -m \"add submodule\"\n> +'\n> +\n> +test_expect_success 'nonshallow clone implies nonshallow submodule' '\n> + test_when_finished \"rm -rf super_clone\" &&\n> + git clone --recurse-submodules . super_clone &&\n> + (\n> + cd super_clone &&\n> + git log --oneline >lines &&\n> + test_line_count = 3 lines\n> + ) &&\n> + (\n> + cd super_clone/sub &&\n> + git log --oneline >lines &&\n> + test_line_count = 2 lines\n> + )\n> +'\n> +\n> +test_expect_success 'shallow clone implies shallow submodule' '\n> + test_when_finished \"rm -rf super_clone\" &&\n> + git clone --recurse-submodules --no-local --depth 1 . super_clone &&\n> + (\n> + cd super_clone &&\n> + git log --oneline >lines &&\n> + test_line_count = 1 lines\n> + ) &&\n> + (\n> + cd super_clone/sub &&\n> + git log --oneline >lines &&\n> + test_line_count = 1 lines\n> + )\n> +'\n> +\n> +test_expect_success 'shallow clone with non shallow submodule' '\n> + test_when_finished \"rm -rf super_clone\" &&\n> + git clone --recurse-submodules --no-local --depth 1 --no-shallow-submodules . super_clone &&\n> + (\n> + cd super_clone &&\n> + git log --oneline >lines &&\n> + test_line_count = 1 lines\n> + ) &&\n> + (\n> + cd super_clone/sub &&\n> + git log --oneline >lines &&\n> + test_line_count = 2 lines\n> + )\n> +'\n> +\n> +test_expect_success 'non shallow clone with shallow submodule' '\n> + test_when_finished \"rm -rf super_clone\" &&\n> + git clone --recurse-submodules --no-local --shallow-submodules . super_clone &&\n> + (\n> + cd super_clone &&\n> + git log --oneline >lines &&\n> + test_line_count = 3 lines\n> + ) &&\n> + (\n> + cd super_clone/sub &&\n> + git log --oneline >lines &&\n> + test_line_count = 1 lines\n> + )\n> +'\n> +\n> +test_done\n> --\n> 2.5.0.264.gc776916.dirty\n>\n"},{"id":"284394","messageId":"65302315-2C33-4801-887C-9AF3CB2F6BB6@gmail.com","threadId":"42009","inReplyTo":"1460504929-19208-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/3] submodule clone: pass along `local` option","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-25T12:18:42Z","receivedAt":"2016-04-25T12:18:42Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 13 Apr 2016, at 01:48, Stefan Beller wrote:\n> \n> When cloning a local repository, the user may choose to use an optimization\n> such that the transfer uses a Git agnostic protocol. Propagate the users\n> choice to submodules or if they don't choose, propagate nothing.\n> A test will be added in a later patch.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> builtin/clone.c | 14 ++++++++++++++\n> builtin/submodule--helper.c | 22 +++++++++++++++++++---\n> git-submodule.sh | 7 +++++++\n> 3 files changed, 40 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index b004fb4..7453244 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -727,6 +727,20 @@ static int checkout(void)\n> \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n> \t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n> \n> +\t\tswitch (option_local) {\n> +\t\tcase 1:\n> +\t\t\targv_array_push(&args, \"--local\");\n> +\t\t\tbreak;\n> +\t\tcase 0:\n> +\t\t\targv_array_push(&args, \"--no-local\");\n> +\t\t\tbreak;\n> +\t\tcase -1:\n> +\t\t\t/* pass nothing */\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tdie(\"BUG: option_local out of range\");\n> +\t\t}\n> +\n> \t\tif (max_jobs != -1)\n> \t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index a484945..822ec69 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -125,7 +125,8 @@ static int module_name(int argc, const char **argv, const char *prefix)\n> \treturn 0;\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, const char *reference, int quiet,\n> +\t\t\t int local)\n> {\n> \tstruct child_process cp;\n> \tchild_process_init(&cp);\n> @@ -140,6 +141,10 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n> \t\targv_array_pushl(&cp.args, \"--reference\", reference, NULL);\n> \tif (gitdir && *gitdir)\n> \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n> +\tif (local == 1)\nbikeshedding: Why do you use == 1 here and !local below? I would either compare both against\nintegers or none (\"if (local)\" should work here, too, no?). Or is this a Git coding guideline\nthat I am not yet aware of?\n\n\n> +\t\targv_array_push(&cp.args, \"--local\");\n> +\telse if (!local)\n> +\t\targv_array_push(&cp.args, \"--no-local\");\n> \n> \targv_array_push(&cp.args, url);\n> \targv_array_push(&cp.args, path);\n> @@ -156,6 +161,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \tconst char *path = NULL, *name = NULL, *url = NULL;\n> \tconst char *reference = NULL, *depth = NULL;\n> \tint quiet = 0;\n> +\tint local = -1;\n> \tFILE *submodule_dot_git;\n> \tchar *sm_gitdir, *cwd, *p;\n> \tstruct strbuf rel_path = STRBUF_INIT;\n> @@ -180,6 +186,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\tOPT_STRING(0, \"depth\", &depth,\n> \t\t\t N_(\"string\"),\n> \t\t\t N_(\"depth for shallow clones\")),\n> +\t\tOPT_BOOL(0, \"local\", &local,\n> +\t\t\t N_(\"to clone from a local repository\")),\nTBH I think \"local\" could be misleading here. How about \"--pass-transfer-protocol-on-to-submodules\" \nor something? If I understand this option correctly then this could be useful for other cases besides\n\"local\". Imagine you clone a repo via HTTPS that has references to SSH submodules on the same\nserver. If you don't have a proper SSH setup then the submodule clone will fail.\n\n\n> \t\tOPT__QUIET(&quiet, \"Suppress output for cloning a submodule\"),\n> \t\tOPT_END()\n> \t};\n> @@ -200,7 +208,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\t\t quiet, local))\n> \t\t\tdie(_(\"clone of '%s' into submodule path '%s' failed\"),\n> \t\t\t url, path);\n> \t} else {\n> @@ -266,6 +275,7 @@ struct submodule_update_clone {\n> \n> \t/* configuration parameters which are passed on to the children */\n> \tint quiet;\n> +\tint local;\n> \tconst char *reference;\n> \tconst char *depth;\n> \tconst char *recursive_prefix;\n> @@ -278,7 +288,7 @@ struct submodule_update_clone {\n> \tunsigned quickstop : 1;\n> };\n> #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n> -\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n> +\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \\\n> \tSTRING_LIST_INIT_DUP, 0}\n> \n> /**\n> @@ -367,6 +377,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n> \tchild->err = -1;\n> \targv_array_push(&child->args, \"submodule--helper\");\n> \targv_array_push(&child->args, \"clone\");\n> +\tif (suc->local == 1)\n> +\t\targv_array_push(&child->args, \"--local\");\n> +\telse if (!suc->local)\n> +\t\targv_array_push(&child->args, \"--no-local\");\n> \tif (suc->quiet)\n> \t\targv_array_push(&child->args, \"--quiet\");\n> \tif (suc->prefix)\n> @@ -451,6 +465,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\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\")),\n> +\t\tOPT_BOOL(0, \"local\", &suc.local,\n> +\t\t\t N_(\"to clone from a local repository\")),\nsame as above\n\n\n> \t\tOPT_INTEGER('j', \"jobs\", &max_jobs,\n> \t\t\t N_(\"parallel jobs\")),\n> \t\tOPT__QUIET(&suc.quiet, N_(\"don't print cloning progress\")),\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 86018ee..4d5e8c7 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -653,6 +653,12 @@ cmd_update()\n> \t\t--jobs=*)\n> \t\t\tjobs=$1\n> \t\t\t;;\n> +\t\t--local)\n> +\t\t\toption_local=\"--local\"\n> +\t\t\t;;\n> +\t\t--no-local)\n> +\t\t\toption_local=\"--no-local\"\n> +\t\t\t;;\n> \t\t--)\n> \t\t\tshift\n> \t\t\tbreak\n> @@ -680,6 +686,7 @@ cmd_update()\n> \t\t${reference:+--reference \"$reference\"} \\\n> \t\t${depth:+--depth \"$depth\"} \\\n> \t\t${jobs:+$jobs} \\\n> +\t\t${option_local:+$option_local} \\\n> \t\t\"$@\" || echo \"#unmatched\"\n> \t} | {\n> \terr=\n> -- \n> 2.5.0.264.gc776916.dirty\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"284395","messageId":"8AF410FF-23B9-41C9-A0B9-0086290A272F@gmail.com","threadId":"42009","inReplyTo":"1460504929-19208-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/3] clone: add t5614 to test cloning submodules with shallowness involved","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-25T12:25:44Z","receivedAt":"2016-04-25T12:25:44Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 13 Apr 2016, at 01:48, Stefan Beller wrote:\n> \n> There are some inherent issues with shallow clones and submodules, such\n> as having not having a commit available the superproject may point to\n> in the submodule due to being shallow. Use the new file t5614 to document\n> and test expectations in this area.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> t/t5614-clone-submodules.sh | 82 +++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 82 insertions(+)\n> create mode 100755 t/t5614-clone-submodules.sh\n> \n> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh\n> new file mode 100755\n> index 0000000..a66c2db\n> --- /dev/null\n> +++ b/t/t5614-clone-submodules.sh\n> @@ -0,0 +1,82 @@\n> +#!/bin/sh\n> +\n> +test_description='Test shallow cloning of repos with submodules'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\tgit checkout -b master &&\n> +\ttest_commit commit1 &&\n> +\ttest_commit commit2 &&\n> +\tmkdir sub &&\n> +\t(\n> +\t\tcd sub &&\n> +\t\tgit init &&\n> +\t\ttest_commit subcommit1 &&\n> +\t\ttest_commit subcommit2\n> +\t) &&\n> +\tgit submodule add ./sub &&\n> +\tgit commit -m \"add submodule\"\n> +'\n> +\n> +test_expect_success 'nonshallow clone implies nonshallow submodule' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules . super_clone &&\n> +\t(\n> +\t\tcd super_clone &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 3 lines\n> +\t) &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 2 lines\n> +\t)\n> +'\n> +\n> +test_expect_success 'shallow clone implies shallow submodule' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules --no-local --depth 1 . super_clone &&\nYou could add another commit to the super repo and then clone with \"--depth 2\".\nThe super repo would then contain 2 lines and the submodule still just 1.\nThis would make it more obvious that shallow submodules always have a depth of 1.\n \n> +\t(\n> +\t\tcd super_clone &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 1 lines\n> +\t) &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 1 lines\n> +\t)\n> +'\n> +\n> +test_expect_success 'shallow clone with non shallow submodule' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules --no-local --depth 1 --no-shallow-submodules . super_clone &&\n> +\t(\n> +\t\tcd super_clone &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 1 lines\n> +\t) &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 2 lines\n> +\t)\n> +'\n> +\n> +test_expect_success 'non shallow clone with shallow submodule' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules --no-local --shallow-submodules . super_clone &&\n> +\t(\n> +\t\tcd super_clone &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 3 lines\n> +\t) &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 1 lines\n> +\t)\n> +'\n> +\n> +test_done\n> -- \n> 2.5.0.264.gc776916.dirty\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"284450","messageId":"CAGZ79kZ-jszUtk4R+nWidmjGBFwAwymgr+nzghGSyUVqx=Y5-Q@mail.gmail.com","threadId":"42009","inReplyTo":"65302315-2C33-4801-887C-9AF3CB2F6BB6@gmail.com","subject":"Re: [PATCH 1/3] submodule clone: pass along `local` option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-25T17:15:50Z","receivedAt":"2016-04-25T17:15:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Apr 25, 2016 at 5:18 AM, Lars Schneider\n wrote:\n>> @@ -140,6 +141,10 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n>> argv_array_pushl(&cp.args, \"--reference\", reference, NULL);\n>> if (gitdir && *gitdir)\n>> argv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n>> + if (local == 1)\n> bikeshedding: Why do you use == 1 here and !local below? I would either compare both against\n> integers or none (\"if (local)\" should work here, too, no?). Or is this a Git coding guideline\n> that I am not yet aware of?\n\n\"if (local)\" doesn't work as it includes -1 as well.\nWhat I want to express here is:\n\n switch (local) {\n case 1: argv_array_push(&cp.args, \"--local\"); break; /* local == 1\ndid that */\n case 0: argv_array_push(&cp.args, \"--no-local\"); break; /* !local*/\n case -1: default: /* pass nothing */\n }\n\nThis seems to be more expressive now, so I'll replace with that?\n\n>> @@ -180,6 +186,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n>> OPT_STRING(0, \"depth\", &depth,\n>> N_(\"string\"),\n>> N_(\"depth for shallow clones\")),\n>> + OPT_BOOL(0, \"local\", &local,\n>> + N_(\"to clone from a local repository\")),\n> TBH I think \"local\" could be misleading here. How about \"--pass-transfer-protocol-on-to-submodules\"\n> or something? If I understand this option correctly then this could be useful for other cases besides\n> \"local\". Imagine you clone a repo via HTTPS that has references to SSH submodules on the same\n> server. If you don't have a proper SSH setup then the submodule clone will fail.\n\nGood point. We just converted an internal repository to use relative\npath for submodules instead of\nabsolute paths and that broke the auth (they used different protocols,\nwhich we were unaware of)\n\nHowever \"--pass-transfer-protocol-on-to-submodules\" would mean more\nthan just \"--[no-]local,\nbut also passing on the protocol prefix (which can be http://,\nhttps://, ssh://, git:// or more),\nso it would enlarge the scope of this series a lot. (Original purpose\nof this patch was to allow\nnot using the explicit file:// protocol in the test for the next\npatch. But I see passing on the protocol\nas a new issue worth resolving :)\n"},{"id":"284455","messageId":"CAGZ79kbL7VyimptOtOkrx=OvVw_Zdbfxq=J+JSUJXanFweYzNQ@mail.gmail.com","threadId":"42009","inReplyTo":"CA+P7+xppaK9d77h74j-4K=oWftdkeJE7048+PMzLS6gBS3E_uQ@mail.gmail.com","subject":"Re: [PATCH 3/3] clone: add t5614 to test cloning submodules with shallowness involved","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-25T17:41:48Z","receivedAt":"2016-04-25T17:41:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Apr 12, 2016 at 11:52 PM, Jacob Keller wrote:\n> On Tue, Apr 12, 2016 at 4:48 PM, Stefan Beller wrote:\n>> There are some inherent issues with shallow clones and submodules, such\n>> as having not having a commit available the superproject may point to\n>> in the submodule due to being shallow. Use the new file t5614 to document\n>> and test expectations in this area.\n>>\n>\n> The description seems to imply that there will be\n> \"test_expect_failure\" tests to indicate what needs to be improved...\n> Maybe I am just mis-reading it?\n\nThat commit message certainly reads like that. I'll tone it down to\njust say it's testing\nthe shallowness.\n\nLars wrote:\n>> +test_expect_success 'shallow clone implies shallow submodule' '\n>> + test_when_finished \"rm -rf super_clone\" &&\n>> + git clone --recurse-submodules --no-local --depth 1 . super_clone &&\n>\n> You could add another commit to the super repo and then clone with \"--depth 2\".\n> The super repo would then contain 2 lines and the submodule still just 1.\n> This would make it more obvious that shallow submodules always have a depth of 1.\n\nok will fix.\n"}]}