All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>, peff@peff.net, newren@gmail.com
Subject: [PATCH v2 5/9] merge-{ort,recursive}: remove add_submodule_odb()
Date: Tue, 28 Sep 2021 13:10:51 -0700	[thread overview]
Message-ID: <dd70820d66307b3943f3c02d6672e99ddd01732f.1632859148.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1632859147.git.jonathantanmy@google.com>

After the parent commit and some of its ancestors, the only place
commits are being accessed through alternates is in the user-facing
message formatting code. Fix those, and remove the add_submodule_odb()
calls.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 merge-ort.c                | 18 ++++-------------
 merge-recursive.c          | 41 +++++++++++++++++++-------------------
 strbuf.c                   | 12 ++++++++---
 strbuf.h                   |  6 ++++--
 t/t6437-submodule-merge.sh |  3 +++
 5 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/merge-ort.c b/merge-ort.c
index b88475475d..fbc5c204c1 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -609,6 +609,7 @@ static int err(struct merge_options *opt, const char *err, ...)
 
 static void format_commit(struct strbuf *sb,
 			  int indent,
+			  struct repository *repo,
 			  struct commit *commit)
 {
 	struct merge_remote_desc *desc;
@@ -622,7 +623,7 @@ static void format_commit(struct strbuf *sb,
 		return;
 	}
 
-	format_commit_message(commit, "%h %s", sb, &ctx);
+	repo_format_commit_message(repo, commit, "%h %s", sb, &ctx);
 	strbuf_addch(sb, '\n');
 }
 
@@ -1578,17 +1579,6 @@ static int merge_submodule(struct merge_options *opt,
 	if (is_null_oid(b))
 		return 0;
 
-	/*
-	 * NEEDSWORK: Remove this when all submodule object accesses are
-	 * through explicitly specified repositores.
-	 */
-	if (add_submodule_odb(path)) {
-		path_msg(opt, path, 0,
-			 _("Failed to merge submodule %s (not checked out)"),
-			 path);
-		return 0;
-	}
-
 	if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) {
 		path_msg(opt, path, 0,
 				_("Failed to merge submodule %s (not checked out)"),
@@ -1653,7 +1643,7 @@ static int merge_submodule(struct merge_options *opt,
 		break;
 
 	case 1:
-		format_commit(&sb, 4,
+		format_commit(&sb, 4, &subrepo,
 			      (struct commit *)merges.objects[0].item);
 		path_msg(opt, path, 0,
 			 _("Failed to merge submodule %s, but a possible merge "
@@ -1670,7 +1660,7 @@ static int merge_submodule(struct merge_options *opt,
 		break;
 	default:
 		for (i = 0; i < merges.nr; i++)
-			format_commit(&sb, 4,
+			format_commit(&sb, 4, &subrepo,
 				      (struct commit *)merges.objects[i].item);
 		path_msg(opt, path, 0,
 			 _("Failed to merge submodule %s, but multiple "
diff --git a/merge-recursive.c b/merge-recursive.c
index 5a2d8a60c0..80594153f1 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -334,7 +334,9 @@ static void output(struct merge_options *opt, int v, const char *fmt, ...)
 		flush_output(opt);
 }
 
-static void output_commit_title(struct merge_options *opt, struct commit *commit)
+static void repo_output_commit_title(struct merge_options *opt,
+				     struct repository *repo,
+				     struct commit *commit)
 {
 	struct merge_remote_desc *desc;
 
@@ -343,23 +345,29 @@ static void output_commit_title(struct merge_options *opt, struct commit *commit
 	if (desc)
 		strbuf_addf(&opt->obuf, "virtual %s\n", desc->name);
 	else {
-		strbuf_add_unique_abbrev(&opt->obuf, &commit->object.oid,
-					 DEFAULT_ABBREV);
+		strbuf_repo_add_unique_abbrev(&opt->obuf, repo,
+					      &commit->object.oid,
+					      DEFAULT_ABBREV);
 		strbuf_addch(&opt->obuf, ' ');
-		if (parse_commit(commit) != 0)
+		if (repo_parse_commit(repo, commit) != 0)
 			strbuf_addstr(&opt->obuf, _("(bad commit)\n"));
 		else {
 			const char *title;
-			const char *msg = get_commit_buffer(commit, NULL);
+			const char *msg = repo_get_commit_buffer(repo, commit, NULL);
 			int len = find_commit_subject(msg, &title);
 			if (len)
 				strbuf_addf(&opt->obuf, "%.*s\n", len, title);
-			unuse_commit_buffer(commit, msg);
+			repo_unuse_commit_buffer(repo, commit, msg);
 		}
 	}
 	flush_output(opt);
 }
 
+static void output_commit_title(struct merge_options *opt, struct commit *commit)
+{
+	repo_output_commit_title(opt, the_repository, commit);
+}
+
 static int add_cacheinfo(struct merge_options *opt,
 			 const struct diff_filespec *blob,
 			 const char *path, int stage, int refresh, int options)
@@ -1149,14 +1157,14 @@ static int find_first_merges(struct repository *repo,
 	return result->nr;
 }
 
-static void print_commit(struct commit *commit)
+static void print_commit(struct repository *repo, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
 	ctx.date_mode.type = DATE_NORMAL;
 	/* FIXME: Merge this with output_commit_title() */
 	assert(!merge_remote_util(commit));
-	format_commit_message(commit, " %h: %m %s", &sb, &ctx);
+	repo_format_commit_message(repo, commit, " %h: %m %s", &sb, &ctx);
 	fprintf(stderr, "%s\n", sb.buf);
 	strbuf_release(&sb);
 }
@@ -1196,15 +1204,6 @@ static int merge_submodule(struct merge_options *opt,
 	if (is_null_oid(b))
 		return 0;
 
-	/*
-	 * NEEDSWORK: Remove this when all submodule object accesses are
-	 * through explicitly specified repositores.
-	 */
-	if (add_submodule_odb(path)) {
-		output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path);
-		return 0;
-	}
-
 	if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) {
 		output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path);
 		return 0;
@@ -1229,7 +1228,7 @@ static int merge_submodule(struct merge_options *opt,
 		oidcpy(result, b);
 		if (show(opt, 3)) {
 			output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
-			output_commit_title(opt, commit_b);
+			repo_output_commit_title(opt, &subrepo, commit_b);
 		} else if (show(opt, 2))
 			output(opt, 2, _("Fast-forwarding submodule %s"), path);
 		else
@@ -1242,7 +1241,7 @@ static int merge_submodule(struct merge_options *opt,
 		oidcpy(result, a);
 		if (show(opt, 3)) {
 			output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
-			output_commit_title(opt, commit_a);
+			repo_output_commit_title(opt, &subrepo, commit_a);
 		} else if (show(opt, 2))
 			output(opt, 2, _("Fast-forwarding submodule %s"), path);
 		else
@@ -1274,7 +1273,7 @@ static int merge_submodule(struct merge_options *opt,
 	case 1:
 		output(opt, 1, _("Failed to merge submodule %s (not fast-forward)"), path);
 		output(opt, 2, _("Found a possible merge resolution for the submodule:\n"));
-		print_commit((struct commit *) merges.objects[0].item);
+		print_commit(&subrepo, (struct commit *) merges.objects[0].item);
 		output(opt, 2, _(
 		       "If this is correct simply add it to the index "
 		       "for example\n"
@@ -1287,7 +1286,7 @@ static int merge_submodule(struct merge_options *opt,
 	default:
 		output(opt, 1, _("Failed to merge submodule %s (multiple merges found)"), path);
 		for (i = 0; i < merges.nr; i++)
-			print_commit((struct commit *) merges.objects[i].item);
+			print_commit(&subrepo, (struct commit *) merges.objects[i].item);
 	}
 
 	object_array_clear(&merges);
diff --git a/strbuf.c b/strbuf.c
index c8a5789694..b22e981655 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1059,15 +1059,21 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,
 	strbuf_setlen(sb, sb->len + len);
 }
 
-void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid,
-			      int abbrev_len)
+void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo,
+				   const struct object_id *oid, int abbrev_len)
 {
 	int r;
 	strbuf_grow(sb, GIT_MAX_HEXSZ + 1);
-	r = find_unique_abbrev_r(sb->buf + sb->len, oid, abbrev_len);
+	r = repo_find_unique_abbrev_r(repo, sb->buf + sb->len, oid, abbrev_len);
 	strbuf_setlen(sb, sb->len + r);
 }
 
+void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid,
+			      int abbrev_len)
+{
+	strbuf_repo_add_unique_abbrev(sb, the_repository, oid, abbrev_len);
+}
+
 /*
  * Returns the length of a line, without trailing spaces.
  *
diff --git a/strbuf.h b/strbuf.h
index 5b1113abf8..2d9e01c16f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -634,8 +634,10 @@ void strbuf_list_free(struct strbuf **list);
  * Add the abbreviation, as generated by find_unique_abbrev, of `sha1` to
  * the strbuf `sb`.
  */
-void strbuf_add_unique_abbrev(struct strbuf *sb,
-			      const struct object_id *oid,
+struct repository;
+void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo,
+				   const struct object_id *oid, int abbrev_len);
+void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid,
 			      int abbrev_len);
 
 /**
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index e5e89c2045..178413c22f 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -5,6 +5,9 @@ test_description='merging with submodules'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
+export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
+
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-merge.sh
 
-- 
2.33.0.685.g46640cef36-goog


  parent reply	other threads:[~2021-09-28 20:11 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 16:51 [PATCH 0/9] No more adding submodule ODB as alternate Jonathan Tan
2021-09-21 16:51 ` [PATCH 1/9] refs: make _advance() check struct repo, not flag Jonathan Tan
2021-09-23  1:00   ` Junio C Hamano
2021-09-24 17:56     ` Jonathan Tan
2021-09-24 19:55       ` Junio C Hamano
2021-09-24 18:13   ` Jeff King
2021-09-24 18:28     ` Jonathan Tan
2021-09-21 16:51 ` [PATCH 2/9] refs: add repo paramater to _iterator_peel() Jonathan Tan
2021-09-21 16:51 ` [PATCH 3/9] refs iterator: support non-the_repository advance Jonathan Tan
2021-09-21 16:51 ` [PATCH 4/9] refs: teach refs_for_each_ref() arbitrary repos Jonathan Tan
2021-09-21 16:51 ` [PATCH 5/9] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-09-28  0:29   ` Elijah Newren
2021-09-21 16:51 ` [PATCH 6/9] object-file: only register submodule ODB if needed Jonathan Tan
2021-09-21 16:51 ` [PATCH 7/9] submodule: pass repo to check_has_commit() Jonathan Tan
2021-09-21 16:51 ` [PATCH 8/9] refs: change refs_for_each_ref_in() to take repo Jonathan Tan
2021-09-21 16:51 ` [PATCH 9/9] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-09-23 18:05 ` [PATCH 0/9] No more " Junio C Hamano
2021-09-28 20:10 ` [PATCH v2 " Jonathan Tan
2021-09-28 20:10   ` [PATCH v2 1/9] refs: plumb repo param in begin-iterator functions Jonathan Tan
2021-09-28 22:24     ` Junio C Hamano
2021-09-28 20:10   ` [PATCH v2 2/9] refs: teach arbitrary repo support to iterators Jonathan Tan
2021-09-28 22:35     ` Junio C Hamano
2021-09-29 17:04       ` Jonathan Tan
2021-09-28 20:10   ` [PATCH v2 3/9] refs: peeling non-the_repository iterators is BUG Jonathan Tan
2021-09-28 20:10   ` [PATCH v2 4/9] refs: teach refs_for_each_ref() arbitrary repos Jonathan Tan
2021-09-28 22:49     ` Junio C Hamano
2021-09-28 20:10   ` Jonathan Tan [this message]
2021-09-28 20:10   ` [PATCH v2 6/9] object-file: only register submodule ODB if needed Jonathan Tan
2021-09-28 20:10   ` [PATCH v2 7/9] submodule: pass repo to check_has_commit() Jonathan Tan
2021-09-28 20:10   ` [PATCH v2 8/9] refs: change refs_for_each_ref_in() to take repo Jonathan Tan
2021-09-28 20:10   ` [PATCH v2 9/9] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 0/7] No more " Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 1/7] refs: plumb repo into ref stores Jonathan Tan
2021-09-30 11:13     ` [PATCH] fixup! " Carlo Marcelo Arenas Belón
2021-10-06 17:42     ` Glen Choo
2021-10-08 20:05       ` Jonathan Tan
2021-10-08 20:07       ` Jonathan Tan
2021-10-07 18:33     ` [PATCH v3 1/7] " Josh Steadmon
2021-10-08 20:08       ` Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 2/7] refs: teach arbitrary repo support to iterators Jonathan Tan
2021-10-07 19:31     ` Glen Choo
2021-10-08 20:12       ` Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 3/7] refs: peeling non-the_repository iterators is BUG Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 4/7] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-10-07 18:34     ` Josh Steadmon
2021-10-08 20:19       ` Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 5/7] object-file: only register submodule ODB if needed Jonathan Tan
2021-10-07 18:34     ` Josh Steadmon
2021-10-08 20:22       ` Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 6/7] submodule: pass repo to check_has_commit() Jonathan Tan
2021-09-29 23:06   ` [PATCH v3 7/7] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-10-07 18:34     ` Josh Steadmon
2021-10-08 20:23       ` Jonathan Tan
2021-10-07 18:32   ` [PATCH v3 0/7] No more " Josh Steadmon
2021-10-08 21:08 ` [PATCH v4 " Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 1/7] refs: plumb repo into ref stores Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 2/7] refs: teach arbitrary repo support to iterators Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 3/7] refs: peeling non-the_repository iterators is BUG Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 4/7] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 5/7] object-file: only register submodule ODB if needed Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 6/7] submodule: pass repo to check_has_commit() Jonathan Tan
2021-10-08 21:08   ` [PATCH v4 7/7] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-10-12 22:10   ` [PATCH v4 0/7] No more " Glen Choo
2021-10-12 22:40   ` Josh Steadmon
2021-10-12 22:49     ` 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=dd70820d66307b3943f3c02d6672e99ddd01732f.1632859148.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.