| Message ID | d1952f014f20d2770c74a311df27f956c8b95e21.1626536508.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Handle pull option precedence | expand |
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Elijah Newren <newren@gmail.com> > > git-pull.txt includes merge-options.txt, which is written assuming > merges will happen. git-pull has allowed rebases for many years; update > the documentation to reflect that. Yes, as I might have said elsewhere recently, "pull --rebase" has been afterthought and the way how it interacts with many niche "features" of "git pull" has never been designed but started with whatever the code happened to do and then fixed/tweaked as people found it suboptimal. Hopefully with this series we are covering these interactions and whipping them into a much better shape with coherent semantics. > diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt > index cad3f6bfcee..6e6e95a7595 100644 > --- a/Documentation/git-pull.txt > +++ b/Documentation/git-pull.txt > @@ -125,6 +125,8 @@ When false, merge the current branch into the upstream branch. > + > When `interactive`, enable the interactive mode of rebase. > + > +Note that `--ff-only` takes precedence over any `--rebase` flag. > ++ "`ff-only` overrides any `--rebase` flag"? > @@ -58,6 +61,10 @@ could instead be resolved as a fast-forward. > + > With `--ff-only`, resolve the merge as a fast-forward when possible. > When not possible, refuse to merge and exit with a non-zero status. This part may want to become conditional with git-pull[] to use a verb different than "merge"? > +ifdef::git-pull[] > ++ > +Note that `--no-ff` and `--ff` are ignored when rebasing is requested. > +endif::git-pull[] OK.
On Tue, Jul 20, 2021 at 5:17 PM Junio C Hamano <gitster@pobox.com> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: Elijah Newren <newren@gmail.com> > > > > git-pull.txt includes merge-options.txt, which is written assuming > > merges will happen. git-pull has allowed rebases for many years; update > > the documentation to reflect that. > > Yes, as I might have said elsewhere recently, "pull --rebase" has > been afterthought and the way how it interacts with many niche > "features" of "git pull" has never been designed but started with > whatever the code happened to do and then fixed/tweaked as people > found it suboptimal. Hopefully with this series we are covering > these interactions and whipping them into a much better shape with > coherent semantics. > > > diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt > > index cad3f6bfcee..6e6e95a7595 100644 > > --- a/Documentation/git-pull.txt > > +++ b/Documentation/git-pull.txt > > @@ -125,6 +125,8 @@ When false, merge the current branch into the upstream branch. > > + > > When `interactive`, enable the interactive mode of rebase. > > + > > +Note that `--ff-only` takes precedence over any `--rebase` flag. > > ++ > > "`ff-only` overrides any `--rebase` flag"? Yeah, ignore that, I had already ripped it out after your comments on patch 1 yesterday. I've also got more changes to the pull documentation to try to do more of what your sentence above says, "covering these interactions and whipping them into a much better shape." > > @@ -58,6 +61,10 @@ could instead be resolved as a fast-forward. > > + > > With `--ff-only`, resolve the merge as a fast-forward when possible. > > When not possible, refuse to merge and exit with a non-zero status. > > This part may want to become conditional with git-pull[] to use a > verb different than "merge"? Yep, that's one of the other changes I'm adding and will send out soon. > > +ifdef::git-pull[] > > ++ > > +Note that `--no-ff` and `--ff` are ignored when rebasing is requested. > > +endif::git-pull[] > > OK.
Elijah Newren <newren@gmail.com> writes: >> > +Note that `--ff-only` takes precedence over any `--rebase` flag. >> > ++ >> >> "`ff-only` overrides any `--rebase` flag"? > > Yeah, ignore that, I had already ripped it out after your comments on > patch 1 yesterday. I've also got more changes to the pull > documentation to try to do more of what your sentence above says, > "covering these interactions and whipping them into a much better > shape." OK. After thinking about it further, "overrides" is not the word, either, I guess. It is more like that there are "merge", "rebase" and "fast-forward-only" for the user to choose from, with "merge" having two sub-variants (i.e. what happens when we do not have any development of our own) and "rebase" with a few sub-variants (i.e. interactive? shape-preserving?). In any case, when the user says "I'll take nothing but fast-forward update to their history", it is a bug if we did not fail when their history is not a descendant of ours, and we have already fixed it in an earlier part of the series. This would be a good place to clarify what the correct behaviour ought to be, if the existing documentation is not already. Thanks.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index cad3f6bfcee..6e6e95a7595 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -125,6 +125,8 @@ When false, merge the current branch into the upstream branch. + When `interactive`, enable the interactive mode of rebase. + +Note that `--ff-only` takes precedence over any `--rebase` flag. ++ See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in linkgit:git-config[1] if you want to make `git pull` always use `--rebase` instead of merging. diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index eb0aabd396f..3b70abf3c87 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -2,6 +2,9 @@ --no-commit:: Perform the merge and commit the result. This option can be used to override --no-commit. +ifdef::git-pull[] + Only useful when merging. +endif::git-pull[] + With --no-commit perform the merge and stop just before creating a merge commit, to give the user a chance to inspect and further @@ -58,6 +61,10 @@ could instead be resolved as a fast-forward. + With `--ff-only`, resolve the merge as a fast-forward when possible. When not possible, refuse to merge and exit with a non-zero status. +ifdef::git-pull[] ++ +Note that `--no-ff` and `--ff` are ignored when rebasing is requested. +endif::git-pull[] -S[<keyid>]:: --gpg-sign[=<keyid>]:: @@ -73,6 +80,9 @@ When not possible, refuse to merge and exit with a non-zero status. In addition to branch names, populate the log message with one-line descriptions from at most <n> actual commits that are being merged. See also linkgit:git-fmt-merge-msg[1]. +ifdef::git-pull[] + Only useful when merging. +endif::git-pull[] + With --no-log do not list one-line descriptions from the actual commits being merged. @@ -102,10 +112,17 @@ With --no-squash perform the merge and commit the result. This option can be used to override --squash. + With --squash, --commit is not allowed, and will fail. +ifdef::git-pull[] ++ +Only useful when merging. +endif::git-pull[] --no-verify:: This option bypasses the pre-merge and commit-msg hooks. See also linkgit:githooks[5]. +ifdef::git-pull[] + Only useful when merging. +endif::git-pull[] -s <strategy>:: --strategy=<strategy>:: @@ -127,6 +144,10 @@ With --squash, --commit is not allowed, and will fail. default trust model, this means the signing key has been signed by a trusted key. If the tip commit of the side branch is not signed with a valid key, the merge is aborted. +ifdef::git-pull[] ++ +Only useful when merging. +endif::git-pull[] --summary:: --no-summary:: @@ -166,3 +187,7 @@ endif::git-pull[] projects that started their lives independently. As that is a very rare occasion, no configuration variable to enable this by default exists and will not be added. +ifdef::git-pull[] ++ +Only useful when merging. +endif::git-pull[] diff --git a/builtin/pull.c b/builtin/pull.c index 2d7f2d765ab..3a61b92f328 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -893,6 +893,8 @@ static int run_rebase(const struct object_id *newbase, strvec_pushv(&args, opt_strategy_opts.v); if (opt_gpg_sign) strvec_push(&args, opt_gpg_sign); + if (opt_signoff) + strvec_push(&args, opt_signoff); if (opt_autostash == 0) strvec_push(&args, "--no-autostash"); else if (opt_autostash == 1)