{"thread":{"id":"12431","subject":"[PATCH/RFC] git-merge.sh: better handling of combined --squash,--no-ff,--no-commit options","startedAt":"2008-03-02T17:58:19Z","lastAt":"2008-03-03T09:22:03Z","messageCount":3,"participants":["Gerrit Pape","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"70677","messageId":"20080302175820.31385.qmail@9e9c5b8314ca7b.315fe32.mid.smarden.org","threadId":"12431","inReplyTo":null,"subject":"[PATCH/RFC] git-merge.sh: better handling of combined --squash,--no-ff,--no-commit options","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-03-02T17:58:19Z","receivedAt":"2008-03-02T17:58:19Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"git-merge used to use either the --squash,--no-squash, --no-ff,--ff,\n--no-commit,--commit option, whichever came last in the command line.\nThis lead to some un-intuitive behavior, having\n\n git merge --no-commit --no-ff <branch>\n\nactually commit the merge.  Now git-merge respects --no-commit together\nwith --no-ff, as well as other combinations of the options.  However,\nthis broke a selftest in t/t7600-merge.sh which expected to have --no-ff\ncompletely override the --squash option, so that\n\n git merge --squash --no-ff <branch>\n\nfast-forwards, and makes a merge commit; now it prepares a squash\ncommit.  Combining --squash with --no-ff doesn't seem to make sense\nthough, so the test is adapted to test --no-ff without the preceding\n--squash.\n\nThe unexpected behavior was reported by John Goerzen through\n http://bing.sdebian.org/468568\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n\n---\n git-merge.sh     |   13 +++++++------\n t/t7600-merge.sh |    1 +\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/git-merge.sh b/git-merge.sh\nindex 1c123a3..2a5c456 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -37,6 +37,7 @@ use_strategies=\n \n allow_fast_forward=t\n allow_trivial_merge=t\n+squash= no_commit=\n \n dropsave() {\n \trm -f -- \"$GIT_DIR/MERGE_HEAD\" \"$GIT_DIR/MERGE_MSG\" \\\n@@ -152,17 +153,17 @@ parse_config () {\n \t\t--summary)\n \t\t\tshow_diffstat=t ;;\n \t\t--squash)\n-\t\t\tallow_fast_forward=t squash=t no_commit=t ;;\n+\t\t\tsquash=t no_commit=t ;;\n \t\t--no-squash)\n-\t\t\tallow_fast_forward=t squash= no_commit= ;;\n+\t\t\tsquash= no_commit= ;;\n \t\t--commit)\n-\t\t\tallow_fast_forward=t squash= no_commit= ;;\n+\t\t\tno_commit= ;;\n \t\t--no-commit)\n-\t\t\tallow_fast_forward=t squash= no_commit=t ;;\n+\t\t\tno_commit=t ;;\n \t\t--ff)\n-\t\t\tallow_fast_forward=t squash= no_commit= ;;\n+\t\t\tallow_fast_forward=t ;;\n \t\t--no-ff)\n-\t\t\tallow_fast_forward=false squash= no_commit= ;;\n+\t\t\tallow_fast_forward=f ;;\n \t\t-s|--strategy)\n \t\t\tshift\n \t\t\tcase \" $all_strategies \" in\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 50c51c8..085f55f 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -419,6 +419,7 @@ test_debug 'gitk --all'\n \n test_expect_success 'merge c0 with c1 (no-ff)' '\n \tgit reset --hard c0 &&\n+\tgit config branch.master.mergeoptions \"\" &&\n \ttest_tick &&\n \tgit merge --no-ff c1 &&\n \tverify_merge file result.1 &&\n-- \n1.5.4.3\n\n"},{"id":"70714","messageId":"7vir04n0rt.fsf@gitster.siamese.dyndns.org","threadId":"12431","inReplyTo":"20080302175820.31385.qmail@9e9c5b8314ca7b.315fe32.mid.smarden.org","subject":"Re: [PATCH/RFC] git-merge.sh: better handling of combined --squash,--no-ff,--no-commit options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T23:50:46Z","receivedAt":"2008-03-02T23:50:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gerrit Pape <pape@smarden.org> writes:\n\n> git-merge used to use either the --squash,--no-squash, --no-ff,--ff,\n> --no-commit,--commit option, whichever came last in the command line.\n> This lead to some un-intuitive behavior, having\n>\n>  git merge --no-commit --no-ff <branch>\n>\n> actually commit the merge.  Now git-merge respects --no-commit together\n> with --no-ff, as well as other combinations of the options.  However,\n> this broke a selftest in t/t7600-merge.sh which expected to have --no-ff\n> completely override the --squash option, so that\n>\n>  git merge --squash --no-ff <branch>\n>\n> fast-forwards, and makes a merge commit; now it prepares a squash ...\n\nBoth make sense when they make sense (i.e. if you and the other side are\nnot fast-forward nor up-to-date and need a real merge).\n\n> ...  Combining --squash with --no-ff doesn't seem to make sense\n\nYeah, I think forbidding this combination would make much more sense.  The\nformer asks there be _no_ merge (the user does not want to have a merge\never), while the other one asks to create a merge even when there is no\nneed to (the user does want a merge).\n\nAre there other combinations that we should forbid?\n\n"},{"id":"70766","messageId":"20080303092203.9752.qmail@34b3887920a333.315fe32.mid.smarden.org","threadId":"12431","inReplyTo":"7vir04n0rt.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-merge.sh: better handling of combined --squash,--no-ff,--no-commit options","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-03-03T09:22:03Z","receivedAt":"2008-03-03T09:22:03Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"git-merge used to use either the --squash,--no-squash, --no-ff,--ff,\n--no-commit,--commit option, whichever came last in the command line.\nThis lead to some un-intuitive behavior, having\n\n git merge --no-commit --no-ff <branch>\n\nactually commit the merge.  Now git-merge respects --no-commit together\nwith --no-ff, as well as other combinations of the options.  However,\nthis broke a selftest in t/t7600-merge.sh which expected to have --no-ff\ncompletely override the --squash option, so that\n\n git merge --squash --no-ff <branch>\n\nfast-forwards, and makes a merge commit; combining --squash with --no-ff\ndoesn't really make sense though, and is now refused by git-merge.  The\ntest is adapted to test --no-ff without the preceding --squash, and\nanother test is added to make sure the --squash --no-ff combination is\nrefused.\n\nThe unexpected behavior was reported by John Goerzen through\n http://bing.sdebian.org/468568\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n\nOn Sun, Mar 02, 2008 at 03:50:46PM -0800, Junio C Hamano wrote:\n> Gerrit Pape <pape@smarden.org> writes:\n> > ...  Combining --squash with --no-ff doesn't seem to make sense\n> Yeah, I think forbidding this combination would make much more sense.  The\n> former asks there be _no_ merge (the user does not want to have a merge\n> ever), while the other one asks to create a merge even when there is no\n> need to (the user does want a merge).\n\nOkay.\n\n> Are there other combinations that we should forbid?\n\nI don't think so, it's just --squash --no-ff, and maybe --squash --ff,\nbut --ff is the default anyway.\n\nThe combination --squash --commit doesn't work with the current\nimplementation though, but it might make sense to have the squash\ncommitted automatically?\n\n\n git-merge.sh     |   17 +++++++++++------\n t/t7600-merge.sh |    6 ++++++\n 2 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/git-merge.sh b/git-merge.sh\nindex 1c123a3..03cd398 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -37,6 +37,7 @@ use_strategies=\n \n allow_fast_forward=t\n allow_trivial_merge=t\n+squash= no_commit=\n \n dropsave() {\n \trm -f -- \"$GIT_DIR/MERGE_HEAD\" \"$GIT_DIR/MERGE_MSG\" \\\n@@ -152,17 +153,21 @@ parse_config () {\n \t\t--summary)\n \t\t\tshow_diffstat=t ;;\n \t\t--squash)\n-\t\t\tallow_fast_forward=t squash=t no_commit=t ;;\n+\t\t\ttest \"$allow_fast_forward\" = t ||\n+\t\t\t\tdie \"You cannot combine --squash with --no-ff.\"\n+\t\t\tsquash=t no_commit=t ;;\n \t\t--no-squash)\n-\t\t\tallow_fast_forward=t squash= no_commit= ;;\n+\t\t\tsquash= no_commit= ;;\n \t\t--commit)\n-\t\t\tallow_fast_forward=t squash= no_commit= ;;\n+\t\t\tno_commit= ;;\n \t\t--no-commit)\n-\t\t\tallow_fast_forward=t squash= no_commit=t ;;\n+\t\t\tno_commit=t ;;\n \t\t--ff)\n-\t\t\tallow_fast_forward=t squash= no_commit= ;;\n+\t\t\tallow_fast_forward=t ;;\n \t\t--no-ff)\n-\t\t\tallow_fast_forward=false squash= no_commit= ;;\n+\t\t\ttest \"$squash\" != t ||\n+\t\t\t\tdie \"You cannot combine --squash with --no-ff.\"\n+\t\t\tallow_fast_forward=f ;;\n \t\t-s|--strategy)\n \t\t\tshift\n \t\t\tcase \" $all_strategies \" in\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 50c51c8..5d16628 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -419,6 +419,7 @@ test_debug 'gitk --all'\n \n test_expect_success 'merge c0 with c1 (no-ff)' '\n \tgit reset --hard c0 &&\n+\tgit config branch.master.mergeoptions \"\" &&\n \ttest_tick &&\n \tgit merge --no-ff c1 &&\n \tverify_merge file result.1 &&\n@@ -427,6 +428,11 @@ test_expect_success 'merge c0 with c1 (no-ff)' '\n \n test_debug 'gitk --all'\n \n+test_expect_success 'combining --squash and --no-ff is refused' '\n+\ttest_must_fail git merge --squash --no-ff c1 &&\n+\ttest_must_fail git merge --no-ff --squash c1\n+'\n+\n test_expect_success 'merge c0 with c1 (ff overrides no-ff)' '\n \tgit reset --hard c0 &&\n \tgit config branch.master.mergeoptions \"--no-ff\" &&\n-- \n1.5.4.3\n\n"}]}