{"thread":{"id":"38912","subject":"[PATCH] merge: deprecate 'git merge HEAD ' syntax","startedAt":"2015-03-26T04:58:45Z","lastAt":"2015-03-26T13:51:10Z","messageCount":4,"participants":["Junio C Hamano","Jeff King","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"258531","messageId":"xmqqvbhoz75m.fsf@gitster.dls.corp.google.com","threadId":"38912","inReplyTo":null,"subject":"[PATCH] merge: deprecate 'git merge HEAD ' syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-26T04:58:45Z","receivedAt":"2015-03-26T04:58:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We had this in \"git merge\" manual for eternity:\n\n 'git merge' HEAD ...\n\n [This] syntax ( `HEAD` ...) is supported for\n historical reasons. Do not use it from the command line or in\n new scripts. It is the same as `git merge -m ...`.\n\nI wanted to see how much damage we would incur to people by checking\nwhat the damage to _our_ system, including the test scripts, if we\ndropped the support for the syntax. The last time I tried this, I\nthought that replacing the use of this syntax in \"git pull\" with its\nobvious and trivial rewrite 'git merge -m ' broke some\noutput, and that is the primary reason why I stayed away from trying\nthis again, but it no longer seems to be the case with today's code\nfor some reason.\n\nThere are quite a few fallouts in the test scripts, and it turns out\nthat \"git cvsimport\" also uses this old syntax to record a merge.\n\nJudging from this result, I would not be surprised if dropping the\nsupport of the old syntax broke scripts people have written and been\nrelying on for the past ten years. But at least we can start the\ndeprecation process by throwing a warning message when the syntax is\nused.\n\nWith luck, we might be able to drop the support in a few years.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/merge.c | 1 +\n git-cvsimport.perl | 2 +-\n git-pull.sh | 2 +-\n t/t3402-rebase-merge.sh | 2 +-\n t/t6021-merge-criss-cross.sh | 6 +++---\n t/t9402-git-cvsserver-refs.sh | 2 +-\n 6 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 3b0f8f9..0795358 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1182,6 +1182,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \n \tif (!have_message && head_commit &&\n \t is_old_style_invocation(argc, argv, head_commit->object.sha1)) {\n+\t\twarning(\"old-style 'git merge HEAD ' is deprecated.\");\n \t\tstrbuf_addstr(&merge_msg, argv[0]);\n \t\thead_arg = argv[1];\n \t\targv += 2;\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 73d367c..82ecb03 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -1162,7 +1162,7 @@ sub commit {\n \t\tdie \"Fast-forward update failed: $?\\n\" if $?;\n \t}\n \telse {\n-\t\tsystem(qw(git merge cvsimport HEAD), \"$remote/$opt_o\");\n+\t\tsystem(qw(git merge -m cvsimport), \"$remote/$opt_o\");\n \t\tdie \"Could not merge $opt_o into the current branch.\\n\" if $?;\n \t}\n } else {\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 4d4fc77..252969e 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -334,7 +334,7 @@ true)\n \teval=\"git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only\"\n \teval=\"$eval $log_arg $strategy_args $merge_args $verbosity $progress\"\n \teval=\"$eval $gpg_sign_args\"\n-\teval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n+\teval=\"$eval -m \\\"\\$merge_name\\\" $merge_head\"\n \t;;\n esac\n eval \"exec $eval\"\ndiff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh\nindex 5a27ec9..8f64505 100755\n--- a/t/t3402-rebase-merge.sh\n+++ b/t/t3402-rebase-merge.sh\n@@ -47,7 +47,7 @@ test_expect_success setup '\n '\n \n test_expect_success 'reference merge' '\n-\tgit merge -s recursive \"reference merge\" HEAD master\n+\tgit merge -s recursive -m \"reference merge\" master\n '\n \n PRE_REBASE=$(git rev-parse test-rebase)\ndiff --git a/t/t6021-merge-criss-cross.sh b/t/t6021-merge-criss-cross.sh\nindex d15b313..213deec 100755\n--- a/t/t6021-merge-criss-cross.sh\n+++ b/t/t6021-merge-criss-cross.sh\n@@ -48,7 +48,7 @@ echo \"1\n \" > file &&\n git commit -m \"C3\" file &&\n git branch C3 &&\n-git merge \"pre E3 merge\" B A &&\n+git merge -m \"pre E3 merge\" A &&\n echo \"1\n 2\n 3 changed in E3, branch B. New file size\n@@ -61,7 +61,7 @@ echo \"1\n \" > file &&\n git commit -m \"E3\" file &&\n git checkout A &&\n-git merge \"pre D8 merge\" A C3 &&\n+git merge -m \"pre D8 merge\" C3 &&\n echo \"1\n 2\n 3 changed in C3, branch B\n@@ -73,7 +73,7 @@ echo \"1\n 9\" > file &&\n git commit -m D8 file'\n \n-test_expect_success 'Criss-cross merge' 'git merge \"final merge\" A B'\n+test_expect_success 'Criss-cross merge' 'git merge -m \"final merge\" B'\n \n cat > file-expect < HEAD ' syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-26T05:00:48Z","receivedAt":"2015-03-26T05:00:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"And then if we and our users survived the previous \"start warning if\nthe old syntax is used\" patch for a few years, we could apply this\nto actually drop the support for the ancient syntax.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-merge.txt | 7 +------\n builtin/merge.c | 38 +-------------------------------------\n 2 files changed, 2 insertions(+), 43 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 1f94908..4a2f519 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -12,7 +12,6 @@ SYNOPSIS\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n \t[-s ] [-X ] [-S[]]\n \t[--[no-]rerere-autoupdate] [-m ] [...]\n-'git merge' HEAD ...\n 'git merge' --abort\n \n DESCRIPTION\n@@ -44,11 +43,7 @@ a log message from the user describing the changes.\n D---E---F---G---H master\n ------------\n \n-The second syntax ( `HEAD` ...) is supported for\n-historical reasons. Do not use it from the command line or in\n-new scripts. It is the same as `git merge -m ...`.\n-\n-The third syntax (\"`git merge --abort`\") can only be run after the\n+The second syntax (\"`git merge --abort`\") can only be run after the\n merge has resulted in conflicts. 'git merge --abort' will abort the\n merge process and try to reconstruct the pre-merge state. However,\n if there were uncommitted changes when the merge started (and\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 0795358..a4ddd31 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -43,7 +43,6 @@ struct strategy {\n \n static const char * const builtin_merge_usage[] = {\n \tN_(\"git merge [] [...]\"),\n-\tN_(\"git merge [] HEAD \"),\n \tN_(\"git merge --abort\"),\n \tNULL\n };\n@@ -902,24 +901,6 @@ static int suggest_conflicts(void)\n \treturn 1;\n }\n \n-static struct commit *is_old_style_invocation(int argc, const char **argv,\n-\t\t\t\t\t const unsigned char *head)\n-{\n-\tstruct commit *second_token = NULL;\n-\tif (argc > 2) {\n-\t\tunsigned char second_sha1[20];\n-\n-\t\tif (get_sha1(argv[1], second_sha1))\n-\t\t\treturn NULL;\n-\t\tsecond_token = lookup_commit_reference_gently(second_sha1, 0);\n-\t\tif (!second_token)\n-\t\t\tdie(_(\"'%s' is not a commit\"), argv[1]);\n-\t\tif (hashcmp(second_token->object.sha1, head))\n-\t\t\treturn NULL;\n-\t}\n-\treturn second_token;\n-}\n-\n static int evaluate_result(void)\n {\n \tint cnt = 0;\n@@ -1171,24 +1152,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_merge_usage,\n \t\t\tbuiltin_merge_options);\n \n-\t/*\n-\t * This could be traditional \"merge HEAD ...\" and\n-\t * the way we can tell it is to see if the second token is HEAD,\n-\t * but some people might have misused the interface and used a\n-\t * commit-ish that is the same as HEAD there instead.\n-\t * Traditional format never would have \"-m\" so it is an\n-\t * additional safety measure to check for it.\n-\t */\n-\n-\tif (!have_message && head_commit &&\n-\t is_old_style_invocation(argc, argv, head_commit->object.sha1)) {\n-\t\twarning(\"old-style 'git merge HEAD ' is deprecated.\");\n-\t\tstrbuf_addstr(&merge_msg, argv[0]);\n-\t\thead_arg = argv[1];\n-\t\targv += 2;\n-\t\targc -= 2;\n-\t\tremoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);\n-\t} else if (!head_commit) {\n+\tif (!head_commit) {\n \t\tstruct commit *remote_head;\n \t\t/*\n \t\t * If the merged head is a valid one there is no reason\n-- \n2.3.4-475-g3180e2e\n"},{"id":"258546","messageId":"20150326132014.GA16427@peff.net","threadId":"38912","inReplyTo":"xmqqvbhoz75m.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] merge: deprecate 'git merge HEAD ' syntax","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-26T13:20:15Z","receivedAt":"2015-03-26T13:20:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 25, 2015 at 09:58:45PM -0700, Junio C Hamano wrote:\n\n> We had this in \"git merge\" manual for eternity:\n> \n> 'git merge' HEAD ...\n> \n> [This] syntax ( `HEAD` ...) is supported for\n> historical reasons. Do not use it from the command line or in\n> new scripts. It is the same as `git merge -m ...`.\n\nIt would be nice to see this ancient wart ago. I agree we need a\ndeprecation period, though, as you've outlined here.\n\n> builtin/merge.c | 1 +\n> git-cvsimport.perl | 2 +-\n> git-pull.sh | 2 +-\n> t/t3402-rebase-merge.sh | 2 +-\n> t/t6021-merge-criss-cross.sh | 6 +++---\n> t/t9402-git-cvsserver-refs.sh | 2 +-\n> 6 files changed, 8 insertions(+), 7 deletions(-)\n\nMaybe squash in:\n\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex 27c3d73..2af1bee 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -24,7 +24,7 @@ test_expect_success 'prepare repository' '\n '\n \n test_expect_success 'Merge with d/f conflicts' '\n-\ttest_expect_code 1 git merge \"merge msg\" B master\n+\ttest_expect_code 1 git merge -m \"merge msg\" master\n '\n \n test_expect_success 'F/D conflict' '\n\n\nWe do not call it HEAD here, but the setup test just before has put us\non branch B, so I think it is equivalent.\n\n-Peff\n"},{"id":"258550","messageId":"CAPig+cR6CiEgQzmXS2w7XLfNWpm=vD-kTu5NmyLBLT3z=JAkAg@mail.gmail.com","threadId":"38912","inReplyTo":"xmqqvbhoz75m.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] merge: deprecate 'git merge HEAD ' syntax","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-26T13:51:10Z","receivedAt":"2015-03-26T13:51:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 26, 2015 at 12:58 AM, Junio C Hamano wrote:\n> We had this in \"git merge\" manual for eternity:\n>\n> 'git merge' HEAD ...\n>\n> [This] syntax ( `HEAD` ...) is supported for\n> historical reasons. Do not use it from the command line or in\n> new scripts. It is the same as `git merge -m ...`.\n>\n> I wanted to see how much damage we would incur to people by checking\n> what the damage to _our_ system, including the test scripts, if we\n> dropped the support for the syntax. The last time I tried this, I\n> thought that replacing the use of this syntax in \"git pull\" with its\n> obvious and trivial rewrite 'git merge -m ' broke some\n> output, and that is the primary reason why I stayed away from trying\n> this again, but it no longer seems to be the case with today's code\n> for some reason.\n>\n> There are quite a few fallouts in the test scripts, and it turns out\n> that \"git cvsimport\" also uses this old syntax to record a merge.\n>\n> Judging from this result, I would not be surprised if dropping the\n> support of the old syntax broke scripts people have written and been\n> relying on for the past ten years. But at least we can start the\n> deprecation process by throwing a warning message when the syntax is\n> used.\n>\n> With luck, we might be able to drop the support in a few years.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 3b0f8f9..0795358 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -1182,6 +1182,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>\n> if (!have_message && head_commit &&\n> is_old_style_invocation(argc, argv, head_commit->object.sha1)) {\n> + warning(\"old-style 'git merge HEAD ' is deprecated.\");\n\nTo be a bit more helpful, perhaps point the user at \"git merge -m\n ...\" as the recommended replacement?\n\n> strbuf_addstr(&merge_msg, argv[0]);\n> head_arg = argv[1];\n> argv += 2;\n"}]}