git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: git@vger.kernel.org
Cc: Emily Shaffer <emilyshaffer@google.com>
Subject: [PATCH v6 5/5] submodule: use config to find superproject worktree
Date: Tue, 16 Nov 2021 16:57:01 -0800	[thread overview]
Message-ID: <20211117005701.371808-6-emilyshaffer@google.com> (raw)
In-Reply-To: <20211117005701.371808-1-emilyshaffer@google.com>

Now that submodule.superprojectGitDir is being treated as the point of
truth for whether a repo is a submodule or not, let's use it in `git
rev-parse --show-superproject-working-tree`.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>

---

This commit may be more of an RFC - to demonstrate what life looks like
if we use submodule.superprojectGitDir as the source of truth. But since
'git rev-parse --show-superproject-working-tree' is used in a lot of
scripts in the wild[1], I'm not so sure it's a great example.

To be honest, I'd prefer to die("Try running 'git submodule update'")
here, but I don't think that's very script-friendly. However, falling
back on the old implementation kind of undermines the idea of treating
submodule.superprojectGitDir as the point of truth. One thought I did
have was to put that error message in builtin/rev-parse.c instead, and
print it to stderr (per usual with user-visible messages) so it doesn't
interfere with scripts, but gives a hint for debugging.

Another thought - captured by the NEEDSWORK in the diff - was that we
could "heal" by adding that config after we know the worktree of the
superproject.

Or, it could be that it won't be a problem for a long time, as anybody
running 'git submodule update' will eventually have that config
specified - that's why I included the traces, so we could try and get an
understanding of how long repos remain in this state where they have
submodules but nobody ran 'git submodule update'. But that will only
give me visibility into submodule users at Google, who we expect to be
making a lot of workflow changes soon, anyway.

1: https://github.com/search?q=%22--show-superproject-working-tree%22&type=code
---
 submodule.c          | 85 +++++++++++++++++++++++++++++++++++++++++++-
 t/t1500-rev-parse.sh |  9 +++++
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/submodule.c b/submodule.c
index d7395c7551..ad95cdda07 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2236,7 +2236,7 @@ void absorb_git_dir_into_superproject(const char *path,
 	}
 }
 
-int get_superproject_working_tree(struct strbuf *buf)
+static int get_superproject_working_tree_from_fs(struct strbuf *buf)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf sb = STRBUF_INIT;
@@ -2320,6 +2320,89 @@ int get_superproject_working_tree(struct strbuf *buf)
 	return ret;
 }
 
+int get_superproject_working_tree(struct strbuf *buf)
+{
+	char *super_gitdir = NULL;
+	const char *cwd = xgetcwd();
+	struct child_process cp = CHILD_PROCESS_INIT;
+	struct strbuf absolute_super_gitdir = STRBUF_INIT;
+	struct strbuf out = STRBUF_INIT;
+	struct string_list lines = STRING_LIST_INIT_NODUP;
+	struct string_list_item *it = NULL;
+	const char *wt_prefix = "worktree ";
+	int rc = 0;
+
+
+	/* Do we know we have a superproject? */
+	if (git_config_get_string("submodule.superprojectgitdir", &super_gitdir))
+		goto fallback;
+
+	strbuf_addf(&absolute_super_gitdir, "%s/%s", get_git_dir(), super_gitdir);
+
+	/*
+	 * NEEDSWORK: This is a child process call because worktree.c still
+	 * relies heavily on the_repository. If we can make worktree.c work with
+	 * a repository object - or, better yet, a gitdir path alone - then we
+	 * can drop the child process and ask worktree.c directly.
+	 *
+	 * Alternatively, if 'git worktree' learns a way to say 'the worktree
+	 * associated with this gitdir' instead of 'all worktrees', that would
+	 * be clearer because we could skip the foreach below.
+	 */
+
+	/* Get the output of `git worktree list` from that superproject */
+	prepare_other_repo_env(&cp.env_array, absolute_super_gitdir.buf);
+	strvec_pushl(&cp.args, "-C", absolute_super_gitdir.buf, "worktree", "list",
+		    "--porcelain", NULL);
+
+	cp.git_cmd = 1;
+	if (capture_command(&cp, &out, 0) ||
+	    !string_list_split_in_place(&lines, out.buf, '\n', -1))
+		die("submodule.superprojectGitDir is stale; run 'git submodule "
+		    "update' from the superproject.");
+
+	for_each_string_list_item(it, &lines) {
+		char *trimmed;
+		/*
+		 * Lines containing worktree dirs look like
+		 * 'worktree /some/path'
+		 */
+		if (strncmp(it->string, wt_prefix, strlen(wt_prefix)))
+			continue;
+		trimmed = it->string + strlen(wt_prefix);
+
+		/*
+		 * '/some/path/to/sub' is a prefix of '/some/path' - that's our
+		 * worktree
+		 */
+		if (!strncmp(cwd, trimmed, strlen(trimmed))) {
+			strbuf_addstr(buf, trimmed);
+			rc = 1;
+			goto cleanup;
+		}
+	}
+
+fallback:
+	/*
+	 * Because a submodule may have been created before
+	 * submodule.superprojectGitDir was introduced, fall back on checking
+	 * whether ../ is the superproject.
+	 */
+	trace2_data_intmax("submodule", the_repository,
+			   "get_superproject_wt/config_missing", rc);
+	rc = get_superproject_working_tree_from_fs(buf);
+
+	/*
+	 * NEEDSWORK: Is it possible to teach a submodule the path to its
+	 * superproject when this happens?
+	 */
+
+cleanup:
+	string_list_clear(&lines, 0);
+	strbuf_release(&out);
+	return rc;
+}
+
 /*
  * Put the gitdir for a submodule (given relative to the main
  * repository worktree) into `buf`, or return -1 on error.
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 1c2df08333..e569910289 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -247,6 +247,15 @@ test_expect_success 'showing the superproject correctly' '
 	test_cmp expect out
 '
 
+test_expect_success 'show the superproject correctly without superprojectGitDir' '
+	# repos created before submodule.superprojectGitDir was introduced which
+	# have not been `git submodule update`-ed lately must not break
+	git -C super/dir/sub config --unset submodule.superprojectGitDir &&
+	echo $(pwd)/super >expect &&
+	git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
+	test_cmp expect out
+'
+
 # at least one external project depends on this behavior:
 test_expect_success 'rev-parse --since= unsqueezed ordering' '
 	x1=--since=1970-01-01T00:00:01Z &&
-- 
2.34.0.rc1.387.gb447b232ab-goog


  parent reply	other threads:[~2021-11-17  0:57 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-17  0:56 [PATCH v6 0/5] teach submodules to know they're submodules Emily Shaffer
2021-11-17  0:56 ` [PATCH v6 1/5] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2021-11-17  0:56 ` [PATCH v6 2/5] introduce submodule.superprojectGitDir record Emily Shaffer
2021-11-17 23:43   ` Jonathan Tan
2021-11-17  0:56 ` [PATCH v6 3/5] submodule: record superproject gitdir during absorbgitdirs Emily Shaffer
2021-11-17  0:57 ` [PATCH v6 4/5] submodule: record superproject gitdir during 'update' Emily Shaffer
2021-11-17  0:57 ` Emily Shaffer [this message]
2021-11-17 11:43 ` [RFC PATCH 0/2] submodule: test what happens if submodule.superprojectGitDir isn't around Ævar Arnfjörð Bjarmason
2021-11-17 11:43   ` [RFC PATCH 1/2] submodule tests: fix potentially broken "config .. --unset" Ævar Arnfjörð Bjarmason
2021-11-17 11:43   ` [RFC PATCH 2/2] submodule: add test mode for checking absence of "superProjectGitDir" Ævar Arnfjörð Bjarmason
2021-11-23 20:08   ` [RFC PATCH 0/2] submodule: test what happens if submodule.superprojectGitDir isn't around Emily Shaffer
2021-11-24  1:38     ` Ævar Arnfjörð Bjarmason
2021-11-17 23:28 ` [PATCH v6 0/5] teach submodules to know they're submodules Jonathan Tan
2021-11-23 20:28   ` Emily Shaffer
2022-02-03 21:59 ` Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 1/4] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 2/4] introduce submodule.superprojectGitDir record Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 3/4] submodule: record superproject gitdir during absorbgitdirs Emily Shaffer
2022-02-03 21:59   ` [PATCH v7 4/4] submodule: record superproject gitdir during 'update' Emily Shaffer
2022-02-03 22:39   ` [PATCH v6 0/5] teach submodules to know they're submodules Junio C Hamano
2022-02-04  1:15   ` Ævar Arnfjörð Bjarmason
2022-02-04 16:20     ` Junio C Hamano
2022-02-07 19:56     ` Jonathan Nieder
2022-02-07 23:21       ` Junio C Hamano
2022-02-08  1:18         ` Jonathan Nieder
2022-02-08 18:24           ` Junio C Hamano
2022-02-10 22:12             ` Emily Shaffer
2022-02-10 22:53               ` Jonathan Nieder
2022-02-12 20:35       ` Ævar Arnfjörð Bjarmason
2022-02-13  6:25         ` Junio C Hamano
2022-03-01  0:26   ` [PATCH v8 0/3] " Emily Shaffer
2022-03-01  0:26     ` [PATCH v8 1/3] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2022-03-01  0:26     ` [PATCH v8 2/3] introduce submodule.hasSuperproject record Emily Shaffer
2022-03-01  7:00       ` Junio C Hamano
2022-03-08 20:04         ` Emily Shaffer
2022-03-08 22:13       ` Glen Choo
2022-03-08 22:29         ` Glen Choo
2022-03-01  0:26     ` [PATCH v8 3/3] rev-parse: short-circuit superproject worktree when config unset Emily Shaffer
2022-03-01  7:06       ` Junio C Hamano
2022-03-09  0:38         ` Emily Shaffer
2022-03-01  3:08     ` [PATCH v8 0/3] teach submodules to know they're submodules Junio C Hamano
2022-03-08 18:54       ` Emily Shaffer
2022-03-10  0:44     ` [PATCH v9 " Emily Shaffer
2022-03-10  0:44       ` [PATCH v9 1/3] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2022-03-10  0:44       ` [PATCH v9 2/3] introduce submodule.hasSuperproject record Emily Shaffer
2022-03-10  2:09         ` Junio C Hamano
2022-03-10 21:29           ` Glen Choo
2022-03-10 21:40           ` Glen Choo
2022-03-10 22:10             ` Junio C Hamano
2022-03-10 23:42               ` Glen Choo
2022-03-10 23:53                 ` Glen Choo
2022-03-15 20:48                   ` Emily Shaffer
2022-03-15 20:56                     ` Emily Shaffer
2022-03-15 21:19                       ` Glen Choo
2022-03-15 18:39               ` Emily Shaffer
2022-03-15 19:19                 ` Junio C Hamano
2022-03-10  2:32         ` Junio C Hamano
2022-03-10 21:54         ` Glen Choo
2022-03-15 18:27           ` Emily Shaffer
2022-03-10  0:44       ` [PATCH v9 3/3] rev-parse: short-circuit superproject worktree when config unset Emily Shaffer
2022-03-10  1:47         ` Junio C Hamano
2022-03-10  4:39           ` Eric Sunshine
2022-03-11  9:09       ` [PATCH v9 0/3] teach submodules to know they're submodules Ævar Arnfjörð Bjarmason
2022-03-13  5:43         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211117005701.371808-6-emilyshaffer@google.com \
    --to=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).