git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [bug?] checkout -m doesn't work without a base version

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 5, 2011, 18:58 UTC
Message-ID
<7vbormn8vk.fsf@alter.siamese.dyndns.org>
In-Reply-To
<4EDBF4D5.6030908@pcharlan.com>
Pete Harlan <pgit@pcharlan.com> writes:
Show 7 quoted lines
> But this only works if there's a base version; if foo.c was added in
> each branch, we get:
>
>    error: path 'foo.c' does not have all three versions
>
> Git didn't need all three versions to create the original conflicted
> file, so why would it need them to recreate it?

Because the original "merge" was a bit more carefully written but "checkout -m" was written without worrying too much about "both sides added differently" corner case and still being defensive about not doing random thing upon getting an unexpected input state.

IOW, being lazy ;-)
How does this look?

-- >8 -- checkout -m: no need to insist on having all 3 stages

The content level merge machinery ll_merge() is prepared to merge correctly in "both sides added differently" case by using an empty blob as if it were the common ancestor. "checkout -m" could do the same, but didn't bother supporting it and instead insisted on having all three stages.

Reported-by: Pete Harlan
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/checkout.c |   60 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2a80772..923d040 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -114,16 +114,21 @@ static int check_stage(int stage, struct cache_entry *ce, int pos)
 		return error(_("path '%s' does not have their version"), ce->name);
 }
 
-static int check_all_stages(struct cache_entry *ce, int pos)
+static int check_stages(unsigned stages, struct cache_entry *ce, int pos)
 {
-	if (ce_stage(ce) != 1 ||
-	    active_nr <= pos + 2 ||
-	    strcmp(active_cache[pos+1]->name, ce->name) ||
-	    ce_stage(active_cache[pos+1]) != 2 ||
-	    strcmp(active_cache[pos+2]->name, ce->name) ||
-	    ce_stage(active_cache[pos+2]) != 3)
-		return error(_("path '%s' does not have all three versions"),
-			     ce->name);
+	unsigned seen = 0;
+	const char *name = ce->name;
+
+	while (pos < active_nr) {
+		ce = active_cache[pos];
+		if (strcmp(name, ce->name))
+			break;
+		seen |= (1 << ce_stage(ce));
+		pos++;
+	}
+	if ((stages & seen) != stages)
+		return error(_("path '%s' does not have all necessary versions"),
+			     name);
 	return 0;
 }
 
@@ -150,18 +155,27 @@ static int checkout_merged(int pos, struct checkout *state)
 	int status;
 	unsigned char sha1[20];
 	mmbuffer_t result_buf;
+	unsigned char threeway[3][20];
+	unsigned mode;
+
+	memset(threeway, 0, sizeof(threeway));
+	while (pos < active_nr) {
+		int stage;
+		stage = ce_stage(ce);
+		if (!stage || strcmp(path, ce->name))
+			break;
+		hashcpy(threeway[stage - 1], ce->sha1);
+		if (stage == 2)
+			mode = create_ce_mode(ce->ce_mode);
+		pos++;
+		ce = active_cache[pos];
+	}
+	if (is_null_sha1(threeway[1]) || is_null_sha1(threeway[2]))
+		return error(_("path '%s' does not have necessary versions"), path);
 
-	if (ce_stage(ce) != 1 ||
-	    active_nr <= pos + 2 ||
-	    strcmp(active_cache[pos+1]->name, path) ||
-	    ce_stage(active_cache[pos+1]) != 2 ||
-	    strcmp(active_cache[pos+2]->name, path) ||
-	    ce_stage(active_cache[pos+2]) != 3)
-		return error(_("path '%s' does not have all 3 versions"), path);
-
-	read_mmblob(&ancestor, active_cache[pos]->sha1);
-	read_mmblob(&ours, active_cache[pos+1]->sha1);
-	read_mmblob(&theirs, active_cache[pos+2]->sha1);
+	read_mmblob(&ancestor, threeway[0]);
+	read_mmblob(&ours, threeway[1]);
+	read_mmblob(&theirs, threeway[2]);
 
 	/*
 	 * NEEDSWORK: re-create conflicts from merges with
@@ -192,9 +206,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	if (write_sha1_file(result_buf.ptr, result_buf.size,
 			    blob_type, sha1))
 		die(_("Unable to add merge result for '%s'"), path);
-	ce = make_cache_entry(create_ce_mode(active_cache[pos+1]->ce_mode),
-			      sha1,
-			      path, 2, 0);
+	ce = make_cache_entry(mode, sha1, path, 2, 0);
 	if (!ce)
 		die(_("make_cache_entry failed for path '%s'"), path);
 	status = checkout_entry(ce, state, NULL);
@@ -252,7 +264,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 			} else if (stage) {
 				errs |= check_stage(stage, ce, pos);
 			} else if (opts->merge) {
-				errs |= check_all_stages(ce, pos);
+				errs |= check_stages((1<<2) | (1<<3), ce, pos);
 			} else {
 				errs = 1;
 				error(_("path '%s' is unmerged"), ce->name);
Previous: Pete HarlanNext: Pete Harlan
Message 2 of 15 in “[bug?] checkout -m doesn't work without a base version”
  1. Pete HarlanDec 4, 2011
  2. Junio C HamanoDec 5, 2011
  3. Pete HarlanDec 7, 2011
  4. Junio C HamanoDec 8, 2011
  5. Pete HarlanDec 12, 2011
  6. Junio C HamanoDec 12, 2011
  7. t/t2023-checkout-m.sh: fix use of test_must_failÆvar Arnfjörð Bjarmason, Dec 20, 2011
  8. Junio C HamanoDec 20, 2011
  9. Michael SchubertDec 14, 2011
  10. Junio C HamanoDec 14, 2011
  11. Miles BaderDec 15, 2011
  12. Michael SchubertDec 15, 2011
  13. Andreas SchwabDec 15, 2011
  14. Junio C HamanoDec 15, 2011
  15. Ramsay JonesDec 16, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.