git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules
@ 2016-12-07 22:49 vi0oss
  2016-12-07 22:49 ` [PATCH 2/2] mailmap: Update my e-mail address vi0oss
  2016-12-08  0:02 ` [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules Stefan Beller
  0 siblings, 2 replies; 3+ messages in thread
From: vi0oss @ 2016-12-07 22:49 UTC (permalink / raw)
  To: git; +Cc: stefanbeller, Vitaly "_Vi" Shukela

From: "Vitaly \"_Vi\" Shukela" <vi0oss@gmail.com>

In 31224cbdc7 (clone: recursive and reference option triggers
submodule alternates, 2016-08-17) a mechanism was added to
have submodules referenced.  It did not address _nested_
submodules, however.

This patch makes all not just the root repository, but also
all submodules (recursively) have submodule.alternateLocation
and submodule.alternateErrorStrategy configured, making Git
search for possible alternates for nested submodules as well.

As submodule's alternate target does not end in .git/objects
(rather .git/modules/qqqqqq/objects), this alternate target
path restriction for in add_possible_reference_from_superproject
relates from "*.git/objects" to just */objects".

New tests have been added to t7408-submodule-reference.

Signed-off-by: Vitaly _Vi Shukela <vi0oss@gmail.com>
---

Notes:
    Resolved issues pointed by Stefan Beller except of
    the one about loosened path check, which he aggreed
    to be relaxed for this case.

 builtin/submodule--helper.c    | 21 ++++++++++--
 t/t7408-submodule-reference.sh | 72 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..7b7633d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -498,9 +498,9 @@ static int add_possible_reference_from_superproject(
 
 	/*
 	 * If the alternate object store is another repository, try the
-	 * standard layout with .git/modules/<name>/objects
+	 * standard layout with .git/(modules/<name>)+/objects
 	 */
-	if (ends_with(alt->path, ".git/objects")) {
+	if (ends_with(alt->path, "/objects")) {
 		char *sm_alternate;
 		struct strbuf sb = STRBUF_INIT;
 		struct strbuf err = STRBUF_INIT;
@@ -583,6 +583,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	struct strbuf rel_path = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list reference = STRING_LIST_INIT_NODUP;
+	char *sm_alternate = NULL, *error_strategy = NULL;
 
 	struct option module_clone_options[] = {
 		OPT_STRING(0, "prefix", &prefix,
@@ -672,6 +673,22 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		die(_("could not get submodule directory for '%s'"), path);
 	git_config_set_in_file(p, "core.worktree",
 			       relative_path(path, sm_gitdir, &rel_path));
+
+	/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
+	git_config_get_string("submodule.alternateLocation", &sm_alternate);
+	if (sm_alternate) {
+		git_config_set_in_file(p, "submodule.alternateLocation",
+					   sm_alternate);
+	}
+	git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
+	if (error_strategy) {
+		git_config_set_in_file(p, "submodule.alternateErrorStrategy",
+					   error_strategy);
+	}
+
+		free(sm_alternate);
+		free(error_strategy);
+
 	strbuf_release(&sb);
 	strbuf_release(&rel_path);
 	free(sm_gitdir);
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 1c1e289..ef7771b 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -125,4 +125,76 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
 	)
 '
 
+test_expect_success 'preparing second superproject with a nested submodule' '
+	test_create_repo supersuper &&
+	(
+		cd supersuper &&
+		echo "I am super super." >file &&
+		git add file &&
+		git commit -m B-super-super-initial
+		git submodule add "file://$base_dir/super" subwithsub &&
+		git commit -m B-super-super-added &&
+		git submodule update --init --recursive &&
+		git repack -ad
+	)
+'
+
+# At this point there are three root-level positories: A, B, super and super2
+
+test_expect_success 'nested submodule alternate in works and is actually used' '
+	test_when_finished "rm -rf supersuper-clone" &&
+	git clone --recursive --reference supersuper supersuper supersuper-clone &&
+	(
+		cd supersuper-clone &&
+		# test superproject has alternates setup correctly
+		test_alternate_is_used .git/objects/info/alternates . &&
+		# immediate submodule has alternate:
+		test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
+		# nested submodule also has alternate:
+		test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+	)
+'
+
+test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
+	test_when_finished "rm -rf supersuper-clone supersuper2" &&
+	git clone supersuper supersuper2 &&
+	(
+		cd supersuper2 &&
+		git submodule update --init
+	) &&
+	test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
+	(
+		cd supersuper-clone &&
+		# test superproject has alternates setup correctly
+		test_alternate_is_used .git/objects/info/alternates . &&
+		# update of the submodule fails
+		test_must_fail git submodule update --init --recursive &&
+		# immediate submodule has alternate:
+		test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
+		# but nested submodule has no alternate:
+		test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+	)
+'
+
+test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
+	test_when_finished "rm -rf supersuper-clone supersuper2" &&
+	git clone supersuper supersuper2 &&
+	(
+		cd supersuper2 &&
+		git submodule update --init
+	) &&
+	git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
+	(
+		cd supersuper-clone &&
+		# test superproject has alternates setup correctly
+		test_alternate_is_used .git/objects/info/alternates . &&
+		# update of the submodule fails
+		test_must_fail git submodule update --init --recursive &&
+		# immediate submodule has alternate:
+		test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
+		# but nested submodule has no alternate:
+		test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+	)
+'
+
 test_done
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] mailmap: Update my e-mail address
  2016-12-07 22:49 [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules vi0oss
@ 2016-12-07 22:49 ` vi0oss
  2016-12-08  0:02 ` [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules Stefan Beller
  1 sibling, 0 replies; 3+ messages in thread
From: vi0oss @ 2016-12-07 22:49 UTC (permalink / raw)
  To: git; +Cc: stefanbeller, Vitaly "_Vi" Shukela

From: "Vitaly \"_Vi\" Shukela" <vi0oss@gmail.com>

Signed-off-by: Vitaly "_Vi" Shukela <vi0oss@gmail.com>
---
 .mailmap | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 9cc33e9..b7ae81a 100644
--- a/.mailmap
+++ b/.mailmap
@@ -246,7 +246,7 @@ Uwe Kleine-König <u.kleine-koenig@pengutronix.de> <ukleinek@informatik.uni-frei
 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> <uzeisberger@io.fsforth.de>
 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> <zeisberg@informatik.uni-freiburg.de>
 Ville Skyttä <ville.skytta@iki.fi> <scop@xemacs.org>
-Vitaly "_Vi" Shukela <public_vi@tut.by>
+Vitaly "_Vi" Shukela <vi0oss@gmail.com> <public_vi@tut.by>
 W. Trevor King <wking@tremily.us> <wking@drexel.edu>
 William Pursell <bill.pursell@gmail.com>
 YONETANI Tomokazu <y0n3t4n1@gmail.com> <qhwt+git@les.ath.cx>
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules
  2016-12-07 22:49 [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules vi0oss
  2016-12-07 22:49 ` [PATCH 2/2] mailmap: Update my e-mail address vi0oss
@ 2016-12-08  0:02 ` Stefan Beller
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Beller @ 2016-12-08  0:02 UTC (permalink / raw)
  To: vi0oss; +Cc: git@vger.kernel.org, Stefan Beller

On Wed, Dec 7, 2016 at 2:49 PM,  <vi0oss@gmail.com> wrote:
> Notes:
>     Resolved issues pointed by Stefan Beller except of
>     the one about loosened path check, which he aggreed
>     to be relaxed for this case.

I am sorry to have given an incomplete review at the first time. :/
More below.

> +       /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
> +       git_config_get_string("submodule.alternateLocation", &sm_alternate);
> +       if (sm_alternate) {
> +               git_config_set_in_file(p, "submodule.alternateLocation",
> +                                          sm_alternate);
> +       }

For a single command, we usually omit the braces for
an if clause, i.e.

    if (foo)
        bar(...);

    /* code goes on */


> +       git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
> +       if (error_strategy) {
> +               git_config_set_in_file(p, "submodule.alternateErrorStrategy",
> +                                          error_strategy);
> +       }
> +
> +               free(sm_alternate);
> +               free(error_strategy);
> +

The indentation seems a bit off for the free here?
(The main nit that motivated me writing the email)


>         strbuf_release(&sb);
>         strbuf_release(&rel_path);
>         free(sm_gitdir);
> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
> index 1c1e289..ef7771b 100755
> --- a/t/t7408-submodule-reference.sh
> +++ b/t/t7408-submodule-reference.sh
> @@ -125,4 +125,76 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
>         )
>  '
>
> +test_expect_success 'preparing second superproject with a nested submodule' '
> +       test_create_repo supersuper &&
> +       (
> +               cd supersuper &&
> +               echo "I am super super." >file &&
> +               git add file &&
> +               git commit -m B-super-super-initial
> +               git submodule add "file://$base_dir/super" subwithsub &&
> +               git commit -m B-super-super-added &&
> +               git submodule update --init --recursive &&
> +               git repack -ad
> +       )
> +'
> +
> +# At this point there are three root-level positories: A, B, super and super2
> +
> +test_expect_success 'nested submodule alternate in works and is actually used' '
> +       test_when_finished "rm -rf supersuper-clone" &&
> +       git clone --recursive --reference supersuper supersuper supersuper-clone &&
> +       (
> +               cd supersuper-clone &&
> +               # test superproject has alternates setup correctly
> +               test_alternate_is_used .git/objects/info/alternates . &&
> +               # immediate submodule has alternate:
> +               test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
> +               # nested submodule also has alternate:
> +               test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> +       )
> +'
> +
> +test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
> +       test_when_finished "rm -rf supersuper-clone supersuper2" &&
> +       git clone supersuper supersuper2 &&
> +       (
> +               cd supersuper2 &&
> +               git submodule update --init
> +       ) &&
> +       test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
> +       (
> +               cd supersuper-clone &&
> +               # test superproject has alternates setup correctly
> +               test_alternate_is_used .git/objects/info/alternates . &&
> +               # update of the submodule fails
> +               test_must_fail git submodule update --init --recursive &&
> +               # immediate submodule has alternate:
> +               test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
> +               # but nested submodule has no alternate:
> +               test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> +       )
> +'
> +
> +test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
> +       test_when_finished "rm -rf supersuper-clone supersuper2" &&
> +       git clone supersuper supersuper2 &&
> +       (
> +               cd supersuper2 &&
> +               git submodule update --init
> +       ) &&
> +       git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
> +       (
> +               cd supersuper-clone &&
> +               # test superproject has alternates setup correctly
> +               test_alternate_is_used .git/objects/info/alternates . &&
> +               # update of the submodule fails
> +               test_must_fail git submodule update --init --recursive &&
> +               # immediate submodule has alternate:
> +               test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
> +               # but nested submodule has no alternate:
> +               test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> +       )

Both the first and the last part are the same in the last two tests,
the only difference is the line with git clone --reference ...
Maybe we can use a function somehow to make this a bit more
obvious?

Otherwise the tests look good to me.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-12-08  0:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-07 22:49 [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules vi0oss
2016-12-07 22:49 ` [PATCH 2/2] mailmap: Update my e-mail address vi0oss
2016-12-08  0:02 ` [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules Stefan Beller

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).