From: Junio C Hamano <gitster@pobox•com>
To: Pete Harlan <pgit@pcharlan•com>
Cc: git@vger•kernel.org
Subject: Re: [bug?] checkout -m doesn't work without a base version
Date: Mon, 05 Dec 2011 10:58:23 -0800 [thread overview]
Message-ID: <7vbormn8vk.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <4EDBF4D5.6030908@pcharlan.com> (Pete Harlan's message of "Sun, 04 Dec 2011 14:31:49 -0800")
Pete Harlan <pgit@pcharlan•com> writes:
> 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);
next prev parent reply other threads:[~2011-12-05 18:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-04 22:31 [bug?] checkout -m doesn't work without a base version Pete Harlan
2011-12-05 18:58 ` Junio C Hamano [this message]
2011-12-07 7:30 ` Pete Harlan
2011-12-08 18:27 ` Junio C Hamano
2011-12-12 1:48 ` Pete Harlan
2011-12-12 5:29 ` Junio C Hamano
2011-12-20 20:37 ` [PATCH] t/t2023-checkout-m.sh: fix use of test_must_fail Ævar Arnfjörð Bjarmason
2011-12-20 21:23 ` Junio C Hamano
2011-12-14 10:19 ` [bug?] checkout -m doesn't work without a base version Michael Schubert
2011-12-14 17:54 ` Junio C Hamano
2011-12-15 4:20 ` Miles Bader
2011-12-15 10:11 ` Michael Schubert
2011-12-15 10:42 ` Andreas Schwab
2011-12-15 17:36 ` Junio C Hamano
2011-12-16 22:38 ` Ramsay Jones
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=7vbormn8vk.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pgit@pcharlan$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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