git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [RFC PATCH 00/35] git update: fix broken git pull
@ 2021-07-05 12:31 Felipe Contreras
  2021-07-05 12:31 ` [RFC PATCH 01/35] merge: improve fatal fast-forward message Felipe Contreras
                   ` (34 more replies)
  0 siblings, 35 replies; 53+ messages in thread
From: Felipe Contreras @ 2021-07-05 12:31 UTC (permalink / raw)
  To: git; +Cc: Alex Henrie, Richard Hansen, Junio C Hamano, Felipe Contreras

Throughout the years many big threads have been started [1] [2] [3] [4]
[5] [6] because `git pull` is broken (for everyone that isn't a
maintainer), and nothing has been fixed.

The two main problems are:

  1. The order of the parents is the opposite of what it should be
  2. By default it should fail unless it's a fast-forward

However, every attempt to fix these issues gets entangled in an
inescapable web of interrelated problems.

That's because `git pull` is in fact *two* commands pretending to be
one.

I have spent several days re-reading old threads looking for anything
related to `git pull` and fast-forward throughout the entire history of
the git mailing list, going as far back as 2008.

This process has been illuminating.

The amount of people that have pondered about, and even suggested, a
`git update` command is striking:

 * Linus Torvalds: [7]
 * Junio C. Hamano: [8]
 * Richard Hansen: [9]
 * John Keeping: [10]

And of course me [11], who not only suggested it, but implemented it
as well [12].

It's not just the name, but the behavior too:

  `git fetch` + `git merge --ff-only`

This way `git pull` can be used by maintainers to integrate pull
requests, and `git update` for everyone else (normal developers) to
update their local branch to its upstream.

Plus it has a --merge mode that creates a merge commit, but with the
parents in the right order (master to upstream, not upstream to master).

Both problems are fixed by this approach.


Also, a thorny issue since a long time ago has been how to properly
advise users what to do in case a fast-forward is not possible.

I propose a new command: `git fast-forward`. When a fast-forward is not
possible `git fast-forward` fails, and shows an advice (that can be
easily turned off).

Therefore in fact `git update` is really `git fetch` +
`git fast-forward`, and it's the latter that throws the advice. The
advice also suggests `git help fast-forward` which has an explanation
about what a fast-forward is, what are the options to synchronize
with the remote branch (merge and rebase), and what do they look like.

In addition there's a ton of fixes for `git pull` which have been sent
before, including the `pull.mode` configuration, the `--merge` option,
the per-branch `pullMode` configuration, `pull.mode=fast-forward`, and
an improved advice message to prepare users for a future default change:

  The pull was not a fast-forward, in the future you will have to choose
  between a merge or a rebase.

  To quell this message you have two main options:

  1. Adopt the new behavior:

    git config --global pull.mode fast-forward

  2. Maintain the current behavior:

    git config --global pull.mode merge

  For now we will fall back to the traditional behavior: merge.
  For more information check "git help fast-forward".

It doesn't just improve the situation for normal developers, but it also
opens the possibility to fix a wart in `git pull --rebase`:

Right now:

 git pull --merge github john (merge "github/john" into "master)
 git pull --rebase github john (rebase "master" onto "github/john")

Since now `git update --rebase` can be used to rebase the current branch
onto its upstream, that leaves `git pull --rebase` open to rebase
github/john onto master, not the other way around like it currently is.


Junio has suggested there's no consensus over fast-forward by default,
but that's not true. Here is a non-exhaustive list of people that have
agreed the default should be fast-forward only.

 * Linus Torvalds
 * Junio C Hamano
 * Jeff King
 * Jonathan Nieder
 * Richard Hansen
 * Philip Oakley
 * Elijah Newren
 * John Keeping
 * Ramkumar Ramachandra
 * Alex Henrie
 * W. Trevor King
 * Greg Troxel
 * Peter Kjellerstedt
 * Konstantin Tokarev
 * Robert Dailey
 * Vít Ondruch
 * Raymond E. Pasco
 * Jacob Keller
 * Felipe Contreras

There's only one person against the change: Matthieu Moy. His argument
back then [13] was: why ask users to merge or rebase if newcomers will
not pick a rebase? To that my response is: for the same reason we ask
them to do `git commit --all`: so that they eventually learn about the
staging area. Just like they can do `git commit --all`, they can do
`git pull --merge`. They can figure out what that means later.

The people in favor of reordering the parents:

 * Andreas Krey
 * John Szakmeister
 * Jeremy Rosen
 * John Keeping

And the people in favor of my old `git update`:

 * Damien Robert
 * Ping Yin
 * Philippe Vaucher


This patch series doesn't implement all of the features my old
`git update` had, but it implements most of them, most
importantly--unlike `git pull`--it's not broken.

Right now only `git update` without arguments works, and only if the
upstream branch is configured. I have a pretty good idea of what the it
should do with different arguments (as I had already implemented that),
but for now that's open to discussion.

If you want to give it a try:

https://github.com/felipec/git/tree/fc/update

[1] https://lore.kernel.org/git/200910201947.50423.trast@student.ethz.ch/
[2] https://lore.kernel.org/git/20130522115042.GA20649@inner.h.apk.li/
[3] https://lore.kernel.org/git/1377988690-23460-1-git-send-email-felipe.contreras@gmail.com/
[4] https://lore.kernel.org/git/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com/
[5] https://lore.kernel.org/git/5363BB9F.40102@xiplink.com/
[6] https://lore.kernel.org/git/20201204061623.1170745-1-felipe.contreras@gmail.com/
[7] https://lore.kernel.org/git/CA+55aFzxsNxgKD1uGZQCiib+=+wCMSa0=B+Ye3Zi-u6kpz8Vrg@mail.gmail.com/
[8] https://lore.kernel.org/git/xmqqppjyhnom.fsf@gitster.dls.corp.google.com/
[9] https://lore.kernel.org/git/5228A14B.3000804@bbn.com/
[10] https://lore.kernel.org/git/20130628174252.GF2232@serenity.lan/
[11] https://lore.kernel.org/git/5366db742d494_18f9e4b308aa@nysa.notmuch/
[12] https://github.com/felipec/git/commit/61e05c9798b3c74bab8b2d47ede4c12e8e305345
[13] https://lore.kernel.org/git/vpqbo3za8r9.fsf@anie.imag.fr/

Felipe Contreras (35):
  merge: improve fatal fast-forward message
  merge: split cmd_merge()
  fast-forward: add new builtin
  doc: fast-forward: explain what it is
  fast-forward: add advice for novices
  fast-forward: make the advise configurable
  fast-forward: add help about merge vs. rebase
  update: new built-in
  update: add options and usage skeleton
  update: add --ff option
  update: add --merge mode
  commit: support for multiple MERGE_MODE
  merge: add --reverse-parents option
  update: reverse the order of parents
  update: fake a reverse order of parents in message
  update: add --rebase mode
  update: add mode configuation
  update: add per-branch mode configuration
  pull: cleanup autostash check
  pull: trivial cleanup
  pull: trivial whitespace style fix
  pull: introduce --merge option
  rebase: add REBASE_DEFAULT
  pull: move configuration fetches
  pull: show warning with --ff options
  pull: add pull.mode
  pull: add per-branch mode configuration
  pull: add pull.mode=fast-forward
  pull: reorganize mode conditionals
  pull: add diverging advice on fast-forward mode
  pull: improve --rebase and pull.rebase interaction
  pull: improve default warning
  pull: advice of future changes
  FUTURE: pull: enable ff-only mode by default
  !fixup FUTURE: pull: enable ff-only mode by default

 .gitignore                             |   2 +
 Documentation/config.txt               |   4 +
 Documentation/config/advice.txt        |   2 +
 Documentation/config/branch.txt        |  10 ++
 Documentation/config/pull.txt          |   6 +
 Documentation/config/update.txt        |   5 +
 Documentation/git-fast-forward.txt     | 104 +++++++++++++++++
 Documentation/git-pull.txt             |  10 +-
 Documentation/git-update.txt           |  47 ++++++++
 Documentation/merge-options.txt        |   4 +
 Makefile                               |   2 +
 advice.c                               |  15 +++
 advice.h                               |   2 +
 builtin.h                              |   2 +
 builtin/commit.c                       |   7 +-
 builtin/merge.c                        |  59 +++++++---
 builtin/pull.c                         | 156 +++++++++++++++++--------
 builtin/update.c                       | 153 ++++++++++++++++++++++++
 contrib/completion/git-completion.bash |  22 ++++
 fmt-merge-msg.c                        |  21 +++-
 fmt-merge-msg.h                        |   3 +-
 git.c                                  |   2 +
 rebase.c                               |  12 ++
 rebase.h                               |  13 ++-
 t/t4013-diff-various.sh                |   2 +-
 t/t5520-pull.sh                        | 123 +++++++++++++++++--
 t/t5521-pull-options.sh                |   4 +-
 t/t5524-pull-msg.sh                    |   4 +-
 t/t5553-set-upstream.sh                |  14 +--
 t/t5563-update.sh                      |  87 ++++++++++++++
 t/t5604-clone-reference.sh             |   4 +-
 t/t6402-merge-rename.sh                |  16 +--
 t/t6409-merge-subtree.sh               |   6 +-
 t/t6417-merge-ours-theirs.sh           |  10 +-
 t/t7600-merge.sh                       |  47 ++++++++
 t/t7601-merge-pull-config.sh           | 116 ------------------
 t/t7603-merge-reduce-heads.sh          |   2 +-
 37 files changed, 870 insertions(+), 228 deletions(-)
 create mode 100644 Documentation/config/update.txt
 create mode 100644 Documentation/git-fast-forward.txt
 create mode 100644 Documentation/git-update.txt
 create mode 100644 builtin/update.c
 create mode 100755 t/t5563-update.sh

-- 
2.32.0.36.g70aac2b1aa


^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2021-07-06 22:26 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-05 12:31 [RFC PATCH 00/35] git update: fix broken git pull Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 01/35] merge: improve fatal fast-forward message Felipe Contreras
2021-07-06 20:07   ` Ævar Arnfjörð Bjarmason
2021-07-06 20:39     ` Felipe Contreras
2021-07-06 20:48       ` Randall S. Becker
2021-07-06 20:56         ` Junio C Hamano
2021-07-06 21:15           ` Felipe Contreras
2021-07-06 21:31           ` Randall S. Becker
2021-07-06 21:54             ` Junio C Hamano
2021-07-06 22:26               ` Randall S. Becker
2021-07-06 21:11         ` Felipe Contreras
2021-07-06 21:27           ` Randall S. Becker
2021-07-06 22:14             ` Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 02/35] merge: split cmd_merge() Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 03/35] fast-forward: add new builtin Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 04/35] doc: fast-forward: explain what it is Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 05/35] fast-forward: add advice for novices Felipe Contreras
2021-07-06 20:09   ` Ævar Arnfjörð Bjarmason
2021-07-06 20:42     ` Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 06/35] fast-forward: make the advise configurable Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 07/35] fast-forward: add help about merge vs. rebase Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 08/35] update: new built-in Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 09/35] update: add options and usage skeleton Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 10/35] update: add --ff option Felipe Contreras
2021-07-06 20:12   ` Ævar Arnfjörð Bjarmason
2021-07-06 20:46     ` Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 11/35] update: add --merge mode Felipe Contreras
2021-07-06 20:13   ` Ævar Arnfjörð Bjarmason
2021-07-06 20:51     ` Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 12/35] commit: support for multiple MERGE_MODE Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 13/35] merge: add --reverse-parents option Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 14/35] update: reverse the order of parents Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 15/35] update: fake a reverse order of parents in message Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 16/35] update: add --rebase mode Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 17/35] update: add mode configuation Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 18/35] update: add per-branch mode configuration Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 19/35] pull: cleanup autostash check Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 20/35] pull: trivial cleanup Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 21/35] pull: trivial whitespace style fix Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 22/35] pull: introduce --merge option Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 23/35] rebase: add REBASE_DEFAULT Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 24/35] pull: move configuration fetches Felipe Contreras
2021-07-05 12:31 ` [RFC PATCH 25/35] pull: show warning with --ff options Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 26/35] pull: add pull.mode Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 27/35] pull: add per-branch mode configuration Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 28/35] pull: add pull.mode=fast-forward Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 29/35] pull: reorganize mode conditionals Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 30/35] pull: add diverging advice on fast-forward mode Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 31/35] pull: improve --rebase and pull.rebase interaction Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 32/35] pull: improve default warning Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 33/35] pull: advice of future changes Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 34/35] FUTURE: pull: enable ff-only mode by default Felipe Contreras
2021-07-05 12:32 ` [RFC PATCH 35/35] !fixup " Felipe Contreras

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).