From 83dec7338a2409c89f6697e2e1d0b4306d72f3e8 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 11 Aug 2016 16:13:58 -0700 Subject: [PATCH 1/8] t7408: modernize style No functional change intended. This commit only changes formatting to the style we recently use, e.g. starting the body of a test with a single quote on the same line as the header, and then having the test indented in the following lines. Whenever we change directories, we do that in subshells. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- t/t7408-submodule-reference.sh | 122 +++++++++++++++++---------------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index eaea19b8f2..b84c674809 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -8,74 +8,76 @@ test_description='test clone --reference' base_dir=$(pwd) -U=$base_dir/UPLOAD_LOG +test_expect_success 'preparing first repository' ' + test_create_repo A && + ( + cd A && + echo first >file1 && + git add file1 && + git commit -m A-initial + ) +' -test_expect_success 'preparing first repository' \ -'test_create_repo A && cd A && -echo first > file1 && -git add file1 && -git commit -m A-initial' +test_expect_success 'preparing second repository' ' + git clone A B && + ( + cd B && + echo second >file2 && + git add file2 && + git commit -m B-addition && + git repack -a -d && + git prune + ) +' -cd "$base_dir" +test_expect_success 'preparing superproject' ' + test_create_repo super && + ( + cd super && + echo file >file && + git add file && + git commit -m B-super-initial + ) +' -test_expect_success 'preparing second repository' \ -'git clone A B && cd B && -echo second > file2 && -git add file2 && -git commit -m B-addition && -git repack -a -d && -git prune' +test_expect_success 'submodule add --reference' ' + ( + cd super && + git submodule add --reference ../B "file://$base_dir/A" sub && + git commit -m B-super-added + ) +' -cd "$base_dir" +test_expect_success 'after add: existence of info/alternates' ' + test_line_count = 1 super/.git/modules/sub/objects/info/alternates +' -test_expect_success 'preparing superproject' \ -'test_create_repo super && cd super && -echo file > file && -git add file && -git commit -m B-super-initial' +test_expect_success 'that reference gets used with add' ' + ( + cd super/sub && + echo "0 objects, 0 kilobytes" >expected && + git count-objects >current && + diff expected current + ) +' -cd "$base_dir" +test_expect_success 'cloning superproject' ' + git clone super super-clone +' -test_expect_success 'submodule add --reference' \ -'cd super && git submodule add --reference ../B "file://$base_dir/A" sub && -git commit -m B-super-added' +test_expect_success 'update with reference' ' + cd super-clone && git submodule update --init --reference ../B +' -cd "$base_dir" +test_expect_success 'after update: existence of info/alternates' ' + test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates +' -test_expect_success 'after add: existence of info/alternates' \ -'test_line_count = 1 super/.git/modules/sub/objects/info/alternates' - -cd "$base_dir" - -test_expect_success 'that reference gets used with add' \ -'cd super/sub && -echo "0 objects, 0 kilobytes" > expected && -git count-objects > current && -diff expected current' - -cd "$base_dir" - -test_expect_success 'cloning superproject' \ -'git clone super super-clone' - -cd "$base_dir" - -test_expect_success 'update with reference' \ -'cd super-clone && git submodule update --init --reference ../B' - -cd "$base_dir" - -test_expect_success 'after update: existence of info/alternates' \ -'test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates' - -cd "$base_dir" - -test_expect_success 'that reference gets used with update' \ -'cd super-clone/sub && -echo "0 objects, 0 kilobytes" > expected && -git count-objects > current && -diff expected current' - -cd "$base_dir" +test_expect_success 'that reference gets used with update' ' + cd super-clone/sub && + echo "0 objects, 0 kilobytes" >expected && + git count-objects >current && + diff expected current +' test_done From 9292536eb40294507544b963538e738637f70e26 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 11 Aug 2016 16:13:59 -0700 Subject: [PATCH 2/8] t7408: merge short tests, factor out testing method Tests consisting of one line each can be consolidated to have fewer tests to run as well as fewer lines of code. When having just a few git commands, do not create a new shell but use the -C flag in Git to execute in the correct directory. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- t/t7408-submodule-reference.sh | 48 ++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index b84c674809..dff47af255 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -8,6 +8,15 @@ test_description='test clone --reference' base_dir=$(pwd) +test_alternate_is_used () { + alternates_file="$1" && + working_dir="$2" && + test_line_count = 1 "$alternates_file" && + echo "0 objects, 0 kilobytes" >expect && + git -C "$working_dir" count-objects >actual && + test_cmp expect actual +} + test_expect_success 'preparing first repository' ' test_create_repo A && ( @@ -40,16 +49,14 @@ test_expect_success 'preparing superproject' ' ) ' -test_expect_success 'submodule add --reference' ' +test_expect_success 'submodule add --reference uses alternates' ' ( cd super && git submodule add --reference ../B "file://$base_dir/A" sub && - git commit -m B-super-added - ) -' - -test_expect_success 'after add: existence of info/alternates' ' - test_line_count = 1 super/.git/modules/sub/objects/info/alternates + git commit -m B-super-added && + git repack -ad + ) && + test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub ' test_expect_success 'that reference gets used with add' ' @@ -61,23 +68,18 @@ test_expect_success 'that reference gets used with add' ' ) ' -test_expect_success 'cloning superproject' ' - git clone super super-clone -' +# The tests up to this point, and repositories created by them +# (A, B, super and super/sub), are about setting up the stage +# for subsequent tests and meant to be kept throughout the +# remainder of the test. +# Tests from here on, if they create their own test repository, +# are expected to clean after themselves. -test_expect_success 'update with reference' ' - cd super-clone && git submodule update --init --reference ../B -' - -test_expect_success 'after update: existence of info/alternates' ' - test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates -' - -test_expect_success 'that reference gets used with update' ' - cd super-clone/sub && - echo "0 objects, 0 kilobytes" >expected && - git count-objects >current && - diff expected current +test_expect_success 'updating superproject keeps alternates' ' + test_when_finished "rm -rf super-clone" && + git clone super super-clone && + git -C super-clone submodule update --init --reference ../B && + test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub ' test_done From 965dbea09a5a71e78f72f11fd792646a4ce2a17a Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 11 Aug 2016 16:14:00 -0700 Subject: [PATCH 3/8] submodule--helper module-clone: allow multiple references Allow users to pass in multiple references, just as clone accepts multiple references as well. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 6f6d67a469..ce9b3e9bb8 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -442,7 +442,7 @@ static int module_name(int argc, const char **argv, const char *prefix) } static int clone_submodule(const char *path, const char *gitdir, const char *url, - const char *depth, const char *reference, int quiet) + const char *depth, struct string_list *reference, int quiet) { struct child_process cp; child_process_init(&cp); @@ -453,8 +453,12 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url argv_array_push(&cp.args, "--quiet"); if (depth && *depth) argv_array_pushl(&cp.args, "--depth", depth, NULL); - if (reference && *reference) - argv_array_pushl(&cp.args, "--reference", reference, NULL); + if (reference->nr) { + struct string_list_item *item; + for_each_string_list_item(item, reference) + argv_array_pushl(&cp.args, "--reference", + item->string, NULL); + } if (gitdir && *gitdir) argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL); @@ -470,13 +474,13 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url static int module_clone(int argc, const char **argv, const char *prefix) { - const char *name = NULL, *url = NULL; - const char *reference = NULL, *depth = NULL; + const char *name = NULL, *url = NULL, *depth = NULL; int quiet = 0; FILE *submodule_dot_git; char *p, *path = NULL, *sm_gitdir; struct strbuf rel_path = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; + struct string_list reference = STRING_LIST_INIT_NODUP; struct option module_clone_options[] = { OPT_STRING(0, "prefix", &prefix, @@ -491,8 +495,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) OPT_STRING(0, "url", &url, N_("string"), N_("url where to clone the submodule from")), - OPT_STRING(0, "reference", &reference, - N_("string"), + OPT_STRING_LIST(0, "reference", &reference, + N_("repo"), N_("reference repository")), OPT_STRING(0, "depth", &depth, N_("string"), @@ -528,7 +532,7 @@ static int module_clone(int argc, const char **argv, const char *prefix) if (!file_exists(sm_gitdir)) { if (safe_create_leading_directories_const(sm_gitdir) < 0) die(_("could not create directory '%s'"), sm_gitdir); - if (clone_submodule(path, sm_gitdir, url, depth, reference, quiet)) + if (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet)) die(_("clone of '%s' into submodule path '%s' failed"), url, path); } else { From 5f50f33e875cc877747a40a80e8ead73db0ec8c1 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 11 Aug 2016 16:14:01 -0700 Subject: [PATCH 4/8] submodule--helper update-clone: allow multiple references Allow the user to pass in multiple references to update_clone. Currently this is only internal API, but once the shell script is replaced by a C version, this is needed. This fixes an API bug between the shell script and the helper. Currently the helper accepts "--reference" "--reference=foo" as a OPT_STRING whose value happens to be "--reference=foo", and then uses if (suc->reference) argv_array_push(&child->args, suc->reference) where suc->reference _is_ "--reference=foo" when invoking the underlying "git clone", it cancels out. With this change we omit one of the "--reference" arguments when passing references from the shell script to the helper. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 14 +++++++++----- git-submodule.sh | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index ce9b3e9bb8..70968485b5 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -584,7 +584,7 @@ struct submodule_update_clone { /* configuration parameters which are passed on to the children */ int quiet; int recommend_shallow; - const char *reference; + struct string_list references; const char *depth; const char *recursive_prefix; const char *prefix; @@ -600,7 +600,8 @@ struct submodule_update_clone { int failed_clones_nr, failed_clones_alloc; }; #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \ - SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \ + SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \ + NULL, NULL, NULL, \ STRING_LIST_INIT_DUP, 0, NULL, 0, 0} @@ -710,8 +711,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, argv_array_pushl(&child->args, "--path", sub->path, NULL); argv_array_pushl(&child->args, "--name", sub->name, NULL); argv_array_pushl(&child->args, "--url", url, NULL); - if (suc->reference) - argv_array_push(&child->args, suc->reference); + if (suc->references.nr) { + struct string_list_item *item; + for_each_string_list_item(item, &suc->references) + argv_array_pushl(&child->args, "--reference", item->string, NULL); + } if (suc->depth) argv_array_push(&child->args, suc->depth); @@ -830,7 +834,7 @@ static int update_clone(int argc, const char **argv, const char *prefix) OPT_STRING(0, "update", &update, N_("string"), N_("rebase, merge, checkout or none")), - OPT_STRING(0, "reference", &suc.reference, N_("repo"), + OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"), N_("reference repository")), OPT_STRING(0, "depth", &suc.depth, "", N_("Create a shallow clone truncated to the " diff --git a/git-submodule.sh b/git-submodule.sh index c90dc335d1..3b412f554b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -575,7 +575,7 @@ cmd_update() ${wt_prefix:+--prefix "$wt_prefix"} \ ${prefix:+--recursive-prefix "$prefix"} \ ${update:+--update "$update"} \ - ${reference:+--reference "$reference"} \ + ${reference:+"$reference"} \ ${depth:+--depth "$depth"} \ ${recommend_shallow:+"$recommend_shallow"} \ ${jobs:+$jobs} \ From 9eeea7d2bc8132cfaa1b79271c077e80317f3ae1 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Mon, 15 Aug 2016 14:53:24 -0700 Subject: [PATCH 5/8] clone: factor out checking for an alternate path In a later patch we want to determine if a path is suitable as an alternate from other commands than builtin/clone. Move the checking functionality of `add_one_reference` to `compute_alternate_path` that is defined in cache.h. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/clone.c | 43 ++++++---------------------- cache.h | 1 + sha1_file.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 34 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index f044a8c27f..4cd3a6691a 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -282,44 +282,19 @@ static void strip_trailing_slashes(char *dir) static int add_one_reference(struct string_list_item *item, void *cb_data) { - char *ref_git; - const char *repo; - struct strbuf alternate = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + struct strbuf sb = STRBUF_INIT; + char *ref_git = compute_alternate_path(item->string, &err); - /* Beware: read_gitfile(), real_path() and mkpath() return static buffer */ - ref_git = xstrdup(real_path(item->string)); + if (!ref_git) + die("%s", err.buf); - repo = read_gitfile(ref_git); - if (!repo) - repo = read_gitfile(mkpath("%s/.git", ref_git)); - if (repo) { - free(ref_git); - ref_git = xstrdup(repo); - } + strbuf_addf(&sb, "%s/objects", ref_git); + add_to_alternates_file(sb.buf); - if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) { - char *ref_git_git = mkpathdup("%s/.git", ref_git); - free(ref_git); - ref_git = ref_git_git; - } else if (!is_directory(mkpath("%s/objects", ref_git))) { - struct strbuf sb = STRBUF_INIT; - if (get_common_dir(&sb, ref_git)) - die(_("reference repository '%s' as a linked checkout is not supported yet."), - item->string); - die(_("reference repository '%s' is not a local repository."), - item->string); - } - - if (!access(mkpath("%s/shallow", ref_git), F_OK)) - die(_("reference repository '%s' is shallow"), item->string); - - if (!access(mkpath("%s/info/grafts", ref_git), F_OK)) - die(_("reference repository '%s' is grafted"), item->string); - - strbuf_addf(&alternate, "%s/objects", ref_git); - add_to_alternates_file(alternate.buf); - strbuf_release(&alternate); free(ref_git); + strbuf_release(&err); + strbuf_release(&sb); return 0; } diff --git a/cache.h b/cache.h index b5f76a4cf4..1087dcaa1f 100644 --- a/cache.h +++ b/cache.h @@ -1342,6 +1342,7 @@ extern struct alternate_object_database { } *alt_odb_list; extern void prepare_alt_odb(void); extern void read_info_alternates(const char * relative_base, int depth); +extern char *compute_alternate_path(const char *path, struct strbuf *err); extern void add_to_alternates_file(const char *reference); typedef int alt_odb_fn(struct alternate_object_database *, void *); extern int foreach_alt_odb(alt_odb_fn, void*); diff --git a/sha1_file.c b/sha1_file.c index cb571ac6e8..0fe5aa35a5 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -425,6 +425,82 @@ void add_to_alternates_file(const char *reference) free(alts); } +/* + * Compute the exact path an alternate is at and returns it. In case of + * error NULL is returned and the human readable error is added to `err` + * `path` may be relative and should point to $GITDIR. + * `err` must not be null. + */ +char *compute_alternate_path(const char *path, struct strbuf *err) +{ + char *ref_git = NULL; + const char *repo, *ref_git_s; + int seen_error = 0; + + ref_git_s = real_path_if_valid(path); + if (!ref_git_s) { + seen_error = 1; + strbuf_addf(err, _("path '%s' does not exist"), path); + goto out; + } else + /* + * Beware: read_gitfile(), real_path() and mkpath() + * return static buffer + */ + ref_git = xstrdup(ref_git_s); + + repo = read_gitfile(ref_git); + if (!repo) + repo = read_gitfile(mkpath("%s/.git", ref_git)); + if (repo) { + free(ref_git); + ref_git = xstrdup(repo); + } + + if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) { + char *ref_git_git = mkpathdup("%s/.git", ref_git); + free(ref_git); + ref_git = ref_git_git; + } else if (!is_directory(mkpath("%s/objects", ref_git))) { + struct strbuf sb = STRBUF_INIT; + seen_error = 1; + if (get_common_dir(&sb, ref_git)) { + strbuf_addf(err, + _("reference repository '%s' as a linked " + "checkout is not supported yet."), + path); + goto out; + } + + strbuf_addf(err, _("reference repository '%s' is not a " + "local repository."), path); + goto out; + } + + if (!access(mkpath("%s/shallow", ref_git), F_OK)) { + strbuf_addf(err, _("reference repository '%s' is shallow"), + path); + seen_error = 1; + goto out; + } + + if (!access(mkpath("%s/info/grafts", ref_git), F_OK)) { + strbuf_addf(err, + _("reference repository '%s' is grafted"), + path); + seen_error = 1; + goto out; + } + +out: + if (seen_error) { + free(ref_git); + ref_git = NULL; + } + + return ref_git; +} + int foreach_alt_odb(alt_odb_fn fn, void *cb) { struct alternate_object_database *ent; From 5e40800df2ef1045d377f01caccee89c83beffab Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Mon, 15 Aug 2016 14:53:25 -0700 Subject: [PATCH 6/8] clone: clarify option_reference as required In the next patch we introduce optional references; To better distinguish between optional and required references we rename the variable. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/clone.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 4cd3a6691a..ef5db7c499 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -50,7 +50,7 @@ static int option_verbosity; static int option_progress = -1; static enum transport_family family; static struct string_list option_config = STRING_LIST_INIT_NODUP; -static struct string_list option_reference = STRING_LIST_INIT_NODUP; +static struct string_list option_required_reference = STRING_LIST_INIT_NODUP; static int option_dissociate; static int max_jobs = -1; @@ -79,7 +79,7 @@ static struct option builtin_clone_options[] = { N_("number of submodules cloned in parallel")), OPT_STRING(0, "template", &option_template, N_("template-directory"), N_("directory from which templates will be used")), - OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"), + OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"), N_("reference repository")), OPT_BOOL(0, "dissociate", &option_dissociate, N_("use --reference only while cloning")), @@ -300,7 +300,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) static void setup_reference(void) { - for_each_string_list(&option_reference, add_one_reference, NULL); + for_each_string_list(&option_required_reference, add_one_reference, NULL); } static void copy_alternates(struct strbuf *src, struct strbuf *dst, @@ -952,7 +952,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) git_config_set(key.buf, repo); strbuf_reset(&key); - if (option_reference.nr) + if (option_required_reference.nr) setup_reference(); fetch_pattern = value.buf; From f7415b4d71150d5c2d52f87c8792591237aaf00e Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Mon, 15 Aug 2016 14:53:26 -0700 Subject: [PATCH 7/8] clone: implement optional references In a later patch we want to try to create alternates for submodules, but they might not exist in the referenced superproject. So add a way to skip the non existing references and report them. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- Documentation/git-clone.txt | 5 ++++- builtin/clone.c | 35 +++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index ec41d3d698..e316c4bd51 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -90,13 +90,16 @@ If you want to break the dependency of a repository cloned with `-s` on its source repository, you can simply run `git repack -a` to copy all objects from the source repository into a pack in the cloned repository. ---reference :: +--reference[-if-able] :: If the reference repository is on the local machine, automatically setup `.git/objects/info/alternates` to obtain objects from the reference repository. Using an already existing repository as an alternate will require fewer objects to be copied from the repository being cloned, reducing network and local storage costs. + When using the `--reference-if-able`, a non existing + directory is skipped with a warning instead of aborting + the clone. + *NOTE*: see the NOTE for the `--shared` option, and also the `--dissociate` option. diff --git a/builtin/clone.c b/builtin/clone.c index ef5db7c499..01826651f9 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -51,6 +51,7 @@ static int option_progress = -1; static enum transport_family family; static struct string_list option_config = STRING_LIST_INIT_NODUP; static struct string_list option_required_reference = STRING_LIST_INIT_NODUP; +static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP; static int option_dissociate; static int max_jobs = -1; @@ -81,6 +82,8 @@ static struct option builtin_clone_options[] = { N_("directory from which templates will be used")), OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"), N_("reference repository")), + OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference, + N_("repo"), N_("reference repository")), OPT_BOOL(0, "dissociate", &option_dissociate, N_("use --reference only while cloning")), OPT_STRING('o', "origin", &option_origin, N_("name"), @@ -283,24 +286,36 @@ static void strip_trailing_slashes(char *dir) static int add_one_reference(struct string_list_item *item, void *cb_data) { struct strbuf err = STRBUF_INIT; - struct strbuf sb = STRBUF_INIT; + int *required = cb_data; char *ref_git = compute_alternate_path(item->string, &err); - if (!ref_git) - die("%s", err.buf); + if (!ref_git) { + if (*required) + die("%s", err.buf); + else + fprintf(stderr, + _("info: Could not add alternate for '%s': %s\n"), + item->string, err.buf); + } else { + struct strbuf sb = STRBUF_INIT; + strbuf_addf(&sb, "%s/objects", ref_git); + add_to_alternates_file(sb.buf); + strbuf_release(&sb); + } - strbuf_addf(&sb, "%s/objects", ref_git); - add_to_alternates_file(sb.buf); - - free(ref_git); strbuf_release(&err); - strbuf_release(&sb); + free(ref_git); return 0; } static void setup_reference(void) { - for_each_string_list(&option_required_reference, add_one_reference, NULL); + int required = 1; + for_each_string_list(&option_required_reference, + add_one_reference, &required); + required = 0; + for_each_string_list(&option_optional_reference, + add_one_reference, &required); } static void copy_alternates(struct strbuf *src, struct strbuf *dst, @@ -952,7 +967,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) git_config_set(key.buf, repo); strbuf_reset(&key); - if (option_required_reference.nr) + if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); fetch_pattern = value.buf; From 31224cbdc723d78a310b4cdbbd5abcfcbd44a4e5 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 17 Aug 2016 15:45:35 -0700 Subject: [PATCH 8/8] clone: recursive and reference option triggers submodule alternates When `--recursive` and `--reference` is given, it is reasonable to expect that the submodules are created with references to the submodules of the given alternate for the superproject. An initial attempt to do this was presented to the mailing list, which used flags that are passed around ("--super-reference") that instructed the submodule clone to look for a reference in the submodules of the referenced superproject. This is not well thought out, as any further `submodule update` should also respect the initial setup. When a new submodule is added to the superproject and the alternate of the superproject does not know about that submodule yet, we rather error out informing the user instead of being unclear if we did or did not use a submodules alternate. To solve this problem introduce new options that store the configuration for what the user wanted originally. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- Documentation/config.txt | 12 ++++ builtin/clone.c | 19 ++++++ builtin/submodule--helper.c | 102 +++++++++++++++++++++++++++++++++ t/t7408-submodule-reference.sh | 43 ++++++++++++++ 4 files changed, 176 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index bc1c433c4e..e2571ea8ea 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2837,6 +2837,18 @@ submodule.fetchJobs:: in parallel. A value of 0 will give some reasonable default. If unset, it defaults to 1. +submodule.alternateLocation:: + Specifies how the submodules obtain alternates when submodules are + cloned. Possible values are `no`, `superproject`. + By default `no` is assumed, which doesn't add references. When the + value is set to `superproject` the submodule to be cloned computes + its alternates location relative to the superprojects alternate. + +submodule.alternateErrorStrategy + Specifies how to treat errors with the alternates for a submodule + as computed via `submodule.alternateLocation`. Possible values are + `ignore`, `info`, `die`. Default is `die`. + tag.forceSignAnnotated:: A boolean to specify whether annotated tags created should be GPG signed. If `--annotate` is specified on the command line, it takes diff --git a/builtin/clone.c b/builtin/clone.c index 01826651f9..404c5e8022 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char *prefix) else fprintf(stderr, _("Cloning into '%s'...\n"), dir); } + + if (option_recursive) { + if (option_required_reference.nr && + option_optional_reference.nr) + die(_("clone --recursive is not compatible with " + "both --reference and --reference-if-able")); + else if (option_required_reference.nr) { + string_list_append(&option_config, + "submodule.alternateLocation=superproject"); + string_list_append(&option_config, + "submodule.alternateErrorStrategy=die"); + } else if (option_optional_reference.nr) { + string_list_append(&option_config, + "submodule.alternateLocation=superproject"); + string_list_append(&option_config, + "submodule.alternateErrorStrategy=info"); + } + } + init_db(option_template, INIT_DB_QUIET); write_config(&option_config); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 70968485b5..a3667578b4 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -472,6 +472,105 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url return run_command(&cp); } +struct submodule_alternate_setup { + const char *submodule_name; + enum SUBMODULE_ALTERNATE_ERROR_MODE { + SUBMODULE_ALTERNATE_ERROR_DIE, + SUBMODULE_ALTERNATE_ERROR_INFO, + SUBMODULE_ALTERNATE_ERROR_IGNORE + } error_mode; + struct string_list *reference; +}; +#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \ + SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL } + +static int add_possible_reference_from_superproject( + struct alternate_object_database *alt, void *sas_cb) +{ + struct submodule_alternate_setup *sas = sas_cb; + + /* directory name, minus trailing slash */ + size_t namelen = alt->name - alt->base - 1; + struct strbuf name = STRBUF_INIT; + strbuf_add(&name, alt->base, namelen); + + /* + * If the alternate object store is another repository, try the + * standard layout with .git/modules//objects + */ + if (ends_with(name.buf, ".git/objects")) { + char *sm_alternate; + struct strbuf sb = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + strbuf_add(&sb, name.buf, name.len - strlen("objects")); + /* + * We need to end the new path with '/' to mark it as a dir, + * otherwise a submodule name containing '/' will be broken + * as the last part of a missing submodule reference would + * be taken as a file name. + */ + strbuf_addf(&sb, "modules/%s/", sas->submodule_name); + + sm_alternate = compute_alternate_path(sb.buf, &err); + if (sm_alternate) { + string_list_append(sas->reference, xstrdup(sb.buf)); + free(sm_alternate); + } else { + switch (sas->error_mode) { + case SUBMODULE_ALTERNATE_ERROR_DIE: + die(_("submodule '%s' cannot add alternate: %s"), + sas->submodule_name, err.buf); + case SUBMODULE_ALTERNATE_ERROR_INFO: + fprintf(stderr, _("submodule '%s' cannot add alternate: %s"), + sas->submodule_name, err.buf); + case SUBMODULE_ALTERNATE_ERROR_IGNORE: + ; /* nothing */ + } + } + strbuf_release(&sb); + } + + strbuf_release(&name); + return 0; +} + +static void prepare_possible_alternates(const char *sm_name, + struct string_list *reference) +{ + char *sm_alternate = NULL, *error_strategy = NULL; + struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT; + + git_config_get_string("submodule.alternateLocation", &sm_alternate); + if (!sm_alternate) + return; + + git_config_get_string("submodule.alternateErrorStrategy", &error_strategy); + + if (!error_strategy) + error_strategy = xstrdup("die"); + + sas.submodule_name = sm_name; + sas.reference = reference; + if (!strcmp(error_strategy, "die")) + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE; + else if (!strcmp(error_strategy, "info")) + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO; + else if (!strcmp(error_strategy, "ignore")) + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_IGNORE; + else + die(_("Value '%s' for submodule.alternateErrorStrategy is not recognized"), error_strategy); + + if (!strcmp(sm_alternate, "superproject")) + foreach_alt_odb(add_possible_reference_from_superproject, &sas); + else if (!strcmp(sm_alternate, "no")) + ; /* do nothing */ + else + die(_("Value '%s' for submodule.alternateLocation is not recognized"), sm_alternate); + + free(sm_alternate); + free(error_strategy); +} + static int module_clone(int argc, const char **argv, const char *prefix) { const char *name = NULL, *url = NULL, *depth = NULL; @@ -532,6 +631,9 @@ static int module_clone(int argc, const char **argv, const char *prefix) if (!file_exists(sm_gitdir)) { if (safe_create_leading_directories_const(sm_gitdir) < 0) die(_("could not create directory '%s'"), sm_gitdir); + + prepare_possible_alternates(name, &reference); + if (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet)) die(_("clone of '%s' into submodule path '%s' failed"), url, path); diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index dff47af255..1c1e289ffd 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -82,4 +82,47 @@ test_expect_success 'updating superproject keeps alternates' ' test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub ' +test_expect_success 'submodules use alternates when cloning a superproject' ' + test_when_finished "rm -rf super-clone" && + git clone --reference super --recursive super super-clone && + ( + cd super-clone && + # test superproject has alternates setup correctly + test_alternate_is_used .git/objects/info/alternates . && + # test submodule has correct setup + test_alternate_is_used .git/modules/sub/objects/info/alternates sub + ) +' + +test_expect_success 'missing submodule alternate fails clone and submodule update' ' + test_when_finished "rm -rf super-clone" && + git clone super super2 && + test_must_fail git clone --recursive --reference super2 super2 super-clone && + ( + cd super-clone && + # test superproject has alternates setup correctly + test_alternate_is_used .git/objects/info/alternates . && + # update of the submodule succeeds + test_must_fail git submodule update --init && + # and we have no alternates: + test_must_fail test_alternate_is_used .git/modules/sub/objects/info/alternates sub && + test_must_fail test_path_is_file sub/file1 + ) +' + +test_expect_success 'ignoring missing submodule alternates passes clone and submodule update' ' + test_when_finished "rm -rf super-clone" && + git clone --reference-if-able super2 --recursive super2 super-clone && + ( + cd super-clone && + # test superproject has alternates setup correctly + test_alternate_is_used .git/objects/info/alternates . && + # update of the submodule succeeds + git submodule update --init && + # and we have no alternates: + test_must_fail test_alternate_is_used .git/modules/sub/objects/info/alternates sub && + test_path_is_file sub/file1 + ) +' + test_done