Message ID | 20210628031642.699156-1-felipe.contreras@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | pull: introduce --merge option | expand |
On Sun, Jun 27, 2021 at 9:16 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > The idea came after a comment from Linus Torvalds regarding what should > be the default mode of "git pull" and why [1]. > > [1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/ Oh wow, I didn't realize that Linus gave his opinion on the subject back in 2013, and he suggested doing nearly exactly what we are trying to do now. > ---no-rebase:: > - Override earlier --rebase. > +-m:: > +--merge:: > + Force a merge. > ++ > +Previously this was --no-rebase, but that usage has been deprecated. I don't think --no-rebase should be "deprecated", at least not yet. After all, the equivalent config option is still pull.rebase=false. Also, --merge doesn't necessarily force a merge because it does not imply --no-ff. I would prefer to list --no-rebase, -m, and --merge together on the same line and leave the description alone. But other than the documentation and the commit message, I like the idea here, and I think it will make Git a lot more user-friendly. -Alex
Alex Henrie wrote: > On Sun, Jun 27, 2021 at 9:16 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > The idea came after a comment from Linus Torvalds regarding what should > > be the default mode of "git pull" and why [1]. > > > > [1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/ > > Oh wow, I didn't realize that Linus gave his opinion on the subject > back in 2013, and he suggested doing nearly exactly what we are trying > to do now. Indeed. I incorporated his feedback--along with the feedback of many others--in my proposal. I have been re-reading the old threads to write a blog post, and the whole story starts from 2008. At this point I have probably read more than one thousand emails, and I'm still far from done. Actually now that I'm re-reading the draft, the first suggestion of --merge came from Thomas Rast in 2009 [1] (I forgot). Long story short: *everyone* (and I do mean everyone) is in favor making ff-only the default. The disagreements come from how we get there. > > ---no-rebase:: > > - Override earlier --rebase. > > +-m:: > > +--merge:: > > + Force a merge. > > ++ > > +Previously this was --no-rebase, but that usage has been deprecated. > > I don't think --no-rebase should be "deprecated", at least not yet. Deprecated means that we discourage people from using it. It doesn't mean it's unsupported. It's not obsolete. > After all, the equivalent config option is still pull.rebase=false. Yes, but the equivalent from the command line is: git pull --rebase=false And that's still properly documented: -r, --rebase[=false|true|merges|preserve|interactive] > Also, --merge doesn't necessarily force a merge because it does not > imply --no-ff. This is a bit of semantics that I have discussed before with Elijah, and somehow I managed to convince him [2] that fast-forward can be used as an adverb, and in fact it already is in plenty of the documentation. Basically this: git merge --ff-only Does a fast-forward merge. So it's a merge of a special kind. What kind? The fast-forward kind. > I would prefer to list --no-rebase, -m, and --merge > together on the same line and leave the description alone. I prefer to deprecate --no-rebase. Both --rebase=false and --merge are less cumbersome alternatives. But I'm not married to this. If other people want to keep --no-rebase at the same level as the other two options, I would not object to it. > But other than the documentation and the commit message, I like the > idea here, and I think it will make Git a lot more user-friendly. Good. And for the record I think if this patch is not merged, eventually somebody else will send something along these lines, since I'm not the first person to think of it, nor will I be the last. Cheers. [1] https://lore.kernel.org/git/200910201947.50423.trast@student.ethz.ch/ [2] https://lore.kernel.org/git/CABPp-BESMs1tuVoLFMy-BahSChFz7oANqTaeJShFa_zDbEnvBA@mail.gmail.com/
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 5c3fb67c01..d23128fa72 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -131,8 +131,11 @@ It rewrites history, which does not bode well when you published that history already. Do *not* use this option unless you have read linkgit:git-rebase[1] carefully. ---no-rebase:: - Override earlier --rebase. +-m:: +--merge:: + Force a merge. ++ +Previously this was --no-rebase, but that usage has been deprecated. Options related to fetching ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/builtin/pull.c b/builtin/pull.c index e8927fc2ff..186adbff71 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -129,6 +129,8 @@ static struct option pull_options[] = { "(false|true|merges|preserve|interactive)", N_("incorporate changes by rebasing rather than merging"), PARSE_OPT_OPTARG, parse_opt_rebase), + OPT_SET_INT('m', "merge", &opt_rebase, + N_("incorporate changes by merging"), REBASE_FALSE), OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL, N_("do not show a diffstat at the end of the merge"), PARSE_OPT_NOARG | PARSE_OPT_NONEG), @@ -936,7 +938,7 @@ static void show_advice_pull_non_ff(void) " git config pull.ff only # fast-forward only\n" "\n" "You can replace \"git config\" with \"git config --global\" to set a default\n" - "preference for all repositories. You can also pass --rebase, --no-rebase,\n" + "preference for all repositories. You can also pass --rebase, --merge,\n" "or --ff-only on the command line to override the configured default per\n" "invocation.\n")); } diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh index 52e8ccc933..6d03e0b9fe 100755 --- a/t/t7601-merge-pull-config.sh +++ b/t/t7601-merge-pull-config.sh @@ -60,9 +60,9 @@ test_expect_success 'pull.rebase not set and --rebase given' ' test_i18ngrep ! "Pulling without specifying how to reconcile" err ' -test_expect_success 'pull.rebase not set and --no-rebase given' ' +test_expect_success 'pull.rebase not set and --merge given' ' git reset --hard c0 && - git pull --no-rebase . c1 2>err && + git pull --merge . c1 2>err && test_i18ngrep ! "Pulling without specifying how to reconcile" err ' @@ -119,9 +119,9 @@ test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' test_i18ngrep ! "Pulling without specifying how to reconcile" err ' -test_expect_success 'pull.rebase not set and --no-rebase given (not-fast-forward)' ' +test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' ' git reset --hard c2 && - git pull --no-rebase . c1 2>err && + git pull --merge . c1 2>err && test_i18ngrep ! "Pulling without specifying how to reconcile" err '
Previously --no-rebase (which still works for backwards compatibility). Now we can update the default warning, and the git-pull(1) man page to use --merge instead of the non-intuitive --no-rebase. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- I've sent many versions of this patch over the years (since 2013) in many different series and nobody has objected to it (to the best of my recollection). The idea came after a comment from Linus Torvalds regarding what should be the default mode of "git pull" and why [1]. It conflicts with some of my previous patch series, but they aren't even in "seen" yet, and the ones that are resolve easily. [1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/ Documentation/git-pull.txt | 7 +++++-- builtin/pull.c | 4 +++- t/t7601-merge-pull-config.sh | 8 ++++---- 3 files changed, 12 insertions(+), 7 deletions(-)