When multiple merge-bases are found for two commits to be merged the
merge machinery will ask twice for a merge resolution. Currently its not
possible to use the revision-walking api for walking the same commits
multiple times. Thats why we now run the revision walking in a seperate
git process.
Signed-off-by: Heiko Voigt <hvoigt@xxxxxxxxxx>
---
Hi,
On Wed, Aug 24, 2011 at 03:43:56PM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@xxxxxxxxxx> writes:
>
> >> I have been suspecting that most of this should be done in a separate
> >> helper program that is run via run_command() interface, without
> >> contaminating the object pool the main merge process has with data from
> >> the submodule object store to begin with (i.e. add_submodule_odb() and
> >> everything below should go). Wouldn't it be a lot cleaner solution?
> >
> > Hmm, I would like to keep it in process. Since there are platforms where
> > spawning new processes is very slow.
>
> Adding submodule's odb into the main process _will_ also have performance
> penalties because it will make it more expensive to look up objects that
> belong to the superproject when the superproject wants its own look up.
>
> In case you haven't realized yet, walking revision graph multiple times
> while making sure that you do not affect other revision traversals in
> effect is hard to arrange right. But more importantly, correctness counts
> more than performing quickly and giving a bogus result with premature
> optimization that makes it harder to implement things correctly (and
> harder to verify the change is correct).
Yes of course correctness has priority one. And so here is a patch
implementing the rev-iteration using a seperate process. Once the
revision walking api is able to walk multiple times we get back to it
again.
I noticed that the suggestion for the merge resolution presented is
wrong which Brad also seems to have found as presented in his patch in
message:
<438ea0b254ccafb3fc9f3431f8f86007cc03132b.1314290439.git.brad.king@xxxxxxxxxxx>
But this breakage depends on the wrong output from rev-list with the
--ancestry-path option. This patch should be correct on its own.
submodule.c | 31 ++++++++++++++++++++++---------
t/t7405-submodule-merge.sh | 2 +-
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/submodule.c b/submodule.c
index 1ba9646..9820df7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -581,29 +581,42 @@ static int find_first_merges(struct object_array *result, const char *path,
char merged_revision[42];
const char *rev_args[] = { "rev-list", "--merges", "--ancestry-path",
"--all", merged_revision, NULL };
- struct rev_info revs;
- struct setup_revision_opt rev_opts;
+ struct child_process cp;
+ struct strbuf one_rev = STRBUF_INIT;
memset(&merges, 0, sizeof(merges));
memset(result, 0, sizeof(struct object_array));
- memset(&rev_opts, 0, sizeof(rev_opts));
+ memset(&cp, 0, sizeof(cp));
/* get all revisions that merge commit a */
snprintf(merged_revision, sizeof(merged_revision), "^%s",
sha1_to_hex(a->object.sha1));
- init_revisions(&revs, NULL);
- rev_opts.submodule = path;
- setup_revisions(sizeof(rev_args)/sizeof(char *)-1, rev_args, &revs, &rev_opts);
+
+ cp.argv = rev_args;
+ cp.env = local_repo_env;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.out = -1;
+ cp.dir = path;
+ if (start_command(&cp))
+ die("Could not run 'git rev-list --merges --ancestry-path --all %s' "
+ "command in submodule %s", merged_revision, path);
+ FILE *out = fdopen(cp.out, "r");
+ if (!out)
+ die("Could not open pipe of rev-list command.");
/* save all revisions from the above list that contain b */
- if (prepare_revision_walk(&revs))
- die("revision walk setup failed");
- while ((commit = get_revision(&revs)) != NULL) {
+ while (strbuf_getline(&one_rev, out, '\n') != EOF) {
+ commit = lookup_commit_reference_by_name(one_rev.buf);
struct object *o = &(commit->object);
if (in_merge_bases(b, &commit, 1))
add_object_array(o, NULL, &merges);
}
+ fclose(out);
+ finish_command(&cp);
+ strbuf_release(&one_rev);
+
/* Now we've got all merges that contain a and b. Prune all
* merges that contain another found merge and save them in
* result.
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 8f6f2d6..603fb72 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh
@@ -269,7 +269,7 @@ test_expect_success 'setup for recursive merge with submodule' '
'
# merge should leave submodule unmerged in index
-test_expect_failure 'recursive merge with submodule' '
+test_expect_success 'recursive merge with submodule' '
(cd merge-recursive &&
test_must_fail git merge top-bc &&
echo "160000 $(git rev-parse top-cb:sub) 2 sub" > expect2 &&
--
1.7.6.551.g0f51b.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html