git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Oakley <andrew@adoakley.name>
To: git@vger.kernel.org, Luke Diamand <luke@diamand.org>,
	Jonathan Tan <jonathantanmy@google.com>
Cc: Andrew Oakley <andrew@adoakley.name>
Subject: [PATCH 5/7] merge-recursive: use separate submodule repository
Date: Tue, 29 Sep 2020 16:53:48 +0100	[thread overview]
Message-ID: <20200929155350.49066-6-andrew@adoakley.name> (raw)
In-Reply-To: <20200929155350.49066-1-andrew@adoakley.name>

Using a separate repository with it's own object store should allow any
objects that need to be fetched from a promisor or an alternate object
store to work correctly.

This change is not expected to alter any behaviour.

Signed-off-by: Andrew Oakley <andrew@adoakley.name>
---
 merge-recursive.c | 73 ++++++++++++++++++++++++++++-------------------
 revision.c        | 21 +++++---------
 revision.h        |  1 -
 submodule.c       | 11 +------
 submodule.h       | 11 +++++++
 5 files changed, 62 insertions(+), 55 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index d0214335a7..4984c1e19e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -25,6 +25,7 @@
 #include "revision.h"
 #include "string-list.h"
 #include "submodule.h"
+#include "submodule-config.h"
 #include "tag.h"
 #include "tree-walk.h"
 #include "unpack-trees.h"
@@ -340,7 +341,8 @@ 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 output_commit_title(struct merge_options *opt,
+				struct repository *repo, struct commit *commit)
 {
 	struct merge_remote_desc *desc;
 
@@ -352,15 +354,16 @@ static void output_commit_title(struct merge_options *opt, struct commit *commit
 		strbuf_add_unique_abbrev(&opt->obuf, &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);
@@ -1110,9 +1113,8 @@ static int find_first_merges(struct repository *repo,
 	xsnprintf(merged_revision, sizeof(merged_revision), "^%s",
 		  oid_to_hex(&a->object.oid));
 	repo_init_revisions(repo, &revs, NULL);
-	rev_opts.submodule = path;
 	/* FIXME: can't handle linked worktrees in submodules yet */
-	revs.single_worktree = path != NULL;
+	revs.single_worktree = repo != the_repository;
 	setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);
 
 	/* save all revisions from the above list that contain b */
@@ -1149,12 +1151,12 @@ 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;
-	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);
 }
@@ -1169,9 +1171,11 @@ static int merge_submodule(struct merge_options *opt,
 			   const struct object_id *base, const struct object_id *a,
 			   const struct object_id *b)
 {
+	struct repository *subrepo;
 	struct commit *commit_base, *commit_a, *commit_b;
 	int parent_count;
 	struct object_array merges;
+	int clean = 0;
 
 	int i;
 	int search = !opt->priv->call_depth;
@@ -1187,49 +1191,52 @@ static int merge_submodule(struct merge_options *opt,
 	if (is_null_oid(b))
 		return 0;
 
-	if (add_submodule_odb(path)) {
+	subrepo = open_submodule(path);
+	if (!subrepo) {
 		output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path);
 		return 0;
 	}
 
-	if (!(commit_base = lookup_commit_reference(opt->repo, base)) ||
-	    !(commit_a = lookup_commit_reference(opt->repo, a)) ||
-	    !(commit_b = lookup_commit_reference(opt->repo, b))) {
+	if (!(commit_base = lookup_commit_reference(subrepo, base)) ||
+	    !(commit_a = lookup_commit_reference(subrepo, a)) ||
+	    !(commit_b = lookup_commit_reference(subrepo, b))) {
 		output(opt, 1, _("Failed to merge submodule %s (commits not present)"), path);
-		return 0;
+		goto cleanup;
 	}
 
 	/* check whether both changes are forward */
-	if (!in_merge_bases(commit_base, commit_a) ||
-	    !in_merge_bases(commit_base, commit_b)) {
+	if (!repo_in_merge_bases(subrepo, commit_base, commit_a) ||
+	    !repo_in_merge_bases(subrepo, commit_base, commit_b)) {
 		output(opt, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path);
-		return 0;
+		goto cleanup;
 	}
 
 	/* Case #1: a is contained in b or vice versa */
-	if (in_merge_bases(commit_a, commit_b)) {
+	if (repo_in_merge_bases(subrepo, commit_a, commit_b)) {
 		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);
+			output_commit_title(opt, subrepo, commit_b);
 		} else if (show(opt, 2))
 			output(opt, 2, _("Fast-forwarding submodule %s"), path);
 		else
 			; /* no output */
 
-		return 1;
+		clean = 1;
+		goto cleanup;
 	}
-	if (in_merge_bases(commit_b, commit_a)) {
+	if (repo_in_merge_bases(subrepo, commit_b, commit_a)) {
 		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);
+			output_commit_title(opt, subrepo, commit_a);
 		} else if (show(opt, 2))
 			output(opt, 2, _("Fast-forwarding submodule %s"), path);
 		else
 			; /* no output */
 
-		return 1;
+		clean = 1;
+		goto cleanup;
 	}
 
 	/*
@@ -1241,10 +1248,10 @@ static int merge_submodule(struct merge_options *opt,
 
 	/* Skip the search if makes no sense to the calling context.  */
 	if (!search)
-		return 0;
+		goto cleanup;
 
 	/* find commit which merges them */
-	parent_count = find_first_merges(opt->repo, &merges, path,
+	parent_count = find_first_merges(subrepo, &merges, path,
 					 commit_a, commit_b);
 	switch (parent_count) {
 	case 0:
@@ -1254,7 +1261,8 @@ 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"
@@ -1267,11 +1275,16 @@ 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);
-	return 0;
+
+cleanup:
+	repo_clear(subrepo);
+	free(subrepo);
+	return clean;
 }
 
 static int merge_mode_and_contents(struct merge_options *opt,
@@ -3548,8 +3561,8 @@ static int merge_recursive_internal(struct merge_options *opt,
 
 	if (show(opt, 4)) {
 		output(opt, 4, _("Merging:"));
-		output_commit_title(opt, h1);
-		output_commit_title(opt, h2);
+		output_commit_title(opt, opt->repo, h1);
+		output_commit_title(opt, opt->repo, h2);
 	}
 
 	if (!merge_bases) {
@@ -3563,7 +3576,7 @@ static int merge_recursive_internal(struct merge_options *opt,
 		output(opt, 5, Q_("found %u common ancestor:",
 				"found %u common ancestors:", cnt), cnt);
 		for (iter = merge_bases; iter; iter = iter->next)
-			output_commit_title(opt, iter->item);
+			output_commit_title(opt, opt->repo, iter->item);
 	}
 
 	merged_merge_bases = pop_commit(&merge_bases);
diff --git a/revision.c b/revision.c
index 067030e64c..a3747aaecc 100644
--- a/revision.c
+++ b/revision.c
@@ -2582,16 +2582,15 @@ static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void
 	return for_each_bisect_ref(refs, fn, cb_data, term_good);
 }
 
-static int handle_revision_pseudo_opt(const char *submodule,
-				struct rev_info *revs,
-				int argc, const char **argv, int *flags)
+static int handle_revision_pseudo_opt(struct rev_info *revs,
+				      int argc, const char **argv, int *flags)
 {
 	const char *arg = argv[0];
 	const char *optarg;
 	struct ref_store *refs;
 	int argcount;
 
-	if (submodule) {
+	if (revs->repo != the_repository) {
 		/*
 		 * We need some something like get_submodule_worktrees()
 		 * before we can go through all worktrees of a submodule,
@@ -2600,9 +2599,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
 		 */
 		if (!revs->single_worktree)
 			BUG("--single-worktree cannot be used together with submodule");
-		refs = get_submodule_ref_store(submodule);
-	} else
-		refs = get_main_ref_store(revs->repo);
+	}
+	refs = get_main_ref_store(revs->repo);
 
 	/*
 	 * NOTE!
@@ -2720,12 +2718,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 {
 	int i, flags, left, seen_dashdash, revarg_opt;
 	struct strvec prune_data = STRVEC_INIT;
-	const char *submodule = NULL;
 	int seen_end_of_options = 0;
 
-	if (opt)
-		submodule = opt->submodule;
-
 	/* First, search for "--" */
 	if (opt && opt->assume_dashdash) {
 		seen_dashdash = 1;
@@ -2754,9 +2748,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		if (!seen_end_of_options && *arg == '-') {
 			int opts;
 
-			opts = handle_revision_pseudo_opt(submodule,
-						revs, argc - i, argv + i,
-						&flags);
+			opts = handle_revision_pseudo_opt(revs, argc - i,
+							  argv + i, &flags);
 			if (opts > 0) {
 				i += opts - 1;
 				continue;
diff --git a/revision.h b/revision.h
index f6bf860d19..2b8396fc44 100644
--- a/revision.h
+++ b/revision.h
@@ -330,7 +330,6 @@ extern volatile show_early_output_fn_t show_early_output;
 struct setup_revision_opt {
 	const char *def;
 	void (*tweak)(struct rev_info *, struct setup_revision_opt *);
-	const char *submodule;	/* TODO: drop this and use rev_info->repo */
 	unsigned int	assume_dashdash:1,
 			allow_exclude_promisor_objects:1;
 	unsigned revarg_opt;
diff --git a/submodule.c b/submodule.c
index 3889dc7d9a..1671800340 100644
--- a/submodule.c
+++ b/submodule.c
@@ -505,16 +505,7 @@ static void prepare_submodule_repo_env_in_gitdir(struct strvec *out)
 	strvec_pushf(out, "%s=.", GIT_DIR_ENVIRONMENT);
 }
 
-/*
- * Initialize a repository struct for a submodule based on the provided 'path'.
- *
- * Unlike repo_submodule_init, this tolerates submodules not present
- * in .gitmodules. This function exists only to preserve historical behavior,
- *
- * Returns the repository struct on success,
- * NULL when the submodule is not present.
- */
-static struct repository *open_submodule(const char *path)
+struct repository *open_submodule(const char *path)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct repository *out = xmalloc(sizeof(*out));
diff --git a/submodule.h b/submodule.h
index 4ac6e31cf1..6a66d3e05f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -99,6 +99,17 @@ int bad_to_remove_submodule(const char *path, unsigned flags);
 
 int add_submodule_odb(const char *path);
 
+/*
+ * Initialize a repository struct for a submodule based on the provided 'path'.
+ *
+ * Unlike repo_submodule_init, this tolerates submodules not present
+ * in .gitmodules. This function exists only to preserve historical behavior,
+ *
+ * Returns the repository struct on success,
+ * NULL when the submodule is not present.
+ */
+struct repository *open_submodule(const char *path);
+
 /*
  * Checks if there are submodule changes in a..b. If a is the null OID,
  * checks b and all its ancestors instead.
-- 
2.26.2


  parent reply	other threads:[~2020-09-29 16:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29 15:53 [PATCH 0/7] Submodules and partial clones Andrew Oakley
2020-09-29 15:53 ` [PATCH 1/7] refs: store owning repository for object lookup Andrew Oakley
2020-09-29 15:53 ` [PATCH 2/7] submodule: use separate submodule repositories Andrew Oakley
2020-09-29 15:53 ` [PATCH 3/7] Add failing test for partial clones with submodules Andrew Oakley
2020-09-29 15:53 ` [PATCH 4/7] refs: use correct repo in refs_peel_ref Andrew Oakley
2020-09-29 15:53 ` Andrew Oakley [this message]
2020-09-29 15:53 ` [PATCH 6/7] submodule: remove add_submodule_odb Andrew Oakley
2020-09-29 15:53 ` [PATCH 7/7] submodule: use partial clone filter Andrew Oakley
2020-09-29 18:05 ` [PATCH 0/7] Submodules and partial clones Jonathan Tan
2020-09-30 13:28   ` Andrew Oakley
2020-09-30 20:41     ` Jonathan Tan

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=20200929155350.49066-6-andrew@adoakley.name \
    --to=andrew@adoakley.name \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=luke@diamand.org \
    --subject='Re: [PATCH 5/7] merge-recursive: use separate submodule repository' \
    /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

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).