{"thread":{"id":"21785","subject":"[PATCH 0/2] User-friendly error messages for merge failure on fast-forward.","startedAt":"2009-11-29T12:18:31Z","lastAt":"2009-11-30T08:42:49Z","messageCount":5,"participants":["Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"128683","messageId":"1259497113-1393-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"21785","inReplyTo":null,"subject":"[PATCH 0/2] User-friendly error messages for merge failure on fast-forward.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-11-29T12:18:31Z","receivedAt":"2009-11-29T12:18:31Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"I noticed that failures of fast-forward merges were still using the\nplumbing error message:\n\n$ git merge master\nUpdating 1fdeef9..e248dad\nerror: Entry 'foo.txt' not uptodate. Cannot merge.\n\nI'm not 100% sure that my patch doesn't trigger the porcelain behavior\nfor plumbing. Someone more familiar than me with the codebase should\nconfirm/infirm this.\n\nMatthieu Moy (2):\n merge-recursive: make the error-message generation an extern function\n builtin-merge: show user-friendly error messages for fast-forward\n too.\n\n builtin-merge.c | 1 +\n merge-recursive.c | 41 +++++++++++++++++++++++------------------\n merge-recursive.h | 3 +++\n 3 files changed, 27 insertions(+), 18 deletions(-)\n"},{"id":"128685","messageId":"1259497113-1393-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"21785","inReplyTo":"1259497113-1393-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/2] merge-recursive: make the error-message generation an extern function","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-11-29T12:18:32Z","receivedAt":"2009-11-29T12:18:32Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The construction of the struct unpack_trees_error_msgs was done within\ngit_merge_trees(), which prevented using the same messages easily from\nanother function.\n\nSigned-off-by: Matthieu Moy \n---\n merge-recursive.c | 41 +++++++++++++++++++++++------------------\n merge-recursive.h | 3 +++\n 2 files changed, 26 insertions(+), 18 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a91208f..70cd6cc 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -172,23 +172,6 @@ static int git_merge_trees(int index_only,\n \tint rc;\n \tstruct tree_desc t[3];\n \tstruct unpack_trees_options opts;\n-\tstruct unpack_trees_error_msgs msgs = {\n-\t\t/* would_overwrite */\n-\t\t\"Your local changes to '%s' would be overwritten by merge. Aborting.\",\n-\t\t/* not_uptodate_file */\n-\t\t\"Your local changes to '%s' would be overwritten by merge. Aborting.\",\n-\t\t/* not_uptodate_dir */\n-\t\t\"Updating '%s' would lose untracked files in it. Aborting.\",\n-\t\t/* would_lose_untracked */\n-\t\t\"Untracked working tree file '%s' would be %s by merge. Aborting\",\n-\t\t/* bind_overlap -- will not happen here */\n-\t\tNULL,\n-\t};\n-\tif (advice_commit_before_merge) {\n-\t\tmsgs.would_overwrite = msgs.not_uptodate_file =\n-\t\t\t\"Your local changes to '%s' would be overwritten by merge. Aborting.\\n\"\n-\t\t\t\"Please, commit your changes or stash them before you can merge.\";\n-\t}\n \n \tmemset(&opts, 0, sizeof(opts));\n \tif (index_only)\n@@ -200,7 +183,7 @@ static int git_merge_trees(int index_only,\n \topts.fn = threeway_merge;\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n-\topts.msgs = msgs;\n+\topts.msgs = get_porcelain_error_msgs();\n \n \tinit_tree_desc_from_tree(t+0, common);\n \tinit_tree_desc_from_tree(t+1, head);\n@@ -1188,6 +1171,28 @@ static int process_entry(struct merge_options *o,\n \treturn clean_merge;\n }\n \n+struct unpack_trees_error_msgs get_porcelain_error_msgs()\n+{\n+\tstruct unpack_trees_error_msgs msgs = {\n+\t\t/* would_overwrite */\n+\t\t\"Your local changes to '%s' would be overwritten by merge. Aborting.\",\n+\t\t/* not_uptodate_file */\n+\t\t\"Your local changes to '%s' would be overwritten by merge. Aborting.\",\n+\t\t/* not_uptodate_dir */\n+\t\t\"Updating '%s' would lose untracked files in it. Aborting.\",\n+\t\t/* would_lose_untracked */\n+\t\t\"Untracked working tree file '%s' would be %s by merge. Aborting\",\n+\t\t/* bind_overlap -- will not happen here */\n+\t\tNULL,\n+\t};\n+\tif (advice_commit_before_merge) {\n+\t\tmsgs.would_overwrite = msgs.not_uptodate_file =\n+\t\t\t\"Your local changes to '%s' would be overwritten by merge. Aborting.\\n\"\n+\t\t\t\"Please, commit your changes or stash them before you can merge.\";\n+\t}\n+\treturn msgs;\n+}\n+\n int merge_trees(struct merge_options *o,\n \t\tstruct tree *head,\n \t\tstruct tree *merge,\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex fd138ca..f4b7f57 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -17,6 +17,9 @@ struct merge_options {\n \tstruct string_list current_directory_set;\n };\n \n+/* Return a list of user-friendly error messages to be used by merge */\n+struct unpack_trees_error_msgs get_porcelain_error_msgs();\n+\n /* merge_trees() but with recursive ancestor consolidation */\n int merge_recursive(struct merge_options *o,\n \t\t struct commit *h1,\n-- \n1.6.5.3.435.g5f2e3.dirty\n"},{"id":"128684","messageId":"1259497113-1393-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"21785","inReplyTo":"1259497113-1393-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[RFC/PATCH 2/2] builtin-merge: show user-friendly error messages for fast-forward too.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-11-29T12:18:33Z","receivedAt":"2009-11-29T12:18:33Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"fadd069d03 (merge-recursive: give less scary messages when merge did not\nstart, Sep 7 2009) introduced some friendlier error message for merge\nfailure, but the messages were shown only for non-fast forward merges.\nThis patch uses the same for fast-forward.\n\nSigned-off-by: Matthieu Moy \n---\nThis is where I'm not 100% sure I'm not breaking some plumbing.\n\n builtin-merge.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 57eedd4..0dd363f 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -656,6 +656,7 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote)\n \topts.verbose_update = 1;\n \topts.merge = 1;\n \topts.fn = twoway_merge;\n+\topts.msgs = get_porcelain_error_msgs();\n \n \ttrees[nr_trees] = parse_tree_indirect(head);\n \tif (!trees[nr_trees++])\n-- \n1.6.5.3.435.g5f2e3.dirty\n"},{"id":"128712","messageId":"7vy6lo7nu8.fsf@alter.siamese.dyndns.org","threadId":"21785","inReplyTo":"1259497113-1393-3-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC/PATCH 2/2] builtin-merge: show user-friendly error messages for fast-forward too.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T02:23:11Z","receivedAt":"2009-11-30T02:23:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> This is where I'm not 100% sure I'm not breaking some plumbing.\n\nI think this change is safe for the current codebase, as the only caller\nof checkout_fast_forward() is cmd_merge(), which is an implementation of\n\"git merge\". Even if there were a plumbing implementation that internally\nruns \"git merge\", either by calling cmd_merge() or via run_command()\ninterface (there isn't as far as I know), or if somebody adds such a thing\nin the future, such a \"plumbing\" will get Porcelain messages from other\ncodepaths in cmd_merge() anyway. You are not introducing a new problem.\n\nOr did you have something a lot more subtle in mind?\n\nSide note. checkout_fast_forward() switches between two commits (the one\nthat matches HEAD) to another (given as remote), which is the same as what\n\"git checkout other-branch\" and \"git checkout -b new-branch\" do. We might\nwant to replace it with the main part of merge_working_tree() from\nbuiltin-checkout.c eventually, which would teach the \"checkout -m\" logic\nthat carries the local changes forward to fast-forward of \"git merge\".\n\n> builtin-merge.c | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-merge.c b/builtin-merge.c\n> index 57eedd4..0dd363f 100644\n> --- a/builtin-merge.c\n> +++ b/builtin-merge.c\n> @@ -656,6 +656,7 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote)\n> \topts.verbose_update = 1;\n> \topts.merge = 1;\n> \topts.fn = twoway_merge;\n> +\topts.msgs = get_porcelain_error_msgs();\n> \n> \ttrees[nr_trees] = parse_tree_indirect(head);\n> \tif (!trees[nr_trees++])\n"},{"id":"128752","messageId":"vpq3a3wtncm.fsf@bauges.imag.fr","threadId":"21785","inReplyTo":"7vy6lo7nu8.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 2/2] builtin-merge: show user-friendly error messages for fast-forward too.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-11-30T08:42:49Z","receivedAt":"2009-11-30T08:42:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> This is where I'm not 100% sure I'm not breaking some plumbing.\n>\n> I think this change is safe [...]\n>\n> Or did you have something a lot more subtle in mind?\n\nNot at all. But since I found the place to add my code more or less\nrandomly with gdb, I feel safer with your comment ;-).\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}