threads / patch / 49973

patch, 4 parts[PATCH 0/4]

Subject: [PATCH 0/4]

## tl;dr

21 messages between Dec 7, 2018 and Dec 28, 2018. Diffs are folded; open one to read it.

replies: 20people: 2as markdown or json

Stefan Beller· Dec 7, 2018, 23:54 UTC · lore

A couple days before the 2.19 release we had a bug report about broken submodules[1] and reverted[2] the commits leading up to them.

The behavior of said bug fixed itself by taking a different approach[3], specifically by a weaker enforcement of having `core.worktree` set in a submodule [4].

The revert [2] was overly broad as we neared the release, such that we wanted to rather keep the known buggy behavior of always having `core.worktree` set, rather than figuring out how to fix the new bug of having 'git submodule update' not working in old style repository setups.

This series re-introduces those reverted patches, with no changes in code, but with drastically changed commit messages, as those focus on why it is safe to re-introduce them instead of explaining the desire for the change.

[1] https://public-inbox.org/git/2659750.rG6xLiZASK@twilight [2] f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07) [3] 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17) [4] 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)

Stefan Beller (4):
  submodule update: add regression test with old style setups
  submodule: unset core.worktree if no working tree is present
  submodule--helper: fix BUG message in ensure_core_worktree
  submodule deinit: unset core.worktree
 builtin/submodule--helper.c        |  4 +++-
 submodule.c                        | 14 ++++++++++++++
 submodule.h                        |  2 ++
 t/lib-submodule-update.sh          |  5 +++--
 t/t7400-submodule-basic.sh         |  5 +++++
 t/t7412-submodule-absorbgitdirs.sh |  7 ++++++-
 6 files changed, 33 insertions(+), 4 deletions(-)
-- 
2.20.0.rc2.403.gdbc3b29805-goog
Stefan Beller· Dec 7, 2018, 23:54 UTC · re: Stefan Beller · lore

[PATCH 1/4] submodule update: add regression test with old style setups

As f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07) was produced shortly before a release, nobody asked for a regression test to be included. Add a regression test that makes sure that the invocation of `git submodule update` on old setups doesn't produce errors as pointed out in f178c13fda.

The place to add such a regression test may look odd in t7412, but that is the best place as there we setup old style submodule setups explicitly.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t7412-submodule-absorbgitdirs.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Show changes to t/t7412-submodule-absorbgitdirs.sh +6 −1
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index ce74c12da2..1cfa150768 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
 	GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
 		core.worktree "../../../nested" &&
 	# make sure this re-setup is correct
-	git status --ignore-submodules=none
+	git status --ignore-submodules=none &&
+
+	# also make sure this old setup does not regress
+	git submodule update --init --recursive >out 2>err &&
+	test_must_be_empty out &&
+	test_must_be_empty err
 '
 
 test_expect_success 'absorb the git dir in a nested submodule' '
-- 
2.20.0.rc2.403.gdbc3b29805-goog
Junio C Hamano· Dec 9, 2018, 00:11 UTC · re: Stefan Beller · lore

Re: [PATCH 1/4] submodule update: add regression test with old style setups

Stefan Beller <sbeller@google.com> writes:
Show 9 quoted lines
> As f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'",
> 2018-09-07) was produced shortly before a release, nobody asked for
> a regression test to be included. Add a regression test that makes sure
> that the invocation of `git submodule update` on old setups doesn't
> produce errors as pointed out in f178c13fda.
>
> The place to add such a regression test may look odd in t7412, but
> that is the best place as there we setup old style submodule setups
> explicitly.
Very good first step.  Thanks.
Show 24 quoted lines
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  t/t7412-submodule-absorbgitdirs.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
> index ce74c12da2..1cfa150768 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
>  	GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
>  		core.worktree "../../../nested" &&
>  	# make sure this re-setup is correct
> -	git status --ignore-submodules=none
> +	git status --ignore-submodules=none &&
> +
> +	# also make sure this old setup does not regress
> +	git submodule update --init --recursive >out 2>err &&
> +	test_must_be_empty out &&
> +	test_must_be_empty err
>  '
>  
>  test_expect_success 'absorb the git dir in a nested submodule' '
Stefan Beller· Dec 7, 2018, 23:54 UTC · re: Stefan Beller · lore

[PATCH 2/4] submodule: unset core.worktree if no working tree is present

This reintroduces 4fa4f90ccd (submodule: unset core.worktree if no working tree is present, 2018-06-12), which was reverted as part of f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07).

4fa4f90ccd was reverted as its followup commit was faulty, but without the accompanying change of the followup, we'd have an incomplete workflow of setting `core.worktree` again, when it is needed such as checking out a revision that contains a submodule.

So re-introduce that commit as-is, focusing on fixing up the followup
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c               | 14 ++++++++++++++
 submodule.h               |  2 ++
 t/lib-submodule-update.sh |  3 ++-
 3 files changed, 18 insertions(+), 1 deletion(-)
Show changes to 3 files +18 −1

submodule.c, submodule.h, t/lib-submodule-update.sh

diff --git a/submodule.c b/submodule.c
index 6415cc5580..d393e947e6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1561,6 +1561,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
 	return ret;
 }
 
+void submodule_unset_core_worktree(const struct submodule *sub)
+{
+	char *config_path = xstrfmt("%s/modules/%s/config",
+				    get_git_common_dir(), sub->name);
+
+	if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
+		warning(_("Could not unset core.worktree setting in submodule '%s'"),
+			  sub->path);
+
+	free(config_path);
+}
+
 static const char *get_super_prefix_or_empty(void)
 {
 	const char *s = get_super_prefix();
@@ -1726,6 +1738,8 @@ int submodule_move_head(const char *path,
 
 			if (is_empty_dir(path))
 				rmdir_or_warn(path);
+
+			submodule_unset_core_worktree(sub);
 		}
 	}
 out:
diff --git a/submodule.h b/submodule.h
index a680214c01..9e18e9b807 100644
--- a/submodule.h
+++ b/submodule.h
@@ -131,6 +131,8 @@ int submodule_move_head(const char *path,
 			const char *new_head,
 			unsigned flags);
 
+void submodule_unset_core_worktree(const struct submodule *sub);
+
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific environment variables, but
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 016391723c..51d4555549 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
 			git branch -t remove_sub1 origin/remove_sub1 &&
 			$command remove_sub1 &&
 			test_superproject_content origin/remove_sub1 &&
-			! test -e sub1
+			! test -e sub1 &&
+			test_must_fail git config -f .git/modules/sub1/config core.worktree
 		)
 	'
 	# ... absorbing a .git directory along the way.
-- 
2.20.0.rc2.403.gdbc3b29805-goog
Junio C Hamano· Dec 8, 2018, 06:44 UTC · re: Stefan Beller · lore

Re: [PATCH 2/4] submodule: unset core.worktree if no working tree is present

Stefan Beller <sbeller@google.com> writes:
Show 10 quoted lines
> This reintroduces 4fa4f90ccd (submodule: unset core.worktree if no working
> tree is present, 2018-06-12), which was reverted as part of f178c13fda
> (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07).
>
> 4fa4f90ccd was reverted as its followup commit was faulty, but without
> the accompanying change of the followup, we'd have an incomplete workflow
> of setting `core.worktree` again, when it is needed such as checking out
> a revision that contains a submodule.
>
> So re-introduce that commit as-is, focusing on fixing up the followup

I was hoping to hear (given what 0/4 claimed) a clearer explanation of what this change wants to achieve, but that is lacking.

No need to grumble about an earlier work was that turned out to be inappropriate for the codebase back then. Repeatedly saying "this is needed" without giving further explaining why it is so, or anything like that, would help readers.

Just pretend that the ealier commits and their reversion never happened, and further pretend that we are doing the best thing that should happen to our codebase. How would we explain this change, what the problem it tries to solve and what the solution looks like in the larger picture?

	When removing a working tree of a submodule (e.g. we may be
	switching back to an earlier commit in the superproject that
	did not have the submodule in question yet), we failed to
	unset core.worktree of the submodule's repository.  That
	caused this and that issues, exhibited by a few new tests
	this commit adds.
	Make sure that core.worktree gets unset so that a leftover
	setting won't cause these issues.

or something like that? I am just guessing by looking at the old commit's text, as the above two paragraphs and one sentence does not say much.

Show 59 quoted lines
> diff --git a/submodule.c b/submodule.c
> index 6415cc5580..d393e947e6 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1561,6 +1561,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
>  	return ret;
>  }
>  
> +void submodule_unset_core_worktree(const struct submodule *sub)
> +{
> +	char *config_path = xstrfmt("%s/modules/%s/config",
> +				    get_git_common_dir(), sub->name);
> +
> +	if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
> +		warning(_("Could not unset core.worktree setting in submodule '%s'"),
> +			  sub->path);
> +
> +	free(config_path);
> +}
> +
>  static const char *get_super_prefix_or_empty(void)
>  {
>  	const char *s = get_super_prefix();
> @@ -1726,6 +1738,8 @@ int submodule_move_head(const char *path,
>  
>  			if (is_empty_dir(path))
>  				rmdir_or_warn(path);
> +
> +			submodule_unset_core_worktree(sub);
>  		}
>  	}
>  out:
> diff --git a/submodule.h b/submodule.h
> index a680214c01..9e18e9b807 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -131,6 +131,8 @@ int submodule_move_head(const char *path,
>  			const char *new_head,
>  			unsigned flags);
>  
> +void submodule_unset_core_worktree(const struct submodule *sub);
> +
>  /*
>   * Prepare the "env_array" parameter of a "struct child_process" for executing
>   * a submodule by clearing any repo-specific environment variables, but
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 016391723c..51d4555549 100755
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
>  			git branch -t remove_sub1 origin/remove_sub1 &&
>  			$command remove_sub1 &&
>  			test_superproject_content origin/remove_sub1 &&
> -			! test -e sub1
> +			! test -e sub1 &&
> +			test_must_fail git config -f .git/modules/sub1/config core.worktree
>  		)
>  	'
>  	# ... absorbing a .git directory along the way.
Stefan Beller· Dec 7, 2018, 23:54 UTC · re: Stefan Beller · lore

[PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree

Shortly after f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07), we had another series that implemented partially the same, ensuring that core.worktree was set in a checked out submodule, namely 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)

As the series 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17) has different goals than the reverted series 7e25437d35 (Merge branch 'sb/submodule-core-worktree', 2018-07-18), I'd wanted to replay the series on top of it to reach the goal of having `core.worktree` correctly set when the submodules worktree is present, and unset when the worktree is not present.

The replay resulted in a strange merge conflict highlighting that the BUG message was not changed in 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13).

Fix the error message.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/submodule--helper.c +1 −1
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d38113a31a..31ac30cf2f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2045,7 +2045,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
 	struct repository subrepo;
 
 	if (argc != 2)
-		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
+		BUG("submodule--helper ensure-core-worktree <path>");
 
 	path = argv[1];
 
-- 
2.20.0.rc2.403.gdbc3b29805-goog
Junio C Hamano· Dec 8, 2018, 06:55 UTC · re: Stefan Beller · lore

Re: [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree

Stefan Beller <sbeller@google.com> writes:
Show 21 quoted lines
> Shortly after f178c13fda (Revert "Merge branch
> 'sb/submodule-core-worktree'", 2018-09-07), we had another series
> that implemented partially the same, ensuring that core.worktree was
> set in a checked out submodule, namely 74d4731da1 (submodule--helper:
> replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)
>
> As the series 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c',
> 2018-09-17) has different goals than the reverted series 7e25437d35
> (Merge branch 'sb/submodule-core-worktree', 2018-07-18), I'd wanted to
> replay the series on top of it to reach the goal of having `core.worktree`
> correctly set when the submodules worktree is present, and unset when the
> worktree is not present.
>
> The replay resulted in a strange merge conflict highlighting that
> the BUG message was not changed in 74d4731da1 (submodule--helper:
> replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13).
>
> Fix the error message.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

Unlike the step 2/4 I commented on, this does explain what this wants to do and why, at least when looked from sideways. Is the above saying the same as the following two-liner?

	An ealier mistake while rebasing to produce 74d4731da1
	failed to update this BUG message.  Fix this.
Show 15 quoted lines
>  builtin/submodule--helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index d38113a31a..31ac30cf2f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2045,7 +2045,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
>  	struct repository subrepo;
>  
>  	if (argc != 2)
> -		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
> +		BUG("submodule--helper ensure-core-worktree <path>");
>  
>  	path = argv[1];
Stefan Beller· Dec 12, 2018, 22:46 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree

Show 6 quoted lines
> Unlike the step 2/4 I commented on, this does explain what this
> wants to do and why, at least when looked from sideways.  Is the
> above saying the same as the following two-liner?
>
>         An ealier mistake while rebasing to produce 74d4731da1
>         failed to update this BUG message.  Fix this.

I am not sure if it was rebasing, which was executed mistakenly. So maybe just saying "74d4731da1 contains a faulty BUG message. Fix it." would do.

The intent of the longer message was to shed light in how I found the BUG (ie. I did not see the BUG message, which would ask me to actually fix a bug, but found it via code inspection), which I thought was valuable information, too.

Junio C Hamano· Dec 13, 2018, 03:14 UTC · re: Stefan Beller · lore

Re: [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree

Stefan Beller <sbeller@google.com> writes:
Show 15 quoted lines
>> Unlike the step 2/4 I commented on, this does explain what this
>> wants to do and why, at least when looked from sideways.  Is the
>> above saying the same as the following two-liner?
>>
>>         An ealier mistake while rebasing to produce 74d4731da1
>>         failed to update this BUG message.  Fix this.
>
> I am not sure if it was rebasing, which was executed mistakenly.
> So maybe just saying "74d4731da1 contains a faulty BUG
> message. Fix it." would do.
>
> The intent of the longer message was to shed light in how I found
> the BUG (ie. I did not see the BUG message, which would ask me
> to actually fix a bug, but found it via code inspection), which I
> thought was valuable information, too.

I guess that it could be stated in a way to make it valuable, but in the presented text, I somehow found it was making the more important part of the description (i.e. "this patch fixes a mistake made by 74d4731da1") buried and harder to grok.

Thanks.
Stefan Beller· Dec 7, 2018, 23:54 UTC · re: Stefan Beller · lore

[PATCH 4/4] submodule deinit: unset core.worktree

This re-introduces 984cd77ddb (submodule deinit: unset core.worktree, 2018-06-18), which was reverted as part of f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07)

The whole series was reverted as the offending commit e98317508c (submodule: ensure core.worktree is set after update, 2018-06-18) was relied on by other commits such as 984cd77ddb.

Keep the offending commit reverted, but its functionality came back via 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17), such that we can reintroduce 984cd77ddb now.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 2 ++
 t/lib-submodule-update.sh   | 2 +-
 t/t7400-submodule-basic.sh  | 5 +++++
 3 files changed, 8 insertions(+), 1 deletion(-)
Show changes to 3 files +8 −1

builtin/submodule--helper.c, t/lib-submodule-update.sh, t/t7400-submodule-basic.sh

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 31ac30cf2f..672b74db89 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1131,6 +1131,8 @@ static void deinit_submodule(const char *path, const char *prefix,
 		if (!(flags & OPT_QUIET))
 			printf(format, displaypath);
 
+		submodule_unset_core_worktree(sub);
+
 		strbuf_release(&sb_rm);
 	}
 
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 51d4555549..5b56b23166 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
 	then
 		mkdir -p submodule_update/.git/modules/sub1/modules &&
 		cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
-		GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+		# core.worktree is unset for sub2 as it is not checked out
 	fi &&
 	# indicate we are interested in the submodule:
 	git -C submodule_update config submodule.sub1.url "bogus" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 76a7cb0af7..aba2d4d6ee 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -984,6 +984,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
 	rmdir init
 '
 
+test_expect_success 'submodule deinit should unset core.worktree' '
+	test_path_is_file .git/modules/example/config &&
+	test_must_fail git config -f .git/modules/example/config core.worktree
+'
+
 test_expect_success 'submodule deinit from subdirectory' '
 	git submodule update --init &&
 	git config submodule.example.foo bar &&
-- 
2.20.0.rc2.403.gdbc3b29805-goog
Junio C Hamano· Dec 8, 2018, 07:03 UTC · re: Stefan Beller · lore

Re: [PATCH 4/4] submodule deinit: unset core.worktree

Stefan Beller <sbeller@google.com> writes:
Show 11 quoted lines
> This re-introduces 984cd77ddb (submodule deinit: unset core.worktree,
> 2018-06-18), which was reverted as part of f178c13fda (Revert "Merge
> branch 'sb/submodule-core-worktree'", 2018-09-07)
>
> The whole series was reverted as the offending commit e98317508c
> (submodule: ensure core.worktree is set after update, 2018-06-18)
> was relied on by other commits such as 984cd77ddb.
>
> Keep the offending commit reverted, but its functionality came back via
> 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17), such
> that we can reintroduce 984cd77ddb now.

None of the above three explains the most important thing directly, so readers fail to grasp what the main theme of the three-patch series is, without looking at the commits that were reverted already.

Is the theme of the overall series to make sure core.worktree is set to point at the working tree when submodule's working tree is instantiated, and unset it when it is not?

2/4 was also explained (in the original) that it wants to unset and did so when "move_head" gets called. This one does the unset when a submodule is deinited. Are these the only two cases a submodule loses its working tree? If so, the log message for this step should declare victory by ending with something like

	... as we covered the only other case in which a submodule
	loses its working tree in the earlier step (i.e. switching
	branches of top-level project to move to a commit that did
	not have the submodule), this makes the code always maintain
	core.worktree correctly unset when there is no working tree
	for a submodule.

Thanks. I think I agree with what the series wants to do (if I read what it wants to do correctly, that is).

Show 49 quoted lines
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  builtin/submodule--helper.c | 2 ++
>  t/lib-submodule-update.sh   | 2 +-
>  t/t7400-submodule-basic.sh  | 5 +++++
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 31ac30cf2f..672b74db89 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1131,6 +1131,8 @@ static void deinit_submodule(const char *path, const char *prefix,
>  		if (!(flags & OPT_QUIET))
>  			printf(format, displaypath);
>  
> +		submodule_unset_core_worktree(sub);
> +
>  		strbuf_release(&sb_rm);
>  	}
>  
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 51d4555549..5b56b23166 100755
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
>  	then
>  		mkdir -p submodule_update/.git/modules/sub1/modules &&
>  		cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
> -		GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
> +		# core.worktree is unset for sub2 as it is not checked out
>  	fi &&
>  	# indicate we are interested in the submodule:
>  	git -C submodule_update config submodule.sub1.url "bogus" &&
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 76a7cb0af7..aba2d4d6ee 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -984,6 +984,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
>  	rmdir init
>  '
>  
> +test_expect_success 'submodule deinit should unset core.worktree' '
> +	test_path_is_file .git/modules/example/config &&
> +	test_must_fail git config -f .git/modules/example/config core.worktree
> +'
> +
>  test_expect_success 'submodule deinit from subdirectory' '
>  	git submodule update --init &&
>  	git config submodule.example.foo bar &&
Junio C Hamano· Dec 8, 2018, 05:57 UTC · re: Stefan Beller · lore

Re: [PATCH 0/4]

Stefan Beller <sbeller@google.com> writes:
Show 15 quoted lines
> A couple days before the 2.19 release we had a bug report about
> broken submodules[1] and reverted[2] the commits leading up to them.
>
> The behavior of said bug fixed itself by taking a different approach[3],
> specifically by a weaker enforcement of having `core.worktree` set in a
> submodule [4].
>
> The revert [2] was overly broad as we neared the release, such that we wanted
> to rather keep the known buggy behavior of always having `core.worktree` set,
> rather than figuring out how to fix the new bug of having 'git submodule update'
> not working in old style repository setups.
>
> This series re-introduces those reverted patches, with no changes in code,
> but with drastically changed commit messages, as those focus on why it is safe
> to re-introduce them instead of explaining the desire for the change.

The above was a bit too cryptic for me to grok, so let me try rephrasing to see if I got them all correctly.

 - three-patch series leading to 984cd77ddb were meant to fix some
   bug, but the series itself was buggy and caused problems; we got
   rid of them
 - the problem 984cd77ddb wanted to fix was fixed differently
   without reintroducing the problem three-patch series introduced.
   That fix is already with us since 4d6d6ef1fc.
 - now these three changes that were problematic in the past is
   resent without any update (other than that it has one preparatory
   patch to add tests).

Is that what is going on? Obviously I am not getting "the other" benefit we wanted to gain out of these three patches (because the above description fails to explain what that is), other than to fix the issue that was fixed by 4d6d6ef1fc.

Sorry for being puzzled...
Show 18 quoted lines
> [1] https://public-inbox.org/git/2659750.rG6xLiZASK@twilight
> [2] f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07)
> [3] 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17)
> [4] 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)
>
> Stefan Beller (4):
>   submodule update: add regression test with old style setups
>   submodule: unset core.worktree if no working tree is present
>   submodule--helper: fix BUG message in ensure_core_worktree
>   submodule deinit: unset core.worktree
>
>  builtin/submodule--helper.c        |  4 +++-
>  submodule.c                        | 14 ++++++++++++++
>  submodule.h                        |  2 ++
>  t/lib-submodule-update.sh          |  5 +++--
>  t/t7400-submodule-basic.sh         |  5 +++++
>  t/t7412-submodule-absorbgitdirs.sh |  7 ++++++-
>  6 files changed, 33 insertions(+), 4 deletions(-)
Stefan Beller· Dec 12, 2018, 22:35 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4]

On Fri, Dec 7, 2018 at 9:57 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 25 quoted lines
>
> Stefan Beller <sbeller@google.com> writes:
>
> > A couple days before the 2.19 release we had a bug report about
> > broken submodules[1] and reverted[2] the commits leading up to them.
> >
> > The behavior of said bug fixed itself by taking a different approach[3],
> > specifically by a weaker enforcement of having `core.worktree` set in a
> > submodule [4].
> >
> > The revert [2] was overly broad as we neared the release, such that we wanted
> > to rather keep the known buggy behavior of always having `core.worktree` set,
> > rather than figuring out how to fix the new bug of having 'git submodule update'
> > not working in old style repository setups.
> >
> > This series re-introduces those reverted patches, with no changes in code,
> > but with drastically changed commit messages, as those focus on why it is safe
> > to re-introduce them instead of explaining the desire for the change.
>
> The above was a bit too cryptic for me to grok, so let me try
> rephrasing to see if I got them all correctly.
>
>  - three-patch series leading to 984cd77ddb were meant to fix some
>    bug, but the series itself was buggy and caused problems; we got
>    rid of them
yes.
>  - the problem 984cd77ddb wanted to fix was fixed differently
e98317508c02*
>    without reintroducing the problem three-patch series introduced.
>    That fix is already with us since 4d6d6ef1fc.
yes.
>  - now these three changes that were problematic in the past is
>    resent without any update (other than that it has one preparatory
>    patch to add tests).

One of the three changes was problematic, (e98317508c02), the other two are good (in company of the third).

But those two were not good on their own, which is why we reverted all three at once.

Now that we have a different approach for the third, we could re-introduce the two. (4fa4f90ccd8, 984cd77ddbf0)

We do that, but with precaution (an extra test); additional careful reading found a typo, hence we have "a third" patch, but that is totally different than what above was referred to "one of three".

> Is that what is going on?  Obviously I am not getting "the other"
> benefit we wanted to gain out of these three patches (because the
> above description fails to explain what that is), other than to fix
> the issue that was fixed by 4d6d6ef1fc.

The other benefit refers to 7e25437d35 (Merge branch 'sb/submodule-core-worktree', 2018-07-18) which was reverted as a whole. It's goal was to handle core.worktree appropriately.

(Instead of having it there all the time, only have it when a working tree is present)

> Sorry for being puzzled...

This means I need to revamp the commit messages and cover letter altogether.

Stefan
Junio C Hamano· Dec 13, 2018, 03:15 UTC · re: Stefan Beller · lore

Re: [PATCH 0/4]

Stefan Beller <sbeller@google.com> writes:
Show 6 quoted lines
>>  - now these three changes that were problematic in the past is
>>    resent without any update (other than that it has one preparatory
>>    patch to add tests).
>
> One of the three changes was problematic, (e98317508c02),
> the other two are good (in company of the third).
Ah, that is what I failed to read.
> This means I need to revamp the commit messages and
> cover letter altogether.
I guess that would help future readers.  Thanks.
Stefan Beller· Dec 14, 2018, 23:59 UTC · re: Junio C Hamano · lore

[PATCH 0/4] submodules: unset core.worktree when no working tree present

v2: I reworded the commit messages to explain the patches from the ground up instead of only linking to the old commits, that got reverted.

> Just pretend that the ealier commits and their reversion never
> happened, and further pretend that we are doing the best thing that
> should happen to our codebase.

I disagree with that first stance (I can freely admit those commits happened), but agree on the second point, so I explained why the code is the best for the code base now. So I kept those pointers in there, too, to make it easier for future code archeologists.

v1:

A couple days before the 2.19 release we had a bug report about broken submodules[1] and reverted[2] the commits leading up to them.

The behavior of said bug fixed itself by taking a different approach[3], specifically by a weaker enforcement of having `core.worktree` set in a submodule [4].

The revert [2] was overly broad as we neared the release, such that we wanted to rather keep the known buggy behavior of always having `core.worktree` set, rather than figuring out how to fix the new bug of having 'git submodule update' not working in old style repository setups.

This series re-introduces those reverted patches, with no changes in code, but with drastically changed commit messages, as those focus on why it is safe to re-introduce them instead of explaining the desire for the change.

[1] https://public-inbox.org/git/2659750.rG6xLiZASK@twilight
[2] f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07)
[3] 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17)
[4] 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)
Stefan Beller (4):
  submodule update: add regression test with old style setups
  submodule: unset core.worktree if no working tree is present
  submodule--helper: fix BUG message in ensure_core_worktree
  submodule deinit: unset core.worktree
 builtin/submodule--helper.c        |  4 +++-
 submodule.c                        | 14 ++++++++++++++
 submodule.h                        |  2 ++
 t/lib-submodule-update.sh          |  5 +++--
 t/t7400-submodule-basic.sh         |  5 +++++
 t/t7412-submodule-absorbgitdirs.sh |  7 ++++++-
 6 files changed, 33 insertions(+), 4 deletions(-)
-- 
2.20.0.405.gbc1bbc6f85-goog
Stefan Beller· Dec 14, 2018, 23:59 UTC · re: Stefan Beller · lore

[PATCH 1/4] submodule update: add regression test with old style setups

As f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07) was produced shortly before a release, nobody asked for a regression test to be included. Add a regression test that makes sure that the invocation of `git submodule update` on old setups doesn't produce errors as pointed out in f178c13fda.

The place to add such a regression test may look odd in t7412, but that is the best place as there we setup old style submodule setups explicitly.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t7412-submodule-absorbgitdirs.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Show changes to t/t7412-submodule-absorbgitdirs.sh +6 −1
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index ce74c12da2..1cfa150768 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
 	GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
 		core.worktree "../../../nested" &&
 	# make sure this re-setup is correct
-	git status --ignore-submodules=none
+	git status --ignore-submodules=none &&
+
+	# also make sure this old setup does not regress
+	git submodule update --init --recursive >out 2>err &&
+	test_must_be_empty out &&
+	test_must_be_empty err
 '
 
 test_expect_success 'absorb the git dir in a nested submodule' '
-- 
2.20.0.405.gbc1bbc6f85-goog
Junio C Hamano· Dec 26, 2018, 18:21 UTC · re: Stefan Beller · lore

Re: [PATCH 1/4] submodule update: add regression test with old style setups

Stefan Beller <sbeller@google.com> writes:
> The place to add such a regression test may look odd in t7412, but
> that is the best place as there we setup old style submodule setups
> explicitly.
Makes sense; thanks.
Show 24 quoted lines
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  t/t7412-submodule-absorbgitdirs.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
> index ce74c12da2..1cfa150768 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
>  	GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
>  		core.worktree "../../../nested" &&
>  	# make sure this re-setup is correct
> -	git status --ignore-submodules=none
> +	git status --ignore-submodules=none &&
> +
> +	# also make sure this old setup does not regress
> +	git submodule update --init --recursive >out 2>err &&
> +	test_must_be_empty out &&
> +	test_must_be_empty err
>  '
>  
>  test_expect_success 'absorb the git dir in a nested submodule' '
Stefan Beller· Dec 14, 2018, 23:59 UTC · re: Stefan Beller · lore

[PATCH 2/4] submodule: unset core.worktree if no working tree is present

When a submodules work tree is removed, we should unset its core.worktree setting as the worktree is no longer present. This is not just in line with the conceptual view of submodules, but it fixes an inconvenience for looking at submodules that are not checked out:

    git clone --recurse-submodules git://github.com/git/git && cd git &&
    git checkout --recurse-submodules v2.13.0
    git -C .git/modules/sha1collisiondetection log
    fatal: cannot chdir to '../../../sha1collisiondetection': \
        No such file or directory

With this patch applied, the final call to git log works instead of dying in its setup, as the checkout will unset the core.worktree setting such that following log will be run in a bare repository.

This patch covers all commands that are in the unpack machinery, i.e. checkout, read-tree, reset. A follow up patch will address "git submodule deinit", which will also make use of the new function submodule_unset_core_worktree(), which is why we expose it in this patch.

This patch was authored as 4fa4f90ccd (submodule: unset core.worktree if no working tree is present, 2018-06-12), which was reverted as part of f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07). The revert was needed as the nearby commit e98317508c (submodule: ensure core.worktree is set after update, 2018-06-18) is faulty and at the time of 7e25437d35 (Merge branch 'sb/submodule-core-worktree', 2018-07-18) we could not revert the faulty commit only, as they were depending on each other: If core.worktree is unset, we have to have ways to ensure that it is set again once the working tree reappears again.

Now that 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17), specifically 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13) is present, we already check and ensure core.worktree is set when populating a new work tree, such that we can re-introduce the commits that unset core.worktree when removing the worktree.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c               | 14 ++++++++++++++
 submodule.h               |  2 ++
 t/lib-submodule-update.sh |  3 ++-
 3 files changed, 18 insertions(+), 1 deletion(-)
Show changes to 3 files +18 −1

submodule.c, submodule.h, t/lib-submodule-update.sh

diff --git a/submodule.c b/submodule.c
index 6415cc5580..d393e947e6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1561,6 +1561,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
 	return ret;
 }
 
+void submodule_unset_core_worktree(const struct submodule *sub)
+{
+	char *config_path = xstrfmt("%s/modules/%s/config",
+				    get_git_common_dir(), sub->name);
+
+	if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
+		warning(_("Could not unset core.worktree setting in submodule '%s'"),
+			  sub->path);
+
+	free(config_path);
+}
+
 static const char *get_super_prefix_or_empty(void)
 {
 	const char *s = get_super_prefix();
@@ -1726,6 +1738,8 @@ int submodule_move_head(const char *path,
 
 			if (is_empty_dir(path))
 				rmdir_or_warn(path);
+
+			submodule_unset_core_worktree(sub);
 		}
 	}
 out:
diff --git a/submodule.h b/submodule.h
index a680214c01..9e18e9b807 100644
--- a/submodule.h
+++ b/submodule.h
@@ -131,6 +131,8 @@ int submodule_move_head(const char *path,
 			const char *new_head,
 			unsigned flags);
 
+void submodule_unset_core_worktree(const struct submodule *sub);
+
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific environment variables, but
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 016391723c..51d4555549 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
 			git branch -t remove_sub1 origin/remove_sub1 &&
 			$command remove_sub1 &&
 			test_superproject_content origin/remove_sub1 &&
-			! test -e sub1
+			! test -e sub1 &&
+			test_must_fail git config -f .git/modules/sub1/config core.worktree
 		)
 	'
 	# ... absorbing a .git directory along the way.
-- 
2.20.0.405.gbc1bbc6f85-goog
Junio C Hamano· Dec 26, 2018, 18:27 UTC · re: Stefan Beller · lore

Re: [PATCH 2/4] submodule: unset core.worktree if no working tree is present

Stefan Beller <sbeller@google.com> writes:
Show 14 quoted lines
> 2018-09-07). The revert was needed as the nearby commit e98317508c
> (submodule: ensure core.worktree is set after update, 2018-06-18) is
> faulty and at the time of 7e25437d35 (Merge branch
> 'sb/submodule-core-worktree', 2018-07-18) we could not revert the faulty
> commit only, as they were depending on each other: If core.worktree is
> unset, we have to have ways to ensure that it is set again once
> the working tree reappears again.
>
> Now that 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17),
> specifically 74d4731da1 (submodule--helper: replace
> connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13) is
> present, we already check and ensure core.worktree is set when
> populating a new work tree, such that we can re-introduce the commits
> that unset core.worktree when removing the worktree.
Cleanly explained.  Will queue.  Thanks.
Show 68 quoted lines
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  submodule.c               | 14 ++++++++++++++
>  submodule.h               |  2 ++
>  t/lib-submodule-update.sh |  3 ++-
>  3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/submodule.c b/submodule.c
> index 6415cc5580..d393e947e6 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1561,6 +1561,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
>  	return ret;
>  }
>  
> +void submodule_unset_core_worktree(const struct submodule *sub)
> +{
> +	char *config_path = xstrfmt("%s/modules/%s/config",
> +				    get_git_common_dir(), sub->name);
> +
> +	if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
> +		warning(_("Could not unset core.worktree setting in submodule '%s'"),
> +			  sub->path);
> +
> +	free(config_path);
> +}
> +
>  static const char *get_super_prefix_or_empty(void)
>  {
>  	const char *s = get_super_prefix();
> @@ -1726,6 +1738,8 @@ int submodule_move_head(const char *path,
>  
>  			if (is_empty_dir(path))
>  				rmdir_or_warn(path);
> +
> +			submodule_unset_core_worktree(sub);
>  		}
>  	}
>  out:
> diff --git a/submodule.h b/submodule.h
> index a680214c01..9e18e9b807 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -131,6 +131,8 @@ int submodule_move_head(const char *path,
>  			const char *new_head,
>  			unsigned flags);
>  
> +void submodule_unset_core_worktree(const struct submodule *sub);
> +
>  /*
>   * Prepare the "env_array" parameter of a "struct child_process" for executing
>   * a submodule by clearing any repo-specific environment variables, but
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 016391723c..51d4555549 100755
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
>  			git branch -t remove_sub1 origin/remove_sub1 &&
>  			$command remove_sub1 &&
>  			test_superproject_content origin/remove_sub1 &&
> -			! test -e sub1
> +			! test -e sub1 &&
> +			test_must_fail git config -f .git/modules/sub1/config core.worktree
>  		)
>  	'
>  	# ... absorbing a .git directory along the way.
Stefan Beller· Dec 14, 2018, 23:59 UTC · re: Stefan Beller · lore

[PATCH 4/4] submodule deinit: unset core.worktree

When a submodule is deinit'd, the working tree is gone, so the setting of core.worktree is bogus. Unset it. As we covered the only other case in which a submodule loses its working tree in the earlier step (i.e. switching branches of top-level project to move to a commit that did not have the submodule), this makes the code always maintain core.worktree correctly unset when there is no working tree for a submodule.

This re-introduces 984cd77ddb (submodule deinit: unset core.worktree, 2018-06-18), which was reverted as part of f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07)

The whole series was reverted as the offending commit e98317508c (submodule: ensure core.worktree is set after update, 2018-06-18) was relied on by other commits such as 984cd77ddb.

Keep the offending commit reverted, but its functionality came back via 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17), such that we can reintroduce 984cd77ddb now.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 2 ++
 t/lib-submodule-update.sh   | 2 +-
 t/t7400-submodule-basic.sh  | 5 +++++
 3 files changed, 8 insertions(+), 1 deletion(-)
Show changes to 3 files +8 −1

builtin/submodule--helper.c, t/lib-submodule-update.sh, t/t7400-submodule-basic.sh

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 31ac30cf2f..672b74db89 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1131,6 +1131,8 @@ static void deinit_submodule(const char *path, const char *prefix,
 		if (!(flags & OPT_QUIET))
 			printf(format, displaypath);
 
+		submodule_unset_core_worktree(sub);
+
 		strbuf_release(&sb_rm);
 	}
 
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 51d4555549..5b56b23166 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
 	then
 		mkdir -p submodule_update/.git/modules/sub1/modules &&
 		cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
-		GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+		# core.worktree is unset for sub2 as it is not checked out
 	fi &&
 	# indicate we are interested in the submodule:
 	git -C submodule_update config submodule.sub1.url "bogus" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 76a7cb0af7..aba2d4d6ee 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -984,6 +984,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
 	rmdir init
 '
 
+test_expect_success 'submodule deinit should unset core.worktree' '
+	test_path_is_file .git/modules/example/config &&
+	test_must_fail git config -f .git/modules/example/config core.worktree
+'
+
 test_expect_success 'submodule deinit from subdirectory' '
 	git submodule update --init &&
 	git config submodule.example.foo bar &&
-- 
2.20.0.405.gbc1bbc6f85-goog
Stefan Beller· Dec 14, 2018, 23:59 UTC · re: Stefan Beller · lore

[PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree

74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13) missed to update the BUG message. Fix it.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/submodule--helper.c +1 −1
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d38113a31a..31ac30cf2f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2045,7 +2045,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
 	struct repository subrepo;
 
 	if (argc != 2)
-		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
+		BUG("submodule--helper ensure-core-worktree <path>");
 
 	path = argv[1];
 
-- 
2.20.0.405.gbc1bbc6f85-goog

← back to recent threads