git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6 1/7] pull: rename pull.rebase to pull.mode

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
May 2, 2014, 21:12 UTC
Message-ID
<53640a26569a5_135215292ec43@nysa.notmuch>
In-Reply-To
<53640403.30600@bbn.com>
Richard Hansen wrote:
Show 39 quoted lines
> On 2014-05-01 20:00, Felipe Contreras wrote:
> > Also 'branch.<name>.rebase' to 'branch.<name>.pullmode'.
> > 
> > This way we can add more modes and the default can be something else,
> > namely it can be set to merge-ff-only, so eventually we can reject
> > non-fast-forward merges by default.
> > 
> > The old configurations still work, but get deprecated.
> 
> s/get/are/
> 
> > 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  Documentation/config.txt   | 39 ++++++++++++++++++++++-----------------
> >  Documentation/git-pull.txt |  2 +-
> >  branch.c                   |  4 ++--
> >  builtin/remote.c           | 14 ++++++++++++--
> >  git-pull.sh                | 31 +++++++++++++++++++++++++++++--
> >  t/t3200-branch.sh          | 40 ++++++++++++++++++++--------------------
> >  t/t5601-clone.sh           |  4 ++--
> >  7 files changed, 88 insertions(+), 46 deletions(-)
> > 
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index c26a7c8..c028aeb 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -708,7 +708,7 @@ branch.autosetupmerge::
> >  branch.autosetuprebase::
> >  	When a new branch is created with 'git branch' or 'git checkout'
> >  	that tracks another branch, this variable tells Git to set
> > -	up pull to rebase instead of merge (see "branch.<name>.rebase").
> > +	up pull to rebase instead of merge (see "branch.<name>.pullmode").
> >  	When `never`, rebase is never automatically set to true.
> >  	When `local`, rebase is set to true for tracked branches of
> >  	other local branches.
> 
> Should branch.autosetuprebase be replaced with a new
> branch.autosetupmode setting?

Maybe. But if so, I think that should be done in another series. Otherwise we'll never have a chance to change anything.

Show 15 quoted lines
> > @@ -764,15 +764,17 @@ branch.<name>.mergeoptions::
> >  	option values containing whitespace characters are currently not
> >  	supported.
> >  
> > -branch.<name>.rebase::
> > -	When true, rebase the branch <name> on top of the fetched branch,
> > -	instead of merging the default branch from the default remote when
> > -	"git pull" is run. See "pull.rebase" for doing this in a non
> > -	branch-specific manner.
> > +branch.<name>.pullmode::
> > +	When "git pull" is run, this determines if it would either merge or
> > +	rebase the fetched branch.
> 
> To me this sentence implies that 'rebase' would rebase the fetched
> branch onto HEAD, when it's actually the other way around.
Right.
This actually interesting mode of thinking:
a) git pull --rebase
We want to rebase HEAD onto @{upstream}.
b) git pull --merge
We want to merge HEAD into @{upstream}. (Why are we doing the opposite?)
c) git pull --rebase github john
We weant to rebase github/john onto HEAD. (We are doing the opposite?)
d) git pull --merge github john
We want to merge github/john into HEAD.
Show 6 quoted lines
> >                                  The possible values are 'merge',
> > +	'rebase', and 'rebase-preserve'.
> 
> While the name 'merge' is mostly self-explanatory, I think it needs
> further clarification:  Does 'merge' imply --no-ff?  Or --ff?  Or the
> value of merge.ff?

'pull.mode=merge' will do the same as `git merge`, I don't see where or how it can be explained more clearly.

> Which side will be the first parent?

The same as things currently are: the pulled branch into the current branch (current branch is first parent).

We should probably change this, but that's out of scope of this series, and hasn't been decided yet.

Show 5 quoted lines
> >                                        See "pull.mode" for doing this in a
> > +	non branch-specific manner.
> 
> I find this sentence to be a bit unclear and would prefer something
> like:  "Defaults to the value of pull.mode."
Hmm, might make sense.
Show 17 quoted lines
> >  +
> > -	When preserve, also pass `--preserve-merges` along to 'git rebase'
> > -	so that locally committed merge commits will not be flattened
> > -	by running 'git pull'.
> > +	When 'rebase-preserve', also pass `--preserve-merges` along to
> > +	'git rebase' so that locally committed merge commits will not be
> > +	flattened by running 'git pull'.
> > ++
> > +	It was named 'branch.<name>.rebase' but that is deprecated now.
> 
> To me this sentence implies that .rebase was simply renamed to .pullmode
> with no other changes.  I'd prefer something like this:
> 
> branch.<name>.rebase::
>     Deprecated in favor of branch.<name>.pullmode.
> 
> (Same goes for pull.rebase.)
Right.
Show 33 quoted lines
> >  +
> >  *NOTE*: this is a possibly dangerous operation; do *not* use
> >  it unless you understand the implications (see linkgit:git-rebase[1]
> > @@ -1881,15 +1883,18 @@ pretty.<name>::
> >  	Note that an alias with the same name as a built-in format
> >  	will be silently ignored.
> >  
> > -pull.rebase::
> > -	When true, rebase branches on top of the fetched branch, instead
> > -	of merging the default branch from the default remote when "git
> > -	pull" is run. See "branch.<name>.rebase" for setting this on a
> > -	per-branch basis.
> > +pull.mode::
> > +	When "git pull" is run, this determines if it would either merge or
> > +	rebase the fetched branch. The possible values are 'merge',
> > +	'rebase', and 'rebase-preserve'. See "branch.<name>.pullmode" for doing
> > +	this in a non branch-specific manner.
> > ++
> > +	When 'rebase-preserve', also pass `--preserve-merges` along to
> > +	'git rebase' so that locally committed merge commits will not be
> > +	flattened by running 'git pull'.
> > ++
> >  +
> > -	When preserve, also pass `--preserve-merges` along to 'git rebase'
> > -	so that locally committed merge commits will not be flattened
> > -	by running 'git pull'.
> 
> The default value should be documented.  Also, rather than copy+paste
> the description from branch.<name>.pullmode, I'd prefer a brief
> reference.  For example:
> 
> pull.mode::
>     See branch.<name>.pullmode.  Defaults to 'merge'.
I'd say pull.mode is the important one.
-- 
Felipe Contreras
Previous: Richard HansenNext: Richard Hansen
Message 5 of 18 in “Reject non-ff pulls by default”
  1. 0/7 Reject non-ff pulls by defaultFelipe Contreras, May 2, 2014
  2. 1/7 pull: rename pull.rebase to pull.modeFelipe Contreras, May 2, 2014
  3. W. Trevor KingMay 2, 2014
  4. Richard HansenMay 2, 2014
  5. Felipe ContrerasMay 2, 2014
  6. Richard HansenMay 2, 2014
  7. 2/7 pull: migrate all the tests to pull.modeFelipe Contreras, May 2, 2014
  8. 3/7 pull: refactor $rebase variable into $modeFelipe Contreras, May 2, 2014
  9. 4/7 pull: add --merge optionFelipe Contreras, May 2, 2014
  10. brian m. carlsonMay 2, 2014
  11. Felipe ContrerasMay 2, 2014
  12. brian m. carlsonMay 2, 2014
  13. Felipe ContrerasMay 2, 2014
  14. brian m. carlsonMay 2, 2014
  15. 5/7 pull: add merge-ff-only optionFelipe Contreras, May 2, 2014
  16. W. Trevor KingMay 2, 2014
  17. 6/7 pull: add warning on non-ff mergesFelipe Contreras, May 2, 2014
  18. 7/7 pull: only allow ff merges by defaultFelipe Contreras, May 2, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.