From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Alex Henrie" <alexhenrie24@gmail.com>,
"Son Luong Ngoc" <sluongng@gmail.com>,
"Matthias Baumgarten" <matthias.baumgarten@aixigo.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Elijah Newren" <newren@gmail.com>,
"Felipe Contreras" <felipe.contreras@gmail.com>,
"Elijah Newren" <newren@gmail.com>
Subject: [PATCH v3 0/8] Handle pull option precedence
Date: Thu, 22 Jul 2021 05:04:42 +0000 [thread overview]
Message-ID: <pull.1049.v3.git.git.1626930290.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1049.v2.git.git.1626831744.gitgitgadget@gmail.com>
Based on a recent list of rules for flag/option precedence for git-pull[1]
from Junio (particularly focusing on rebase vs. merge vs. fast-forward),
here's an attempt to implement and document it. Given multiple recent
surprises from users about some of these behaviors[2][3] and a coworker just
yesterday expressing some puzzlement with git-pull and rebase vs. merge, it
seems like a good time to address some of these issues.
Since the handling of conflicting options was holding up two of Alex's
patches[4][5], and his patches fix some of the tests, I also include those
two patches in my series, with a few small changes to the first (so I've
kept him as author) and more substantial changes to the second (so I've
given him an Initial-patch-by attribution).
Changes since v2:
* Remove some unnecessary changes in patch 4, pointed out by Junio.
Changes since v1:
* Rebased on latest master (resolved a simple conflict with
dd/test-stdout-count-lines)
* Patch 1: based on feedback from Junio, fixed some style issues, clarified
function names, added a few new tests, and took a stab at fixing up the
comments and test descriptions (but still unsure if I hit the mark on the
last point)
* Patch 2: changed the test expectations for one of the multiple head tests
as per Junio's suggestion, and made one of the other tests expect a more
specific error message
* Patches 4 & 5 were squashed and fixed: these now address a submodule bug
interaction with --ff-only
* Old patch 6 (now 5): added a code comment explaining a subtle point
* Old patch 8 (now 7): a few more documentation updates, especially making
--ff-only not sound merge-specific
* Old patch 9 (now 8): Updates for new test expectation from patch 2
Quick overview:
* Patches 1-2: new testcases (see the commit messages for the rules)
* Patch 3: Alex's recent patch (abort if --ff-only but can't do so)
* Patches 4-6: fix the precedence parts Alex didn't cover
* Patch 7: Alex's other patch, abort if rebase vs. merge not specified
* Patch 8: Compatibility of git-pull with merge-options.txt (think
rebasing)
* Patch 9: Fix multiple heads handling too
[1] https://lore.kernel.org/git/xmqqwnpqot4m.fsf@gitster.g/ [2]
https://lore.kernel.org/git/CAL3xRKdOyVWvcLXK7zoXtFPiHBjgL24zi5hhg+3yjowwSUPgmg@mail.gmail.com/
[3]
https://lore.kernel.org/git/c62933fb-96b2-99f5-7169-372f486f6e39@aixigo.com/
[4]
https://lore.kernel.org/git/20210711012604.947321-1-alexhenrie24@gmail.com/
[5]
https://lore.kernel.org/git/20210627000855.530985-1-alexhenrie24@gmail.com/
Alex Henrie (1):
pull: abort if --ff-only is given and fast-forwarding is impossible
Elijah Newren (7):
t7601: test interaction of merge/rebase/fast-forward flags and options
t7601: add tests of interactions with multiple merge heads and config
pull: since --ff-only overrides, handle it first
pull: make --rebase and --no-rebase override pull.ff=only
pull: abort by default when fast-forwarding is not possible
pull: update docs & code for option compatibility with rebasing
pull: fix handling of multiple heads
Documentation/git-merge.txt | 2 +
Documentation/git-pull.txt | 21 +--
Documentation/merge-options.txt | 40 ++++++
advice.c | 5 +
advice.h | 1 +
builtin/merge.c | 2 +-
builtin/pull.c | 55 +++++--
t/t4013-diff-various.sh | 2 +-
t/t5520-pull.sh | 20 +--
t/t5521-pull-options.sh | 4 +-
t/t5524-pull-msg.sh | 4 +-
t/t5553-set-upstream.sh | 14 +-
t/t5604-clone-reference.sh | 4 +-
t/t6402-merge-rename.sh | 18 +--
t/t6409-merge-subtree.sh | 6 +-
t/t6417-merge-ours-theirs.sh | 10 +-
t/t7601-merge-pull-config.sh | 244 +++++++++++++++++++++++++++++---
t/t7603-merge-reduce-heads.sh | 2 +-
18 files changed, 371 insertions(+), 83 deletions(-)
base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1049%2Fnewren%2Fhandle-pull-option-precedence-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1049/newren/handle-pull-option-precedence-v3
Pull-Request: https://github.com/git/git/pull/1049
Range-diff vs v2:
1: 17560927211 = 1: 17560927211 t7601: test interaction of merge/rebase/fast-forward flags and options
2: 66fe7f7f934 = 2: 66fe7f7f934 t7601: add tests of interactions with multiple merge heads and config
3: c45cd239666 = 3: c45cd239666 pull: abort if --ff-only is given and fast-forwarding is impossible
4: 1a821d3b1dd ! 4: 0682b2250f4 pull: since --ff-only overrides, handle it first
@@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
if (opt_rebase) {
int ret = 0;
-@@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
- submodule_touches_in_range(the_repository, &upstream, &curr_head))
- die(_("cannot rebase with locally recorded submodule modifications"));
-
-- if (can_ff) {
-- /* we can fast-forward this without invoking rebase */
-- opt_ff = "--ff-only";
-- ret = run_merge();
-- } else {
-- ret = run_rebase(&newbase, &upstream);
-- }
-+ ret = run_rebase(&newbase, &upstream);
-
- if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
- recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
-
- ## t/t5520-pull.sh ##
-@@ t/t5520-pull.sh: test_expect_success '--rebase (merge) fast forward' '
- # The above only validates the result. Did we actually bypass rebase?
- git reflog -1 >reflog.actual &&
- sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
-- echo "OBJID HEAD@{0}: pull --rebase . ff: Fast-forward" >reflog.expected &&
-+ echo "OBJID HEAD@{0}: pull --rebase . ff (finish): returning to refs/heads/to-rebase" >reflog.expected &&
- test_cmp reflog.expected reflog.fuzzy
- '
-
-@@ t/t5520-pull.sh: test_expect_success '--rebase (am) fast forward' '
-
- # The above only validates the result. Did we actually bypass rebase?
- git reflog -1 >reflog.actual &&
-- sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
-- echo "OBJID HEAD@{0}: pull --rebase . ff: Fast-forward" >reflog.expected &&
-+ sed -e "s/^[0-9a-f][0-9a-f]*/OBJID/" -e "s/[0-9a-f][0-9a-f]*$/OBJID/" reflog.actual >reflog.fuzzy &&
-+ echo "OBJID HEAD@{0}: rebase finished: refs/heads/to-rebase onto OBJID" >reflog.expected &&
- test_cmp reflog.expected reflog.fuzzy
- '
-
5: 9b116f3d284 = 5: b242d001132 pull: make --rebase and --no-rebase override pull.ff=only
6: f061f8b4e75 = 6: 7447c719bd7 pull: abort by default when fast-forwarding is not possible
7: 90d49e0fb78 = 7: 726082f2e79 pull: update docs & code for option compatibility with rebasing
8: f03b15b7eb0 = 8: 768dec71558 pull: fix handling of multiple heads
--
gitgitgadget
next prev parent reply other threads:[~2021-07-22 5:05 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-17 15:41 [PATCH 0/9] Handle pull option precedence Elijah Newren via GitGitGadget
2021-07-17 15:41 ` [PATCH 1/9] t7601: add relative precedence tests for merge and rebase flags/options Elijah Newren via GitGitGadget
2021-07-17 18:03 ` Felipe Contreras
2021-07-19 18:23 ` Junio C Hamano
2021-07-20 17:10 ` Elijah Newren
2021-07-20 18:22 ` Junio C Hamano
2021-07-20 20:29 ` Felipe Contreras
2021-07-17 15:41 ` [PATCH 2/9] t7601: add tests of interactions with multiple merge heads and config Elijah Newren via GitGitGadget
2021-07-17 18:04 ` Felipe Contreras
2021-07-20 23:11 ` Junio C Hamano
2021-07-21 0:45 ` Elijah Newren
2021-07-17 15:41 ` [PATCH 3/9] pull: abort if --ff-only is given and fast-forwarding is impossible Alex Henrie via GitGitGadget
2021-07-17 18:14 ` Felipe Contreras
2021-07-17 15:41 ` [PATCH 4/9] pull: since --ff-only overrides, handle it first Elijah Newren via GitGitGadget
2021-07-17 18:22 ` Felipe Contreras
2021-07-17 15:41 ` [PATCH 5/9] pull: ensure --rebase overrides ability to ff Elijah Newren via GitGitGadget
2021-07-17 18:25 ` Felipe Contreras
2021-07-20 23:35 ` Junio C Hamano
2021-07-21 0:14 ` Elijah Newren
2021-07-17 15:41 ` [PATCH 6/9] pull: make --rebase and --no-rebase override pull.ff=only Elijah Newren via GitGitGadget
2021-07-17 18:28 ` Felipe Contreras
2021-07-20 23:53 ` Junio C Hamano
2021-07-21 0:09 ` Felipe Contreras
2021-07-17 15:41 ` [PATCH 7/9] pull: abort by default when fast-forwarding is not possible Elijah Newren via GitGitGadget
2021-07-17 18:31 ` Felipe Contreras
2021-07-17 15:41 ` [PATCH 8/9] pull: update docs & code for option compatibility with rebasing Elijah Newren via GitGitGadget
2021-07-21 0:17 ` Junio C Hamano
2021-07-21 0:44 ` Elijah Newren
2021-07-21 1:25 ` Junio C Hamano
2021-07-17 15:41 ` [PATCH 9/9] pull: fix handling of multiple heads Elijah Newren via GitGitGadget
2021-07-17 18:39 ` [PATCH 0/9] Handle pull option precedence Felipe Contreras
2021-07-21 1:42 ` [PATCH v2 0/8] " Elijah Newren via GitGitGadget
2021-07-21 1:42 ` [PATCH v2 1/8] t7601: test interaction of merge/rebase/fast-forward flags and options Elijah Newren via GitGitGadget
2021-07-21 12:24 ` Felipe Contreras
2021-07-21 1:42 ` [PATCH v2 2/8] t7601: add tests of interactions with multiple merge heads and config Elijah Newren via GitGitGadget
2021-07-21 1:42 ` [PATCH v2 3/8] pull: abort if --ff-only is given and fast-forwarding is impossible Alex Henrie via GitGitGadget
2021-07-21 12:25 ` Felipe Contreras
2021-07-21 1:42 ` [PATCH v2 4/8] pull: since --ff-only overrides, handle it first Elijah Newren via GitGitGadget
2021-07-21 19:18 ` Matthias Baumgarten
2021-07-21 21:18 ` Felipe Contreras
2021-07-21 20:18 ` Junio C Hamano
2021-07-22 3:42 ` Elijah Newren
2021-07-21 1:42 ` [PATCH v2 5/8] pull: make --rebase and --no-rebase override pull.ff=only Elijah Newren via GitGitGadget
2021-07-21 12:26 ` Felipe Contreras
2021-07-21 1:42 ` [PATCH v2 6/8] pull: abort by default when fast-forwarding is not possible Elijah Newren via GitGitGadget
2021-07-21 12:27 ` Felipe Contreras
2021-07-21 1:42 ` [PATCH v2 7/8] pull: update docs & code for option compatibility with rebasing Elijah Newren via GitGitGadget
2021-07-21 1:42 ` [PATCH v2 8/8] pull: fix handling of multiple heads Elijah Newren via GitGitGadget
2021-07-21 12:15 ` [PATCH v2 0/8] Handle pull option precedence Felipe Contreras
2021-07-22 5:04 ` Elijah Newren via GitGitGadget [this message]
2021-07-22 5:04 ` [PATCH v3 1/8] t7601: test interaction of merge/rebase/fast-forward flags and options Elijah Newren via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 2/8] t7601: add tests of interactions with multiple merge heads and config Elijah Newren via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 3/8] pull: abort if --ff-only is given and fast-forwarding is impossible Alex Henrie via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 4/8] pull: since --ff-only overrides, handle it first Elijah Newren via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 5/8] pull: make --rebase and --no-rebase override pull.ff=only Elijah Newren via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 6/8] pull: abort by default when fast-forwarding is not possible Elijah Newren via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 7/8] pull: update docs & code for option compatibility with rebasing Elijah Newren via GitGitGadget
2021-07-22 5:04 ` [PATCH v3 8/8] pull: fix handling of multiple heads Elijah Newren via GitGitGadget
2021-07-22 7:09 ` [PATCH v3 0/8] Handle pull option precedence Felipe Contreras
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1049.v3.git.git.1626930290.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=alexhenrie24@gmail.com \
--cc=avarab@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=matthias.baumgarten@aixigo.com \
--cc=newren@gmail.com \
--cc=sluongng@gmail.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).