{"thread":{"id":"29083","subject":"[bug?] checkout -m doesn't work without a base version","startedAt":"2011-12-04T22:31:49Z","lastAt":"2011-12-20T21:23:56Z","messageCount":15,"participants":["Pete Harlan","Junio C Hamano","Michael Schubert","Miles Bader","Andreas Schwab","Ramsay Jones","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"180299","messageId":"4EDBF4D5.6030908@pcharlan.com","threadId":"29083","inReplyTo":null,"subject":"[bug?] checkout -m doesn't work without a base version","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2011-12-04T22:31:49Z","receivedAt":"2011-12-04T22:31:49Z","isPatch":false,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"Hi,\n\nIf during a merge I've resolved conflicts in foo.c but want to start\nover with foo.c to resolve them differently, I can say \"git checkout\n-m foo.c\" to restore it to its un-resolved state.\n\nBut this only works if there's a base version; if foo.c was added in\neach branch, we get:\n\n error: path 'foo.c' does not have all three versions\n\nGit didn't need all three versions to create the original conflicted\nfile, so why would it need them to recreate it?\n\n(The message is the same if I explicitly tell Git I don't want diff3\nvia \"git checkout --conflict=merge foo.c\".)\n\nIf this is considered a bug worth fixing I'll write a test that it\nfails; if it's expected behavior I think the docs should mention\nthat.\n\nThanks,\n\nPete Harlan\npgit@pcharlan.com\n"},{"id":"180309","messageId":"7vbormn8vk.fsf@alter.siamese.dyndns.org","threadId":"29083","inReplyTo":"4EDBF4D5.6030908@pcharlan.com","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-05T18:58:23Z","receivedAt":"2011-12-05T18:58:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Harlan writes:\n\n> But this only works if there's a base version; if foo.c was added in\n> each branch, we get:\n>\n> error: path 'foo.c' does not have all three versions\n>\n> Git didn't need all three versions to create the original conflicted\n> file, so why would it need them to recreate it?\n\nBecause the original \"merge\" was a bit more carefully written but\n\"checkout -m\" was written without worrying too much about \"both sides\nadded differently\" corner case and still being defensive about not doing\nrandom thing upon getting an unexpected input state.\n\nIOW, being lazy ;-)\n\nHow does this look?\n\n-- >8 --\ncheckout -m: no need to insist on having all 3 stages\n\nThe content level merge machinery ll_merge() is prepared to merge\ncorrectly in \"both sides added differently\" case by using an empty blob as\nif it were the common ancestor. \"checkout -m\" could do the same, but didn't\nbother supporting it and instead insisted on having all three stages.\n\nReported-by: Pete Harlan\nSigned-off-by: Junio C Hamano \n---\n builtin/checkout.c | 60 +++++++++++++++++++++++++++++++--------------------\n 1 files changed, 36 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2a80772..923d040 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -114,16 +114,21 @@ static int check_stage(int stage, struct cache_entry *ce, int pos)\n \t\treturn error(_(\"path '%s' does not have their version\"), ce->name);\n }\n \n-static int check_all_stages(struct cache_entry *ce, int pos)\n+static int check_stages(unsigned stages, struct cache_entry *ce, int pos)\n {\n-\tif (ce_stage(ce) != 1 ||\n-\t active_nr <= pos + 2 ||\n-\t strcmp(active_cache[pos+1]->name, ce->name) ||\n-\t ce_stage(active_cache[pos+1]) != 2 ||\n-\t strcmp(active_cache[pos+2]->name, ce->name) ||\n-\t ce_stage(active_cache[pos+2]) != 3)\n-\t\treturn error(_(\"path '%s' does not have all three versions\"),\n-\t\t\t ce->name);\n+\tunsigned seen = 0;\n+\tconst char *name = ce->name;\n+\n+\twhile (pos < active_nr) {\n+\t\tce = active_cache[pos];\n+\t\tif (strcmp(name, ce->name))\n+\t\t\tbreak;\n+\t\tseen |= (1 << ce_stage(ce));\n+\t\tpos++;\n+\t}\n+\tif ((stages & seen) != stages)\n+\t\treturn error(_(\"path '%s' does not have all necessary versions\"),\n+\t\t\t name);\n \treturn 0;\n }\n \n@@ -150,18 +155,27 @@ static int checkout_merged(int pos, struct checkout *state)\n \tint status;\n \tunsigned char sha1[20];\n \tmmbuffer_t result_buf;\n+\tunsigned char threeway[3][20];\n+\tunsigned mode;\n+\n+\tmemset(threeway, 0, sizeof(threeway));\n+\twhile (pos < active_nr) {\n+\t\tint stage;\n+\t\tstage = ce_stage(ce);\n+\t\tif (!stage || strcmp(path, ce->name))\n+\t\t\tbreak;\n+\t\thashcpy(threeway[stage - 1], ce->sha1);\n+\t\tif (stage == 2)\n+\t\t\tmode = create_ce_mode(ce->ce_mode);\n+\t\tpos++;\n+\t\tce = active_cache[pos];\n+\t}\n+\tif (is_null_sha1(threeway[1]) || is_null_sha1(threeway[2]))\n+\t\treturn error(_(\"path '%s' does not have necessary versions\"), path);\n \n-\tif (ce_stage(ce) != 1 ||\n-\t active_nr <= pos + 2 ||\n-\t strcmp(active_cache[pos+1]->name, path) ||\n-\t ce_stage(active_cache[pos+1]) != 2 ||\n-\t strcmp(active_cache[pos+2]->name, path) ||\n-\t ce_stage(active_cache[pos+2]) != 3)\n-\t\treturn error(_(\"path '%s' does not have all 3 versions\"), path);\n-\n-\tread_mmblob(&ancestor, active_cache[pos]->sha1);\n-\tread_mmblob(&ours, active_cache[pos+1]->sha1);\n-\tread_mmblob(&theirs, active_cache[pos+2]->sha1);\n+\tread_mmblob(&ancestor, threeway[0]);\n+\tread_mmblob(&ours, threeway[1]);\n+\tread_mmblob(&theirs, threeway[2]);\n \n \t/*\n \t * NEEDSWORK: re-create conflicts from merges with\n@@ -192,9 +206,7 @@ static int checkout_merged(int pos, struct checkout *state)\n \tif (write_sha1_file(result_buf.ptr, result_buf.size,\n \t\t\t blob_type, sha1))\n \t\tdie(_(\"Unable to add merge result for '%s'\"), path);\n-\tce = make_cache_entry(create_ce_mode(active_cache[pos+1]->ce_mode),\n-\t\t\t sha1,\n-\t\t\t path, 2, 0);\n+\tce = make_cache_entry(mode, sha1, path, 2, 0);\n \tif (!ce)\n \t\tdie(_(\"make_cache_entry failed for path '%s'\"), path);\n \tstatus = checkout_entry(ce, state, NULL);\n@@ -252,7 +264,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \t\t\t} else if (stage) {\n \t\t\t\terrs |= check_stage(stage, ce, pos);\n \t\t\t} else if (opts->merge) {\n-\t\t\t\terrs |= check_all_stages(ce, pos);\n+\t\t\t\terrs |= check_stages((1<<2) | (1<<3), ce, pos);\n \t\t\t} else {\n \t\t\t\terrs = 1;\n \t\t\t\terror(_(\"path '%s' is unmerged\"), ce->name);\n"},{"id":"180459","messageId":"4EDF1631.5090906@pcharlan.com","threadId":"29083","inReplyTo":"7vbormn8vk.fsf@alter.siamese.dyndns.org","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2011-12-07T07:30:57Z","receivedAt":"2011-12-07T07:30:57Z","isPatch":false,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 12/05/2011 10:58 AM, Junio C Hamano wrote:\n> Pete Harlan writes:\n> \n>> But this only works if there's a base version; if foo.c was added in\n>> each branch, we get:\n>>\n>> error: path 'foo.c' does not have all three versions\n>>\n>> Git didn't need all three versions to create the original conflicted\n>> file, so why would it need them to recreate it?\n> \n> Because the original \"merge\" was a bit more carefully written but\n> \"checkout -m\" was written without worrying too much about \"both sides\n> added differently\" corner case and still being defensive about not doing\n> random thing upon getting an unexpected input state.\n> \n> IOW, being lazy ;-)\n> \n> How does this look?\n\nWow, thanks for the quick response! That does indeed let me checkout\nthe file as expected.\n\nI wrote a test (below) to be folded in with your patch, but the test\nfails because it expects the restored file to be the same as the\noriginally-conflicted file, but the conflict-line labels change from\n\"HEAD\" and \"master\":\n\n <<<<<<< HEAD\n in_topic\n =======\n in_master\n >>>>>>> master\n\nto \"ours\" and \"theirs\". (The same thing happens in the 3-way merge\ncase.)\n\nIf the label change is expected then I can rewrite the test to ignore\nlabels, or to expect \"ours\" and \"theirs\", whichever you think is best.\nIf the label change is unexpected, then I guess the test is good :)\n\n--Pete\n\n-- >8 --\nTest 'checkout -m -- path' when path is a 2-way merge\n\nSigned-off-by: Pete Harlan \n---\n t/t2023-checkout-m-twoway.sh | 29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n create mode 100755 t/t2023-checkout-m-twoway.sh\n\ndiff --git a/t/t2023-checkout-m-twoway.sh b/t/t2023-checkout-m-twoway.sh\nnew file mode 100755\nindex 0000000..5b50360\n--- /dev/null\n+++ b/t/t2023-checkout-m-twoway.sh\n@@ -0,0 +1,29 @@\n+#!/bin/sh\n+\n+test_description='checkout -m -- conflicted/path/with/2-way/merge\n+\n+Ensures that checkout -m on a resolved file restores the conflicted file\n+when it conflicted with a 2-way merge.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\ttest_tick &&\n+\ttest_commit initial_commit &&\n+\tgit branch topic &&\n+\ttest_commit added_in_master file.txt in_master &&\n+\tgit checkout topic &&\n+\ttest_commit added_in_topic file.txt in_topic\n+'\n+\n+test_must_fail git merge master\n+\n+test_expect_success '-m restores 2-way conflicted+resolved file' '\n+\tcp file.txt file.txt.conflicted &&\n+\techo resolved >file.txt &&\n+\tgit add file.txt &&\n+\tgit checkout -m -- file.txt &&\n+\ttest_cmp file.txt.conflicted file.txt\n+'\n+\n+test_done\n"},{"id":"180606","messageId":"7vvcpqj4vr.fsf@alter.siamese.dyndns.org","threadId":"29083","inReplyTo":"4EDF1631.5090906@pcharlan.com","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-08T18:27:20Z","receivedAt":"2011-12-08T18:27:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Harlan writes:\n\n> to \"ours\" and \"theirs\". (The same thing happens in the 3-way merge\n> case.)\n\nThat is entirely expected. \"checkout -m\" does not know or care _how_ the\nindex came to the conflicted state and reproduces the three-way conflicted\nstate in the file in the working tree solely from the contents of the\nindex which records only what the common thing looked like (stage #1),\nwhat one side looked like (stage #2) and what the other side looked like\n(stage #3) before the mergy operation began, so there is no way for it to\nsay \"the other side came from foo/blah branch\". There are even cases where\nthe conflict originally came not from any branch (think \"am -3\").\n"},{"id":"180839","messageId":"4EE55D5E.1090908@pcharlan.com","threadId":"29083","inReplyTo":"7vvcpqj4vr.fsf@alter.siamese.dyndns.org","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2011-12-12T01:48:14Z","receivedAt":"2011-12-12T01:48:14Z","isPatch":false,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 12/08/2011 10:27 AM, Junio C Hamano wrote:\n> Pete Harlan writes:\n> \n>> to \"ours\" and \"theirs\". (The same thing happens in the 3-way merge\n>> case.)\n> \n> That is entirely expected. \"checkout -m\" does not know or care _how_ the\n> index came to the conflicted state and reproduces the three-way conflicted\n> state in the file in the working tree solely from the contents of the\n> index which records only what the common thing looked like (stage #1),\n> what one side looked like (stage #2) and what the other side looked like\n> (stage #3) before the mergy operation began, so there is no way for it to\n> say \"the other side came from foo/blah branch\". There are even cases where\n> the conflict originally came not from any branch (think \"am -3\").\n\nThanks for taking the time to explain this.\n\nHere's a test that strips branch info off the conflict lines before\nverifying checkout -m, and it tests both the 2-way and 3-way cases.\nThe 3-way works before and after your patch; the 2-way fails before\nyour patch but passes after. If the you think the test is worth\nincluding feel free to fold it in to your patch.\n\n--Pete\n\n-------------------\n\n>From a4522e06515231034c1ada65e1e91614a6c4809e Mon Sep 17 00:00:00 2001\nFrom: Pete Harlan \nDate: Tue, 6 Dec 2011 23:01:28 -0800\nSubject: [PATCH] Test 'checkout -m -- path'\n\nSigned-off-by: Pete Harlan \n---\n t/t2023-checkout-m.sh | 47\n+++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 47 insertions(+), 0 deletions(-)\n create mode 100755 t/t2023-checkout-m.sh\n\ndiff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh\nnew file mode 100755\nindex 0000000..1a40ce0\n--- /dev/null\n+++ b/t/t2023-checkout-m.sh\n@@ -0,0 +1,47 @@\n+#!/bin/sh\n+\n+test_description='checkout -m -- \n+\n+Ensures that checkout -m on a resolved file restores the conflicted file'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\ttest_tick &&\n+\ttest_commit both.txt both.txt initial &&\n+\tgit branch topic &&\n+\ttest_commit modified_in_master both.txt in_master &&\n+\ttest_commit added_in_master each.txt in_master &&\n+\tgit checkout topic &&\n+\ttest_commit modified_in_topic both.txt in_topic &&\n+\ttest_commit added_in_topic each.txt in_topic\n+'\n+\n+test_must_fail git merge master\n+\n+clean_branchnames () {\n+\t# Remove branch names after conflict lines\n+\tsed 's/^\\([<>]\\{5,\\}\\) .*$/\\1/'\n+}\n+\n+test_expect_success '-m restores 2-way conflicted+resolved file' '\n+\tcp each.txt each.txt.conflicted &&\n+\techo resolved >each.txt &&\n+\tgit add each.txt &&\n+\tgit checkout -m -- each.txt &&\n+\tclean_branchnames each.txt.cleaned &&\n+\tclean_branchnames each.txt.conflicted.cleaned &&\n+\ttest_cmp each.txt.conflicted.cleaned each.txt.cleaned\n+'\n+\n+test_expect_success '-m restores 3-way conflicted+resolved file' '\n+\tcp both.txt both.txt.conflicted &&\n+\techo resolved >both.txt &&\n+\tgit add both.txt &&\n+\tgit checkout -m -- both.txt &&\n+\tclean_branchnames both.txt.cleaned &&\n+\tclean_branchnames both.txt.conflicted.cleaned &&\n+\ttest_cmp both.txt.conflicted.cleaned both.txt.cleaned\n+'\n+\n+test_done\n-- \n1.7.8\n"},{"id":"180910","messageId":"7v4nx66vl2.fsf@alter.siamese.dyndns.org","threadId":"29083","inReplyTo":"4EE55D5E.1090908@pcharlan.com","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T05:29:09Z","receivedAt":"2011-12-12T05:29:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"},{"id":"181125","messageId":"4EE8782A.9040507@elegosoft.com","threadId":"29083","inReplyTo":"7vbormn8vk.fsf@alter.siamese.dyndns.org","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Michael Schubert","fromEmail":"mschub@elegosoft.com","sentAt":"2011-12-14T10:19:22Z","receivedAt":"2011-12-14T10:19:22Z","isPatch":false,"sender":{"key":"mschub@elegosoft.com","avatar":null},"body":"On 12/05/2011 07:58 PM, Junio C Hamano wrote:\n> @@ -150,18 +155,27 @@ static int checkout_merged(int pos, struct checkout *state)\n> \tint status;\n> \tunsigned char sha1[20];\n> \tmmbuffer_t result_buf;\n> +\tunsigned char threeway[3][20];\n> +\tunsigned mode;\n> +\n> +\tmemset(threeway, 0, sizeof(threeway));\n> +\twhile (pos < active_nr) {\n> +\t\tint stage;\n> +\t\tstage = ce_stage(ce);\n> +\t\tif (!stage || strcmp(path, ce->name))\n> +\t\t\tbreak;\n> +\t\thashcpy(threeway[stage - 1], ce->sha1);\n> +\t\tif (stage == 2)\n> +\t\t\tmode = create_ce_mode(ce->ce_mode);\n> +\t\tpos++;\n> +\t\tce = active_cache[pos];\n> +\t}\n> +\tif (is_null_sha1(threeway[1]) || is_null_sha1(threeway[2]))\n> +\t\treturn error(_(\"path '%s' does not have necessary versions\"), path);\n> \n> -\tif (ce_stage(ce) != 1 ||\n> -\t active_nr <= pos + 2 ||\n> -\t strcmp(active_cache[pos+1]->name, path) ||\n> -\t ce_stage(active_cache[pos+1]) != 2 ||\n> -\t strcmp(active_cache[pos+2]->name, path) ||\n> -\t ce_stage(active_cache[pos+2]) != 3)\n> -\t\treturn error(_(\"path '%s' does not have all 3 versions\"), path);\n> -\n> -\tread_mmblob(&ancestor, active_cache[pos]->sha1);\n> -\tread_mmblob(&ours, active_cache[pos+1]->sha1);\n> -\tread_mmblob(&theirs, active_cache[pos+2]->sha1);\n> +\tread_mmblob(&ancestor, threeway[0]);\n> +\tread_mmblob(&ours, threeway[1]);\n> +\tread_mmblob(&theirs, threeway[2]);\n> \n> \t/*\n> \t * NEEDSWORK: re-create conflicts from merges with\n> @@ -192,9 +206,7 @@ static int checkout_merged(int pos, struct checkout *state)\n> \tif (write_sha1_file(result_buf.ptr, result_buf.size,\n> \t\t\t blob_type, sha1))\n> \t\tdie(_(\"Unable to add merge result for '%s'\"), path);\n> -\tce = make_cache_entry(create_ce_mode(active_cache[pos+1]->ce_mode),\n> -\t\t\t sha1,\n> -\t\t\t path, 2, 0);\n> +\tce = make_cache_entry(mode, sha1, path, 2, 0);\n> \tif (!ce)\n> \t\tdie(_(\"make_cache_entry failed for path '%s'\"), path);\n> \tstatus = checkout_entry(ce, state, NULL);\n\ngcc 4.6.2:\n\nbuiltin/checkout.c: In function ‘cmd_checkout’:\nbuiltin/checkout.c:210:5: warning: ‘mode’ may be used uninitialized in this function [-Wuninitialized]\nbuiltin/checkout.c:160:11: note: ‘mode’ was declared here\n"},{"id":"181175","messageId":"7vhb13qbs6.fsf@alter.siamese.dyndns.org","threadId":"29083","inReplyTo":"4EE8782A.9040507@elegosoft.com","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-14T17:54:33Z","receivedAt":"2011-12-14T17:54:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Schubert writes:\n\n>> ...\n>> +\tmemset(threeway, 0, sizeof(threeway));\n>> +\twhile (pos < active_nr) {\n>> +\t\tint stage;\n>> +\t\tstage = ce_stage(ce);\n>> +\t\tif (!stage || strcmp(path, ce->name))\n>> +\t\t\tbreak;\n>> +\t\thashcpy(threeway[stage - 1], ce->sha1);\n>> +\t\tif (stage == 2)\n>> +\t\t\tmode = create_ce_mode(ce->ce_mode);\n>> +\t\tpos++;\n>> +\t\tce = active_cache[pos];\n>> +\t}\n>> +\tif (is_null_sha1(threeway[1]) || is_null_sha1(threeway[2]))\n>> +\t\treturn error(_(\"path '%s' does not have necessary versions\"), path);\n>> ...\n>> +\tce = make_cache_entry(mode, sha1, path, 2, 0);\n>> \tif (!ce)\n>> \t\tdie(_(\"make_cache_entry failed for path '%s'\"), path);\n>> \tstatus = checkout_entry(ce, state, NULL);\n>\n> gcc 4.6.2:\n>\n> builtin/checkout.c: In function ‘cmd_checkout’:\n> builtin/checkout.c:210:5: warning: ‘mode’ may be used uninitialized in this function [-Wuninitialized]\n> builtin/checkout.c:160:11: note: ‘mode’ was declared here\n\nIsn't this just your gcc being overly cautious (aka \"silly\")?\n\nThe variable \"mode\" is assigned to when we see an stage #2 entry in the\nloop, and we should have updated threeway[1] immediately before doing so.\nIf threeway[1] is not updated, we would have already returned before using\nthe variable in make_cache_entry().\n"},{"id":"181207","messageId":"buoipli8nzy.fsf@dhlpc061.dev.necel.com","threadId":"29083","inReplyTo":"7vhb13qbs6.fsf@alter.siamese.dyndns.org","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2011-12-15T04:20:17Z","receivedAt":"2011-12-15T04:20:17Z","isPatch":false,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Junio C Hamano writes:\n>> builtin/checkout.c: In function ‘cmd_checkout’:\n>> builtin/checkout.c:210:5: warning: ‘mode’ may be used uninitialized\n>> in this function [-Wuninitialized]\n>> builtin/checkout.c:160:11: note: ‘mode’ was declared here\n>\n> Isn't this just your gcc being overly cautious (aka \"silly\")?\n>\n> The variable \"mode\" is assigned to when we see an stage #2 entry in the\n> loop, and we should have updated threeway[1] immediately before doing so.\n> If threeway[1] is not updated, we would have already returned before using\n> the variable in make_cache_entry().\n\nMaybe that is actually guaranteed (I dunno), but it's certainly not\nobvious from the code here, even to a human... any guarantee would\nhave to come from external invariants that the compiler doesn't know\nabout.\n\nGiven that, I think it's a fair warning, certainly not \"silly.\" This\naspect of the code doesn't seem easy to understand...\n\n-Miles\n\n-- \n`To alcohol! The cause of, and solution to,\n all of life's problems' --Homer J. Simpson\n"},{"id":"181220","messageId":"4EE9C7D9.8020407@elegosoft.com","threadId":"29083","inReplyTo":"buoipli8nzy.fsf@dhlpc061.dev.necel.com","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Michael Schubert","fromEmail":"mschub@elegosoft.com","sentAt":"2011-12-15T10:11:37Z","receivedAt":"2011-12-15T10:11:37Z","isPatch":false,"sender":{"key":"mschub@elegosoft.com","avatar":null},"body":"On 12/15/2011 05:20 AM, Miles Bader wrote:\n> Junio C Hamano writes:\n>>> builtin/checkout.c: In function ‘cmd_checkout’:\n>>> builtin/checkout.c:210:5: warning: ‘mode’ may be used uninitialized\n>>> in this function [-Wuninitialized]\n>>> builtin/checkout.c:160:11: note: ‘mode’ was declared here\n>>\n>> Isn't this just your gcc being overly cautious (aka \"silly\")?\n>>\n>> The variable \"mode\" is assigned to when we see an stage #2 entry in the\n>> loop, and we should have updated threeway[1] immediately before doing so.\n>> If threeway[1] is not updated, we would have already returned before using\n>> the variable in make_cache_entry().\n> \n> Maybe that is actually guaranteed (I dunno), but it's certainly not\n> obvious from the code here, even to a human... any guarantee would\n> have to come from external invariants that the compiler doesn't know\n> about.\n> \n> Given that, I think it's a fair warning, certainly not \"silly.\" This\n> aspect of the code doesn't seem easy to understand...\n\nSilly or not: That's what gcc 4.6.x is warning about with -Wuninitialized\n(-Wall) set - I didn't set any additional options, just plain `make all`.\n"},{"id":"181221","messageId":"m2vcpiw1z1.fsf@igel.home","threadId":"29083","inReplyTo":"7vhb13qbs6.fsf@alter.siamese.dyndns.org","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2011-12-15T10:42:10Z","receivedAt":"2011-12-15T10:42:10Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Junio C Hamano writes:\n\n> The variable \"mode\" is assigned to when we see an stage #2 entry in the\n> loop, and we should have updated threeway[1] immediately before doing so.\n> If threeway[1] is not updated, we would have already returned before using\n> the variable in make_cache_entry().\n\nHow can you be sure that ce_stage(ce) ever returns 2?\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"181231","messageId":"7v4nx1pwjg.fsf@alter.siamese.dyndns.org","threadId":"29083","inReplyTo":"m2vcpiw1z1.fsf@igel.home","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-15T17:36:03Z","receivedAt":"2011-12-15T17:36:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab writes:\n\n> Junio C Hamano writes:\n>\n>> The variable \"mode\" is assigned to when we see an stage #2 entry in the\n>> loop, and we should have updated threeway[1] immediately before doing so.\n>> If threeway[1] is not updated, we would have already returned before using\n>> the variable in make_cache_entry().\n>\n> How can you be sure that ce_stage(ce) ever returns 2?\n\nYou cannot and and there are cases where you exit the loop without finding\na stage #2 entry. But in that case threeway[1] stays 0{40} and control is\nreturned to the caller without ever getting to the place where the\nvariable is used.\n\nYou could do the usual \"unnecessary initialization\" trick, though.\n\n builtin/checkout.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 31aa248..064e7a1 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -152,7 +152,7 @@ static int checkout_merged(int pos, struct checkout *state)\n \tunsigned char sha1[20];\n \tmmbuffer_t result_buf;\n \tunsigned char threeway[3][20];\n-\tunsigned mode;\n+\tunsigned mode = 0;\n \n \tmemset(threeway, 0, sizeof(threeway));\n \twhile (pos < active_nr) {\n"},{"id":"181347","messageId":"4EEBC85D.2040901@ramsay1.demon.co.uk","threadId":"29083","inReplyTo":"7v4nx1pwjg.fsf@alter.siamese.dyndns.org","subject":"Re: [bug?] checkout -m doesn't work without a base version","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-12-16T22:38:21Z","receivedAt":"2011-12-16T22:38:21Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> You could do the usual \"unnecessary initialization\" trick, though.\n\nYes, I wrote exactly this patch some days ago (when it first appeared\nin pu), but haven't found the time to send it ... :-D\n\nATB,\nRamsay Jones\n"},{"id":"181537","messageId":"1324413465-25614-1-git-send-email-avarab@gmail.com","threadId":"29083","inReplyTo":"4EDF1631.5090906@pcharlan.com","subject":"[PATCH] t/t2023-checkout-m.sh: fix use of test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-12-20T20:37:45Z","receivedAt":"2011-12-20T20:37:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change an invocation of test_must_fail() to be inside a\ntest_expect_success() as is our usual pattern. Having it outside\ncaused our tests to fail under prove(1) since we wouldn't print a\nnewline before TAP output:\n\n CONFLICT (content): Merge conflict in both.txt\n # GETTEXT POISON #ok 2 - -m restores 2-way conflicted+resolved file\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t2023-checkout-m.sh | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh\nindex 1a40ce0..7e18985 100755\n--- a/t/t2023-checkout-m.sh\n+++ b/t/t2023-checkout-m.sh\n@@ -17,7 +17,9 @@ test_expect_success setup '\n \ttest_commit added_in_topic each.txt in_topic\n '\n \n-test_must_fail git merge master\n+test_expect_success 'git merge master' '\n+ test_must_fail git merge master\n+'\n \n clean_branchnames () {\n \t# Remove branch names after conflict lines\n-- \n1.7.7.3\n"},{"id":"181539","messageId":"7v8vm7j5sj.fsf@alter.siamese.dyndns.org","threadId":"29083","inReplyTo":"1324413465-25614-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] t/t2023-checkout-m.sh: fix use of test_must_fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-20T21:23:56Z","receivedAt":"2011-12-20T21:23:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change an invocation of test_must_fail() to be inside a\n> test_expect_success() as is our usual pattern. Having it outside\n> caused our tests to fail under prove(1) since we wouldn't print a\n> newline before TAP output:\n>\n> CONFLICT (content): Merge conflict in both.txt\n> # GETTEXT POISON #ok 2 - -m restores 2-way conflicted+resolved file\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n\nThanks.\n\n> ---\n> t/t2023-checkout-m.sh | 4 +++-\n> 1 files changed, 3 insertions(+), 1 deletions(-)\n>\n> diff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh\n> index 1a40ce0..7e18985 100755\n> --- a/t/t2023-checkout-m.sh\n> +++ b/t/t2023-checkout-m.sh\n> @@ -17,7 +17,9 @@ test_expect_success setup '\n> \ttest_commit added_in_topic each.txt in_topic\n> '\n> \n> -test_must_fail git merge master\n> +test_expect_success 'git merge master' '\n> + test_must_fail git merge master\n> +'\n> \n> clean_branchnames () {\n> \t# Remove branch names after conflict lines\n"}]}