{"thread":{"id":"22179","subject":"[PATCH (resend)] Be more user-friendly when refusing to do something because of conflict.","startedAt":"2010-01-12T09:54:44Z","lastAt":"2010-01-12T09:54:44Z","messageCount":1,"participants":["Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"131361","messageId":"1263290084-21390-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22179","inReplyTo":null,"subject":"[PATCH (resend)] Be more user-friendly when refusing to do something because of conflict.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-12T09:54:44Z","receivedAt":"2010-01-12T09:54:44Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Various commands refuse to run in the presence of conflicts (commit,\nmerge, pull, cherry-pick/revert). They all used to provide rough, and\ninconsistant error messages.\n\nA new variable advice.resolveconflict is introduced, and allows more\nverbose messages, pointing the user to the appropriate solution.\n\nFor commit, the error message used to look like this:\n\n$ git commit\nfoo.txt: needs merge\nfoo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\nfoo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\nfoo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\nerror: Error building trees\n\nThe \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\noption to make the output more consistant with the other porcelain\ncommands, and catch the error in return, to stop with a clean error\nmessage. The next lines were displayed by a call to cache_tree_update(),\nwhich is not reached anymore if we noticed the conflict.\n\nThe new output looks like:\n\nU       foo.txt\nfatal: 'commit' is not possible because you have unmerged files.\nPlease, fix them up in the work tree, and then use 'git add/rm <file>' as\nappropriate to mark resolution and make a commit, or use 'git commit -a'.\n\nPull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD\nexists instead of waiting for merge to complain.\n\nThe behavior of merge and the test-case are slightly modified to reflect\nthe usual flow: start with conflicts, fix them, and afterwards get rid of\nMERGE_HEAD, with different error messages at each stage.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nNo real change since last version, I just rebased it on pu (and\nremoved a trailing whitespace on the way). Just making sure my patch\nisn't lost.\n\n Documentation/config.txt      |    4 ++++\n advice.c                      |   16 ++++++++++++++++\n advice.h                      |    5 +++++\n builtin-commit.c              |   14 ++++++++++++--\n builtin-merge.c               |   20 +++++++++++++++-----\n builtin-revert.c              |   15 ++++++++++++++-\n git-pull.sh                   |   25 +++++++++++++++++++++++--\n t/t3030-merge-recursive.sh    |    6 ++++--\n t/t3501-revert-cherry-pick.sh |    2 +-\n 9 files changed, 94 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7955174..29ac66f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -130,6 +130,10 @@ advice.*::\n \t\tAdvice shown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwritting local changes.\n \t\tDefault: true.\n+\tresolveConflict::\n+\t\tAdvices shown by various commands when conflicts\n+\t\tprevent the operation from being performed.\n+\t\tDefault: true.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex cb666ac..3309521 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -3,6 +3,7 @@\n int advice_push_nonfastforward = 1;\n int advice_status_hints = 1;\n int advice_commit_before_merge = 1;\n+int advice_resolve_conflict = 1;\n \n static struct {\n \tconst char *name;\n@@ -11,6 +12,7 @@ static struct {\n \t{ \"pushnonfastforward\", &advice_push_nonfastforward },\n \t{ \"statushints\", &advice_status_hints },\n \t{ \"commitbeforemerge\", &advice_commit_before_merge },\n+\t{ \"resolveconflict\", &advice_resolve_conflict },\n };\n \n int git_default_advice_config(const char *var, const char *value)\n@@ -27,3 +29,17 @@ int git_default_advice_config(const char *var, const char *value)\n \n \treturn 0;\n }\n+\n+void NORETURN die_resolve_conflict(const char *me)\n+{\n+\tif (advice_resolve_conflict)\n+\t\t/*\n+\t\t * Message used both when 'git commit' fails and when\n+\t\t * other commands doing a merge do.\n+\t\t */\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n+\t\t    \"Please, fix them up in the work tree, and then use 'git add/rm <file>' as\\n\"\n+\t\t    \"appropriate to mark resolution and make a commit, or use 'git commit -a'.\", me);\n+\telse\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n+}\ndiff --git a/advice.h b/advice.h\nindex 3de5000..acd5fdd 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -1,10 +1,15 @@\n #ifndef ADVICE_H\n #define ADVICE_H\n \n+#include \"git-compat-util.h\"\n+\n extern int advice_push_nonfastforward;\n extern int advice_status_hints;\n extern int advice_commit_before_merge;\n+extern int advice_resolve_conflict;\n \n int git_default_advice_config(const char *var, const char *value);\n \n+extern void NORETURN die_resolve_conflict(const char *me);\n+\n #endif /* ADVICE_H */\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 6199db7..a716603 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -245,6 +245,16 @@ static void create_base_index(void)\n \t\texit(128); /* We've already reported the error, finish dying */\n }\n \n+static void refresh_cache_or_die(int refresh_flags)\n+{\n+\t/*\n+\t * refresh_flags contains REFRESH_QUIET, so the only errors\n+\t * are for unmerged entries.\n+\t */\n+\tif (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))\n+\t\tdie_resolve_conflict(\"commit\");\n+}\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)\n {\n \tint fd;\n@@ -284,7 +294,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (all || (also && pathspec && *pathspec)) {\n \t\tint fd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n@@ -303,7 +313,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 4fe516e..3aaec7b 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -886,11 +886,21 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n \tstruct commit_list **remotes = &remoteheads;\n \n-\tif (file_exists(git_path(\"MERGE_HEAD\")))\n-\t\tdie(\"You have not concluded your merge. (MERGE_HEAD exists)\");\n-\tif (read_cache_unmerged())\n-\t\tdie(\"You are in the middle of a conflicted merge.\"\n-\t\t\t\t\" (index unmerged)\");\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(\"merge\");\n+\t}\n+\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n+\t\t/*\n+\t\t * There is no unmerged entry, don't advise 'git\n+\t\t * add/rm <file>', just 'git commit'.\n+\t\t */\n+\t\tif (advice_resolve_conflict)\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n+\t\t\t    \"Please, commit your changes before you can merge.\");\n+\t\telse\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n+\t}\n+\n \tresolve_undo_clear();\n \t/*\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 857ca2e..8ac86f0 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -235,6 +235,19 @@ static struct tree *empty_tree(void)\n \treturn tree;\n }\n \n+static NORETURN void die_dirty_index(const char *me)\n+{\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(me);\n+\t} else {\n+\t\tif (advice_commit_before_merge)\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\"\n+\t\t\t    \"Please, commit your changes or stash them to proceed.\", me);\n+\t\telse\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\", me);\n+\t}\n+}\n+\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n@@ -271,7 +284,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\tif (get_sha1(\"HEAD\", head))\n \t\t\tdie (\"You do not have a valid HEAD\");\n \t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie (\"Dirty index: cannot %s\", me);\n+\t\t\tdie_dirty_index(me);\n \t}\n \tdiscard_cache();\n \ndiff --git a/git-pull.sh b/git-pull.sh\nindex 8c0905a..754daaa 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -13,8 +13,29 @@ set_reflog_action \"pull $*\"\n require_work_tree\n cd_to_toplevel\n \n-test -z \"$(git ls-files -u)\" ||\n-\tdie \"You are in the middle of a conflicted merge.\"\n+\n+die_conflict () {\n+    git diff-index --cached --name-status -r --ignore-submodules HEAD --\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"Pull is not possible because you have unmerged files.\n+Please, fix them up in the work tree, and then use 'git add/rm <file>'\n+as appropriate to mark resolution, or use 'git commit -a'.\"\n+    else\n+\tdie \"Pull is not possible because you have unmerged files.\"\n+    fi\n+}\n+\n+die_merge () {\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\n+Please, commit your changes before you can merge.\"\n+    else\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\"\n+    fi\n+}\n+\n+test -z \"$(git ls-files -u)\" || die_conflict\n+test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity=\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 9b3fa2b..9929f82 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -276,11 +276,13 @@ test_expect_success 'fail if the index has unresolved entries' '\n \n \ttest_must_fail git merge \"$c5\" &&\n \ttest_must_fail git merge \"$c5\" 2> out &&\n+\tgrep \"not possible because you have unmerged files\" out &&\n+\tgit add -u &&\n+\ttest_must_fail git merge \"$c5\" 2> out &&\n \tgrep \"You have not concluded your merge\" out &&\n \trm -f .git/MERGE_HEAD &&\n \ttest_must_fail git merge \"$c5\" 2> out &&\n-\tgrep \"You are in the middle of a conflicted merge\" out\n-\n+\tgrep \"Your local changes to .* would be overwritten by merge.\" out\n '\n \n test_expect_success 'merge-recursive remove conflict' '\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex bb4cf00..7f85815 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -66,7 +66,7 @@ test_expect_success 'revert forbidden on dirty working tree' '\n \techo content >extra_file &&\n \tgit add extra_file &&\n \ttest_must_fail git revert HEAD 2>errors &&\n-\tgrep \"Dirty index\" errors\n+\tgrep \"Your local changes would be overwritten by \" errors\n \n '\n \n-- \n1.6.6.198.gbaea2\n"}]}