| Message ID | 329802382bfa24241c2333bd38284aa77e3eb9f0.1626536508.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Handle pull option precedence | expand |
Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > There were already code checking that --rebase was incompatible with > a merge of multiple heads. However, we were sometimes throwing warnings > about lack of specification of rebase vs. merge when given multiple > heads. Since rebasing is disallowed with multiple merge heads, that > seems like a poor warning to print; we should instead just assume > merging is wanted. Where is that explained in the documentation?
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > +test_expect_failure 'Multiple heads does not warn about fast forwarding' ' > + git reset --hard c1 && > + git pull . c2 c3 2>err && > + test_i18ngrep ! "Pulling without specifying how to reconcile" err > +' This does not look like "warning about fast-forwarding". But more importantly, are we sure we want to expect this outcome? We are at c1 and try to integrate with c2 and c3 at the same time, neither of which is a descendant of c1. We know that the only possible action is to create an octopus in this case, and that it is pretty much fundamental (i.e. it is not like "rebase" with further development will be able to handle this case). I however do not know if it is also obvious to total newbies who haven't even chosen between merge and rebase. I can see them complaining "why didn't I get asked to choose between rebase and merge" if we went ahead and created an octopus merge, especially the ones who would choose pull.rebase=yes once they learned Git a bit more. > +test_expect_success 'Cannot fast-forward with multiple heads' ' > + git reset --hard c0 && > + test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err && > + test_i18ngrep ! "Pulling without specifying how to reconcile" err && > + test_i18ngrep "Not possible to fast-forward, aborting" err > +' This one looks sensible to me. > +test_expect_success 'Cannot rebase with multiple heads' ' > + git reset --hard c0 && > + test_must_fail git -c pull.rebase=true pull . c1 c2 c3 2>err && > + test_i18ngrep ! "Pulling without specifying how to reconcile" err && > + test_i18ngrep "Cannot rebase onto multiple branches." err > +' This one, too. Thanks.
On Tue, Jul 20, 2021 at 4:11 PM Junio C Hamano <gitster@pobox.com> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > +test_expect_failure 'Multiple heads does not warn about fast forwarding' ' > > + git reset --hard c1 && > > + git pull . c2 c3 2>err && > > + test_i18ngrep ! "Pulling without specifying how to reconcile" err > > +' > > This does not look like "warning about fast-forwarding". > > But more importantly, are we sure we want to expect this outcome? > > We are at c1 and try to integrate with c2 and c3 at the same time, > neither of which is a descendant of c1. > > We know that the only possible action is to create an octopus in > this case, and that it is pretty much fundamental (i.e. it is not > like "rebase" with further development will be able to handle this > case). I however do not know if it is also obvious to total newbies > who haven't even chosen between merge and rebase. I can see them > complaining "why didn't I get asked to choose between rebase and > merge" if we went ahead and created an octopus merge, especially the > ones who would choose pull.rebase=yes once they learned Git a bit > more. That's a fair point; I'll modify the test accordingly (and update the description). > > +test_expect_success 'Cannot fast-forward with multiple heads' ' > > + git reset --hard c0 && > > + test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err && > > + test_i18ngrep ! "Pulling without specifying how to reconcile" err && > > + test_i18ngrep "Not possible to fast-forward, aborting" err > > +' > > This one looks sensible to me. > > > +test_expect_success 'Cannot rebase with multiple heads' ' > > + git reset --hard c0 && > > + test_must_fail git -c pull.rebase=true pull . c1 c2 c3 2>err && > > + test_i18ngrep ! "Pulling without specifying how to reconcile" err && > > + test_i18ngrep "Cannot rebase onto multiple branches." err > > +' > > This one, too. > > Thanks.
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh index b24c98cc1b6..40d8fb95113 100755 --- a/t/t7601-merge-pull-config.sh +++ b/t/t7601-merge-pull-config.sh @@ -297,6 +297,26 @@ test_expect_success 'pull.rebase=true takes precedence over --ff' ' # End of precedence rules +test_expect_failure 'Multiple heads does not warn about fast forwarding' ' + git reset --hard c1 && + git pull . c2 c3 2>err && + test_i18ngrep ! "Pulling without specifying how to reconcile" err +' + +test_expect_success 'Cannot fast-forward with multiple heads' ' + git reset --hard c0 && + test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err && + test_i18ngrep ! "Pulling without specifying how to reconcile" err && + test_i18ngrep "Not possible to fast-forward, aborting" err +' + +test_expect_success 'Cannot rebase with multiple heads' ' + git reset --hard c0 && + test_must_fail git -c pull.rebase=true pull . c1 c2 c3 2>err && + test_i18ngrep ! "Pulling without specifying how to reconcile" err && + test_i18ngrep "Cannot rebase onto multiple branches." err +' + test_expect_success 'merge c1 with c2' ' git reset --hard c1 && test -f c0.c &&