{"thread":{"id":"15277","subject":"[PATCH 1/2] checkout: do not check out unmerged higher stages randomly","startedAt":"2008-08-29T21:39:54Z","lastAt":"2008-08-30T18:02:48Z","messageCount":4,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"89162","messageId":"7vbpzb4i1h.fsf@gitster.siamese.dyndns.org","threadId":"15277","inReplyTo":null,"subject":"[PATCH 1/2] checkout: do not check out unmerged higher stages randomly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T21:39:54Z","receivedAt":"2008-08-29T21:39:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"During a conflicted merge when you have unmerged stages for a path F in\nthe index, if you asked:\n\n    $ git checkout F\n\nwe rewrote F as many times as we have stages for it, and the last one\n(typically \"theirs\") was left in the work tree, without resolving the\nconflict.\n\nThis patch fixes it by noticing that a specified pathspec pattern matches\nan unmerged path, and by erroring out.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-checkout.c |   27 +++++++++++++++++++++++++++\n t/t7201-co.sh      |   23 +++++++++++++++++++++++\n 2 files changed, 50 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex b380ad6..9b33f3a 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -76,6 +76,15 @@ static int read_tree_some(struct tree *tree, const char **pathspec)\n \treturn 0;\n }\n \n+static int skip_same_name(struct cache_entry *ce, int pos)\n+{\n+\twhile (++pos < active_nr &&\n+\t       !strcmp(active_cache[pos]->name, ce->name))\n+\t\t; /* skip */\n+\treturn pos;\n+}\n+\n+\n static int checkout_paths(struct tree *source_tree, const char **pathspec)\n {\n \tint pos;\n@@ -107,6 +116,20 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)\n \tif (report_path_error(ps_matched, pathspec, 0))\n \t\treturn 1;\n \n+\t/* Any unmerged paths? */\n+\tfor (pos = 0; pos < active_nr; pos++) {\n+\t\tstruct cache_entry *ce = active_cache[pos];\n+\t\tif (pathspec_match(pathspec, NULL, ce->name, 0) &&\n+\t\t    ce_stage(ce)) {\n+\t\t\terrs = 1;\n+\t\t\terror(\"path '%s' is unmerged\", ce->name);\n+\t\t\tpos = skip_same_name(ce, pos) - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t}\n+\tif (errs)\n+\t\treturn 1;\n+\n \t/* Now we are committed to check them out */\n \tmemset(&state, 0, sizeof(state));\n \tstate.force = 1;\n@@ -114,6 +137,10 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)\n \tfor (pos = 0; pos < active_nr; pos++) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tif (pathspec_match(pathspec, NULL, ce->name, 0)) {\n+\t\t\tif (ce_stage(ce)) {\n+\t\t\t\tpos = skip_same_name(ce, pos) - 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\terrs |= checkout_entry(ce, &state, NULL);\n \t\t}\n \t}\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 1dff84d..303cf62 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -369,4 +369,27 @@ test_expect_success \\\n     'checkout with --track, but without -b, fails with too short tracked name' '\n     test_must_fail git checkout --track renamer'\n \n+test_expect_success 'checkout an unmerged path should fail' '\n+\trm -f .git/index &&\n+\tO=$(echo original | git hash-object -w --stdin) &&\n+\tA=$(echo ourside | git hash-object -w --stdin) &&\n+\tB=$(echo theirside | git hash-object -w --stdin) &&\n+\t(\n+\t\techo \"100644 $A 0\tfild\" &&\n+\t\techo \"100644 $O 1\tfile\" &&\n+\t\techo \"100644 $A 2\tfile\" &&\n+\t\techo \"100644 $B 3\tfile\" &&\n+\t\techo \"100644 $A 0\tfilf\"\n+\t) | git update-index --index-info &&\n+\techo \"none of the above\" >sample &&\n+\tcat sample >fild &&\n+\tcat sample >file &&\n+\tcat sample >filf &&\n+\ttest_must_fail git checkout fild file filf &&\n+\ttest_cmp sample fild &&\n+\ttest_cmp sample filf &&\n+\ttest_cmp sample file\n+'\n+\n test_done\n+\n-- \n1.6.0.1.90.g27a6e\n"},{"id":"89164","messageId":"7v7i9z4hx2.fsf@gitster.siamese.dyndns.org","threadId":"15277","inReplyTo":"7vbpzb4i1h.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/2] checkout: allow ignoring unmerged paths when checking out of the index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T21:42:33Z","receivedAt":"2008-08-29T21:42:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier we made \"git checkout $pathspec\" to atomically refuse the\noperation, if $pathspec matched any path with unmerged stages.\n\nThis patch allows:\n\n    $ git checkout -f a b c\n\nto ignore, instead of erroring out on, such unmerged paths.  The fix to\nprevent checkout of an unmerged path from random stages is still there.\n\nSigned-off-by: Junio C Hamano <junio@twinsun.com>\n---\n builtin-checkout.c |   41 +++++++++++++++++++++++------------------\n t/t7201-co.sh      |   23 +++++++++++++++++++++++\n 2 files changed, 46 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 9b33f3a..49c43d9 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -20,6 +20,17 @@ static const char * const checkout_usage[] = {\n \tNULL,\n };\n \n+struct checkout_opts {\n+\tint quiet;\n+\tint merge;\n+\tint force;\n+\tint writeout_error;\n+\n+\tconst char *new_branch;\n+\tint new_branch_log;\n+\tenum branch_track track;\n+};\n+\n static int post_checkout_hook(struct commit *old, struct commit *new,\n \t\t\t      int changed)\n {\n@@ -85,7 +96,8 @@ static int skip_same_name(struct cache_entry *ce, int pos)\n }\n \n \n-static int checkout_paths(struct tree *source_tree, const char **pathspec)\n+static int checkout_paths(struct tree *source_tree, const char **pathspec,\n+\t\t\t  struct checkout_opts *opts)\n {\n \tint pos;\n \tstruct checkout state;\n@@ -121,8 +133,12 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tif (pathspec_match(pathspec, NULL, ce->name, 0) &&\n \t\t    ce_stage(ce)) {\n-\t\t\terrs = 1;\n-\t\t\terror(\"path '%s' is unmerged\", ce->name);\n+\t\t\tif (!opts->force) {\n+\t\t\t\terrs = 1;\n+\t\t\t\terror(\"path '%s' is unmerged\", ce->name);\n+\t\t\t} else {\n+\t\t\t\twarning(\"path '%s' is unmerged\", ce->name);\n+\t\t\t}\n \t\t\tpos = skip_same_name(ce, pos) - 1;\n \t\t\tcontinue;\n \t\t}\n@@ -178,17 +194,6 @@ static void describe_detached_head(char *msg, struct commit *commit)\n \tstrbuf_release(&sb);\n }\n \n-struct checkout_opts {\n-\tint quiet;\n-\tint merge;\n-\tint force;\n-\tint writeout_error;\n-\n-\tconst char *new_branch;\n-\tint new_branch_log;\n-\tenum branch_track track;\n-};\n-\n static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree)\n {\n \tstruct unpack_trees_options opts;\n@@ -569,15 +574,15 @@ no_reference:\n \t\t\tdie(\"invalid path specification\");\n \n \t\t/* Checkout paths */\n-\t\tif (opts.new_branch || opts.force || opts.merge) {\n+\t\tif (opts.new_branch || opts.merge) {\n \t\t\tif (argc == 1) {\n-\t\t\t\tdie(\"git checkout: updating paths is incompatible with switching branches/forcing\\nDid you intend to checkout '%s' which can not be resolved as commit?\", argv[0]);\n+\t\t\t\tdie(\"git checkout: updating paths is incompatible with switching branches.\\nDid you intend to checkout '%s' which can not be resolved as commit?\", argv[0]);\n \t\t\t} else {\n-\t\t\t\tdie(\"git checkout: updating paths is incompatible with switching branches/forcing\");\n+\t\t\t\tdie(\"git checkout: updating paths is incompatible with switching branches.\");\n \t\t\t}\n \t\t}\n \n-\t\treturn checkout_paths(source_tree, pathspec);\n+\t\treturn checkout_paths(source_tree, pathspec, &opts);\n \t}\n \n \tif (new.name && !new.commit) {\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 303cf62..bbf86cb 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -391,5 +391,28 @@ test_expect_success 'checkout an unmerged path should fail' '\n \ttest_cmp sample file\n '\n \n+test_expect_success 'checkout with an unmerged path can ignore' '\n+\trm -f .git/index &&\n+\tO=$(echo original | git hash-object -w --stdin) &&\n+\tA=$(echo ourside | git hash-object -w --stdin) &&\n+\tB=$(echo theirside | git hash-object -w --stdin) &&\n+\t(\n+\t\techo \"100644 $A 0\tfild\" &&\n+\t\techo \"100644 $O 1\tfile\" &&\n+\t\techo \"100644 $A 2\tfile\" &&\n+\t\techo \"100644 $B 3\tfile\" &&\n+\t\techo \"100644 $A 0\tfilf\"\n+\t) | git update-index --index-info &&\n+\techo \"none of the above\" >sample &&\n+\techo ourside >expect &&\n+\tcat sample >fild &&\n+\tcat sample >file &&\n+\tcat sample >filf &&\n+\tgit checkout -f fild file filf &&\n+\ttest_cmp expect fild &&\n+\ttest_cmp expect filf &&\n+\ttest_cmp sample file\n+'\n+\n test_done\n \n-- \n1.6.0.1.90.g27a6e\n"},{"id":"89165","messageId":"7vzlmv32uv.fsf@gitster.siamese.dyndns.org","threadId":"15277","inReplyTo":"7v7i9z4hx2.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] checkout: allow ignoring unmerged paths when checking out of the index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T21:53:12Z","receivedAt":"2008-08-29T21:53:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is an obvious sequel to this story.  We can enhance the behaviour of\ncheckout out of the index during a conflicted merge by doing the\nfollowing, which is the reason why I stumbled upon this bug that was fixed\nby these two patches:\n\n * Teach \"git checkout --ours\", \"git checkout ---theirs\" so that stages 2\n   and stages 3 for unmerged paths are checked out (and possibly resolve\n   the conflict).\n\n * Teach \"git checkout --merge\" to redo the file level merge between\n   stages 1/2/3, and update the work tree with the result.\n"},{"id":"89230","messageId":"7v1w06ido7.fsf@gitster.siamese.dyndns.org","threadId":"15277","inReplyTo":"7vbpzb4i1h.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] checkout: do not check out unmerged higher stages randomly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-30T18:02:48Z","receivedAt":"2008-08-30T18:02:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> During a conflicted merge when you have unmerged stages for a path F in\n> the index, if you asked:\n>\n>     $ git checkout F\n>\n> we rewrote F as many times as we have stages for it, and the last one\n> (typically \"theirs\") was left in the work tree, without resolving the\n> conflict.\n>\n> This patch fixes it by noticing that a specified pathspec pattern matches\n> an unmerged path, and by erroring out.\n\nThe patch claims that it is a bugfix, but I have a slight worry that\nsomebody might be depending on this broken behaviour in their workflow.\n\n    $ git pull\n    Conflicts in foo.c (modify/modify)\n    .. Eeek, conflicts.  My changes are always crap, and I trust my\n    .. upstream more than I trust myself, so I'll take advantage of\n    .. the bug to check out \"their\" version.\n    $ git checkout foo.c\n\nIn modify/modify case, because we will have stage #3 (state after their\nchange), that will be the version you will get as the final result of this\nbroken behaviour of \"checkout\".\n\nThis patch alone will make the above to check out \"theirs\" an error.  In a\nlater one in the series, you can still say:\n\n    $ git checkout --theirs foo.c ;# or \"git checkout -3 foo.c\"\n\nto get the feature back, at the same time you will now be able to say\n\n    $ git checkout --ours foo.c ;# or \"git checkout -2 foo.c\"\n\nfor symmetry (admittably, it is the same thing as \"git checkout HEAD\nfoo.c\").\n\nSo we _could_ choose to stay backward compatible by leaving this \"bug\"\nunattended without loss of ability to checkout either theirs or ours, even\nthough without fixing this, you cannot sanely add \"git checkout -m foo.c\"\nfeature that comes later in the updated series, which allows you to go\nback to the original conflicted state after you (or rerere) messed up the\nworking tree state.\n\nThoughts?\n"}]}