Re: [PATCH 6/6] clone: reference flag is used for submodules as well

[Junio C Hamano]

Stefan Beller  writes:

> When giving a --reference while also giving --recurse, the alternates
> for the submodules are assumed to be in the superproject as well.
>
> In case they are not, we error out when cloning the submodule.
> However the update command succeeds as usual (with no alternates).

I covered most of what I want to say on this in my reply to 0/6; I
do not have strong objection against what single layout you chose to
support, nor I have strong opinion on which single layout we should
support by default, or what mechanism, if any, we should give users
to specify different layout.

But please make sure the choice you made is explained for the users.
The end-user documentation should talk about the effect of giving
these two options together.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] clone: reference flag is used for submodules as well

[Stefan Beller]

When giving a --reference while also giving --recurse, the alternates
for the submodules are assumed to be in the superproject as well.

In case they are not, we error out when cloning the submodule.
However the update command succeeds as usual (with no alternates).

Signed-off-by: Stefan Beller 
---
 builtin/clone.c| 22 ++
 t/t7408-submodule-reference.sh | 31 ++-
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f044a8c..f586df5 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_reference = STRING_LIST_INIT_NODUP;
+static struct string_list superreferences = STRING_LIST_INIT_DUP;
 static int option_dissociate;
 static int max_jobs = -1;
 
@@ -280,10 +281,11 @@ static void strip_trailing_slashes(char *dir)
*end = '\0';
 }
 
-static int add_one_reference(struct string_list_item *item, void *cb_data)
+static int add_one_reference(struct string_list_item *item, void *cb_dir)
 {
char *ref_git;
const char *repo;
+   const char *dir = cb_dir;
struct strbuf alternate = STRBUF_INIT;
 
/* Beware: read_gitfile(), real_path() and mkpath() return static 
buffer */
@@ -316,6 +318,13 @@ static int add_one_reference(struct string_list_item 
*item, void *cb_data)
if (!access(mkpath("%s/info/grafts", ref_git), F_OK))
die(_("reference repository '%s' is grafted"), item->string);
 
+   if (option_recursive) {
+   struct strbuf sb = STRBUF_INIT;
+   char *rel = xstrdup(relative_path(item->string, dir, ));
+   string_list_append(, rel);
+   strbuf_reset();
+   }
+
strbuf_addf(, "%s/objects", ref_git);
add_to_alternates_file(alternate.buf);
strbuf_release();
@@ -323,9 +332,9 @@ static int add_one_reference(struct string_list_item *item, 
void *cb_data)
return 0;
 }
 
-static void setup_reference(void)
+static void setup_reference(char *dir)
 {
-   for_each_string_list(_reference, add_one_reference, NULL);
+   for_each_string_list(_reference, add_one_reference, dir);
 }
 
 static void copy_alternates(struct strbuf *src, struct strbuf *dst,
@@ -736,6 +745,7 @@ static int checkout(void)
 
if (!err && option_recursive) {
struct argv_array args = ARGV_ARRAY_INIT;
+   static struct string_list_item *item;
argv_array_pushl(, "submodule", "update", "--init", 
"--recursive", NULL);
 
if (option_shallow_submodules == 1)
@@ -744,6 +754,10 @@ static int checkout(void)
if (max_jobs != -1)
argv_array_pushf(, "--jobs=%d", max_jobs);
 
+   if (superreferences.nr)
+   for_each_string_list_item(item, )
+   argv_array_pushf(, "--super-reference=%s", 
item->string);
+
err = run_command_v_opt(args.argv, RUN_GIT_CMD);
argv_array_clear();
}
@@ -978,7 +992,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
strbuf_reset();
 
if (option_reference.nr)
-   setup_reference();
+   setup_reference(dir);
 
fetch_pattern = value.buf;
refspec = parse_fetch_refspec(1, _pattern);
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 1416cbd..2652cfe 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -56,7 +56,8 @@ 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
+   git commit -m B-super-added &&
+   git repack -ad
) &&
test_alternate_usage super/.git/modules/sub/objects/info/alternates 
super/sub
 '
@@ -68,4 +69,32 @@ test_expect_success 'updating superproject keeps alternates' 
'
test_alternate_usage 
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_usage .git/objects/info/alternates . &&
+   # test submodule has correct setup
+   test_alternate_usage .git/modules/sub/objects/info/alternates 
sub
+   )
+'
+
+test_expect_success 'cloning superproject, missing submodule alternates' '
+   test_when_finished "rm -rf super-clone"