{"thread":{"id":"21940","subject":"[PATCH 1/3] commit/status: check $GIT_DIR/MERGE_HEAD only once","startedAt":"2009-12-12T09:02:00Z","lastAt":"2009-12-12T22:06:07Z","messageCount":10,"participants":["Junio C Hamano","Jeff King","Nanako Shiraishi","Sverre Rabbelier","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"129791","messageId":"1260608523-15579-1-git-send-email-gitster@pobox.com","threadId":"21940","inReplyTo":"7vk4wtysyu.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/3] Update advice in commit/status output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-12T09:02:00Z","receivedAt":"2009-12-12T09:02:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian noticed that we give \"git reset HEAD \" as an instruction\nto get rid of the local change that has already been added to the index\neven when is unmerged, or it is merged and we are about to commit a\nmerge.\n\nIn neither case, \"git reset HEAD \" is absolutely a wrong thing to do\nwhile merging.\n\nThis miniseries updates the advices given in status/commit. It applies on\ntop of the jk/1.7.0-status topic, and has trivial conflicts in wt-status.c\nwith the jk/unwanted-advices topic that has already graduated to 'maint'.\n\nJunio C Hamano (3):\n commit/status: check $GIT_DIR/MERGE_HEAD only once\n commit/status: \"git add \" is not necessarily how to resolve\n status/commit: do not suggest \"reset HEAD \" while merging\n\n builtin-commit.c | 12 ++++++------\n t/t7060-wtstatus.sh | 3 +--\n wt-status.c | 16 +++++++++++-----\n wt-status.h | 1 +\n 4 files changed, 19 insertions(+), 13 deletions(-)\n"},{"id":"129789","messageId":"1260608523-15579-2-git-send-email-gitster@pobox.com","threadId":"21940","inReplyTo":"1260608523-15579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] commit/status: check $GIT_DIR/MERGE_HEAD only once","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-12T09:02:01Z","receivedAt":"2009-12-12T09:02:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The code checked for the MERGE_HEAD file to see if we were about\nto commit a merge twice in the codepath; also one of them used a\nvariable merge_head_sha1[] which was set but was never used.\n\nJust check it once, but do so also in \"git status\", too, as\nwe will be using this for status generation in the next patch.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-commit.c | 10 ++++------\n 1 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex b39295f..17dd462 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -36,7 +36,7 @@ static const char * const builtin_status_usage[] = {\n \tNULL\n };\n \n-static unsigned char head_sha1[20], merge_head_sha1[20];\n+static unsigned char head_sha1[20];\n static char *use_message_buffer;\n static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n static struct lock_file index_lock; /* real index */\n@@ -319,7 +319,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tcommit_style = COMMIT_PARTIAL;\n \n-\tif (file_exists(git_path(\"MERGE_HEAD\")))\n+\tif (in_merge)\n \t\tdie(\"cannot do a partial commit during a merge.\");\n \n \tmemset(&partial, 0, sizeof(partial));\n@@ -758,9 +758,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (get_sha1(\"HEAD\", head_sha1))\n \t\tinitial_commit = 1;\n \n-\tif (!get_sha1(\"MERGE_HEAD\", merge_head_sha1))\n-\t\tin_merge = 1;\n-\n \t/* Sanity check options */\n \tif (amend && initial_commit)\n \t\tdie(\"You have nothing to amend.\");\n@@ -951,6 +948,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \twt_status_prepare(&s);\n \tgit_config(git_status_config, &s);\n+\tin_merge = file_exists(git_path(\"MERGE_HEAD\"));\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_status_options,\n \t\t\t builtin_status_usage, 0);\n@@ -1057,10 +1055,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \n \twt_status_prepare(&s);\n \tgit_config(git_commit_config, &s);\n+\tin_merge = file_exists(git_path(\"MERGE_HEAD\"));\n \n \tif (s.use_color == -1)\n \t\ts.use_color = git_use_color_default;\n-\n \targc = parse_and_validate_options(argc, argv, builtin_commit_usage,\n \t\t\t\t\t prefix, &s);\n \tif (dry_run) {\n-- \n1.6.6.rc2.5.g49666\n"},{"id":"129792","messageId":"1260608523-15579-3-git-send-email-gitster@pobox.com","threadId":"21940","inReplyTo":"1260608523-15579-2-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] commit/status: \"git add \" is not necessarily how to resolve","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-12T09:02:02Z","receivedAt":"2009-12-12T09:02:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When the desired resolution is to remove the path, \"git rm \" is the\ncommand the user needs to use. Just like in \"Changed but not updated\"\nsection, suggest to use \"git add/rm\" as appropriate.\n\nSigned-off-by: Junio C Hamano \n---\n t/t7060-wtstatus.sh | 2 +-\n wt-status.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh\nindex 7b5db80..6c1af26 100755\n--- a/t/t7060-wtstatus.sh\n+++ b/t/t7060-wtstatus.sh\n@@ -32,7 +32,7 @@ cat >expect <...\" to unstage)\n-# (use \"git add ...\" to mark resolution)\n+# (use \"git add/rm ...\" as appropriately to mark resolution)\n #\n #\tdeleted by us: foo\n #\ndiff --git a/wt-status.c b/wt-status.c\nindex 3fdcf97..5271b6a 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -52,7 +52,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git reset %s ...\\\" to unstage)\", s->reference);\n \telse\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git rm --cached ...\\\" to unstage)\");\n-\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add ...\\\" to mark resolution)\");\n+\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add/rm ...\\\" as appropriately to mark resolution)\");\n \tcolor_fprintf_ln(s->fp, c, \"#\");\n }\n \n-- \n1.6.6.rc2.5.g49666\n"},{"id":"129790","messageId":"1260608523-15579-4-git-send-email-gitster@pobox.com","threadId":"21940","inReplyTo":"1260608523-15579-3-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] status/commit: do not suggest \"reset HEAD \" while merging","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-12T09:02:03Z","receivedAt":"2009-12-12T09:02:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Suggesting \"'reset HEAD ' to unstage\" is dead wrong if we are about\nto record a merge commit. For either an unmerged path (i.e. with\nunresolved conflicts), or an updated path, it would result in discarding\nwhat the other branch did.\n\nNote that we do not do anything special in a case where we are amending a\nmerge. The user is making an evil merge starting from an already\ncommitted merge, and running \"reset HEAD \" is the right way to get\nrid of the local edit that has been added to the index.\n\nOnce \"reset --unresolve \" becomes available, we might want to\nsuggest it for a merged path that has unresolve information, but until\nthen, just remove the incorrect advice.\n\nWe might also want to suggest \"checkout --conflict \" to revert the\nfile in the work tree to the state of failed automerge for an unmerged\npath, but we never did that, and this commit does not change that.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-commit.c | 2 ++\n t/t7060-wtstatus.sh | 1 -\n wt-status.c | 14 ++++++++++----\n wt-status.h | 1 +\n 4 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 17dd462..7218454 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -960,6 +960,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tread_cache();\n \trefresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);\n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n+\ts.in_merge = in_merge;\n \twt_status_collect(&s);\n \n \tif (s.relative_paths)\n@@ -1056,6 +1057,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \twt_status_prepare(&s);\n \tgit_config(git_commit_config, &s);\n \tin_merge = file_exists(git_path(\"MERGE_HEAD\"));\n+\ts.in_merge = in_merge;\n \n \tif (s.use_color == -1)\n \t\ts.use_color = git_use_color_default;\ndiff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh\nindex 6c1af26..6dd7077 100755\n--- a/t/t7060-wtstatus.sh\n+++ b/t/t7060-wtstatus.sh\n@@ -31,7 +31,6 @@ test_expect_success 'Report new path with conflict' '\n cat >expect <...\" to unstage)\n # (use \"git add/rm ...\" as appropriately to mark resolution)\n #\n #\tdeleted by us: foo\ndiff --git a/wt-status.c b/wt-status.c\nindex 5271b6a..3f62c44 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -47,8 +47,11 @@ void wt_status_prepare(struct wt_status *s)\n static void wt_status_print_unmerged_header(struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n+\n \tcolor_fprintf_ln(s->fp, c, \"# Unmerged paths:\");\n-\tif (!s->is_initial)\n+\tif (s->in_merge)\n+\t\t;\n+\telse if (!s->is_initial)\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git reset %s ...\\\" to unstage)\", s->reference);\n \telse\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git rm --cached ...\\\" to unstage)\");\n@@ -59,12 +62,14 @@ static void wt_status_print_unmerged_header(struct wt_status *s)\n static void wt_status_print_cached_header(struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n+\n \tcolor_fprintf_ln(s->fp, c, \"# Changes to be committed:\");\n-\tif (!s->is_initial) {\n+\tif (s->in_merge)\n+\t\t; /* NEEDSWORK: use \"git reset --unresolve\"??? */\n+\telse if (!s->is_initial)\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git reset %s ...\\\" to unstage)\", s->reference);\n-\t} else {\n+\telse\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git rm --cached ...\\\" to unstage)\");\n-\t}\n \tcolor_fprintf_ln(s->fp, c, \"#\");\n }\n \n@@ -72,6 +77,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,\n \t\t\t\t\t int has_deleted)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n+\n \tcolor_fprintf_ln(s->fp, c, \"# Changed but not updated:\");\n \tif (!has_deleted)\n \t\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add ...\\\" to update what will be committed)\");\ndiff --git a/wt-status.h b/wt-status.h\nindex a4bddcf..c60f40a 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -34,6 +34,7 @@ struct wt_status {\n \tconst char **pathspec;\n \tint verbose;\n \tint amend;\n+\tint in_merge;\n \tint nowarn;\n \tint use_color;\n \tint relative_paths;\n-- \n1.6.6.rc2.5.g49666\n"},{"id":"129793","messageId":"20091212091556.GA30509@coredump.intra.peff.net","threadId":"21940","inReplyTo":"1260608523-15579-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/3] commit/status: \"git add \" is not necessarily how to resolve","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-12T09:15:56Z","receivedAt":"2009-12-12T09:15:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 12, 2009 at 01:02:02AM -0800, Junio C Hamano wrote:\n\n> When the desired resolution is to remove the path, \"git rm \" is the\n> command the user needs to use. Just like in \"Changed but not updated\"\n> section, suggest to use \"git add/rm\" as appropriate.\n\nI no longer even see these messages due to advice.statushints, but the\noverall direction of the series looks sane to me.\n\nHowever:\n\n> -\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add ...\\\" to mark resolution)\");\n> +\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add/rm ...\\\" as appropriately to mark resolution)\");\n\nThis should be \"as appropriate\".\n\n-Peff\n"},{"id":"129794","messageId":"20091212181925.6117@nanako3.lavabit.com","threadId":"21940","inReplyTo":"1260608523-15579-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/3] commit/status: \"git add \" is not necessarily how to resolve","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-12T09:19:25Z","receivedAt":"2009-12-12T09:19:25Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano \n\n> # On branch side\n> # Unmerged paths:\n> # (use \"git reset HEAD ...\" to unstage)\n> -# (use \"git add ...\" to mark resolution)\n> +# (use \"git add/rm ...\" as appropriately to mark resolution)\n\nShouldn't this either be \"as appropriate\" or just \"appropriately\"?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"129795","messageId":"7vbpi4v92s.fsf@alter.siamese.dyndns.org","threadId":"21940","inReplyTo":"20091212091556.GA30509@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] commit/status: \"git add \" is not necessarily how to resolve","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-12T09:24:27Z","receivedAt":"2009-12-12T09:24:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Dec 12, 2009 at 01:02:02AM -0800, Junio C Hamano wrote:\n>\n>> When the desired resolution is to remove the path, \"git rm \" is the\n>> command the user needs to use. Just like in \"Changed but not updated\"\n>> section, suggest to use \"git add/rm\" as appropriate.\n>\n> I no longer even see these messages due to advice.statushints, but the\n> overall direction of the series looks sane to me.\n>\n> However:\n>\n>> -\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add ...\\\" to mark resolution)\");\n>> +\tcolor_fprintf_ln(s->fp, c, \"# (use \\\"git add/rm ...\\\" as appropriately to mark resolution)\");\n>\n> This should be \"as appropriate\".\n\nHeh, I kant speel. Fixed and re-pushed out on 'pu'.\n\nThanks.\n"},{"id":"129800","messageId":"fabb9a1e0912120813h1c41b7bfg4525f8f01e04ddb1@mail.gmail.com","threadId":"21940","inReplyTo":"1260608523-15579-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/3] Update advice in commit/status output","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-12-12T16:13:09Z","receivedAt":"2009-12-12T16:13:09Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Sat, Dec 12, 2009 at 10:02, Junio C Hamano wrote:\n> In neither case, \"git reset HEAD \" is absolutely a wrong thing to do\n> while merging.\n\n>From the patches I'm guessing you mean \"In either case\" instead?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"129801","messageId":"7vws0st5e2.fsf@alter.siamese.dyndns.org","threadId":"21940","inReplyTo":"fabb9a1e0912120813h1c41b7bfg4525f8f01e04ddb1@mail.gmail.com","subject":"Re: [PATCH 0/3] Update advice in commit/status output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-12T18:27:01Z","receivedAt":"2009-12-12T18:27:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> Heya,\n>\n> On Sat, Dec 12, 2009 at 10:02, Junio C Hamano wrote:\n>> In neither case, \"git reset HEAD \" is absolutely a wrong thing to do\n>> while merging.\n>\n>>From the patches I'm guessing you mean \"In either case\" instead?\n\nCorrect.\n"},{"id":"129809","messageId":"76718490912121406q31ddcd12s3d813659d5096a22@mail.gmail.com","threadId":"21940","inReplyTo":"1260608523-15579-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/3] Update advice in commit/status output","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-12-12T22:06:07Z","receivedAt":"2009-12-12T22:06:07Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Dec 12, 2009 at 4:02 AM, Junio C Hamano wrote:\n> Jay Soffian noticed that we give \"git reset HEAD \" as an instruction\n> to get rid of the local change that has already been added to the index\n> even when is unmerged, or it is merged and we are about to commit a\n> merge.\n>\n> In neither case, \"git reset HEAD \" is absolutely a wrong thing to do\n> while merging.\n>\n> This miniseries updates the advices given in status/commit.  It applies on\n> top of the jk/1.7.0-status topic, and has trivial conflicts in wt-status.c\n> with the jk/unwanted-advices topic that has already graduated to 'maint'.\n\nSeries looks good to me after the spelling correction. Thank you.\n\nj.\n"}]}