[PATCH] pull: introduce a pull.rebase option to enable --rebase

[PATCH] pull: introduce a pull.rebase option to enable --rebase

[Ævar Arnfjörð Bjarmason]

Currently we either need to set branch.<name>.rebase for existing
branches if we'd like "git pull" to mean "git pull --rebase", or have
the forethought of setting "branch.autosetuprebase" before we create
the branch.

But there's no way to globally configure "git pull" to mean "git pull
--rebase" for existing branches, introduce a "pull.rebase" option to
do that.

This option will be considered at a lower priority than
branch.<name>.rebase, i.e. we could set pull.rebase=true and
branch.<name>.rebase=false and the latter configuration option would
win.

Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
Reviewed-by: Eric Herman <eric@freesa.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt   |   14 +++++++++++++-
 Documentation/git-pull.txt |    2 +-
 git-pull.sh                |    4 ++++
 t/t5520-pull.sh            |   24 ++++++++++++++++++++++--
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..b2d7d92 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -677,7 +677,9 @@ branch.<name>.mergeoptions::
 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.
+	"git pull" is run. See "pull.rebase" for doing this in a non
+	branch-specific manner.
+
 	*NOTE*: this is a possibly dangerous operation; do *not* use
 	it unless you understand the implications (see linkgit:git-rebase[1]
 	for details).
@@ -1590,6 +1592,16 @@ 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.
+
+	*NOTE*: this is a possibly dangerous operation; do *not* use
+	it unless you understand the implications (see linkgit:git-rebase[1]
+	for details).
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index e1da468..0f18ec8 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -108,7 +108,7 @@ include::merge-options.txt[]
 	fetched, the rebase uses that information to avoid rebasing
 	non-local changes.
 +
-See `branch.<name>.rebase` and `branch.autosetuprebase` in
+See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `{litdd}rebase` instead of merging.
 +
diff --git a/git-pull.sh b/git-pull.sh
index 9868a0b..24b6b7c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -44,6 +44,10 @@ merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
+if test -z "$rebase"
+then 
+	rebase=$(git config --bool pull.rebase)
+fi
 dry_run=
 while :
 do
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 0e5eb67..ecc4fdc 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -94,16 +94,36 @@ test_expect_success '--rebase' '
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
+test_expect_success 'pull.rebase' '
+	git reset --hard before-rebase &&
+	git config --bool pull.rebase true &&
+	test_when_finished "git config --unset pull.rebase" &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
+	test new = $(git show HEAD:file2)
+'
 
 test_expect_success 'branch.to-rebase.rebase' '
 	git reset --hard before-rebase &&
-	git config branch.to-rebase.rebase 1 &&
+	git config --bool branch.to-rebase.rebase true &&
+	test_when_finished "git config --unset branch.to-rebase.rebase" &&
 	git pull . copy &&
-	git config branch.to-rebase.rebase 0 &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
 
+test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
+	git reset --hard before-rebase &&
+	git config --bool pull.rebase true &&
+	test_when_finished "git config --unset pull.rebase" &&
+	git config --bool branch.to-rebase.rebase false &&
+	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	git pull . copy &&
+	git config --unset branch.to-rebase.rebase &&
+	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
+	test new = $(git show HEAD:file2)
+'
+
 test_expect_success '--rebase with rebased upstream' '
 
 	git remote add -f me . &&
-- 
1.7.7

Re: [PATCH] pull: introduce a pull.rebase option to enable --rebase

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 674 bytes --]

Hi,

On Sat, 5 Nov 2011, Ævar Arnfjörð Bjarmason wrote:

> Currently we either need to set branch.<name>.rebase for existing 
> branches if we'd like "git pull" to mean "git pull --rebase", or have 
> the forethought of setting "branch.autosetuprebase" before we create the 
> branch.
> 
> But there's no way to globally configure "git pull" to mean "git pull 
> --rebase" for existing branches, introduce a "pull.rebase" option to do 
> that.
> 
> This option will be considered at a lower priority than 
> branch.<name>.rebase, i.e. we could set pull.rebase=true and 
> branch.<name>.rebase=false and the latter configuration option would 
> win.

Nice.

Ciao,
Johannes

Re: [PATCH] pull: introduce a pull.rebase option to enable --rebase

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> This option will be considered at a lower priority than
> branch.<name>.rebase, i.e. we could set pull.rebase=true and
> branch.<name>.rebase=false and the latter configuration option would
> win.
>
> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
> Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
> Reviewed-by: Eric Herman <eric@freesa.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

I see many reviewed-by lines, but what kind of review did this patch have,
exactly? It seem to break its own test (branch.to-rebase.rebase should
override pull.rebase).

I think I've queued most (if not all) of the patches in flight except for
Ram's sequencer reroll to 'pu' and pu^ passes the test but the tip of pu
does not due to this topic.

Thanks for other topics from the Amsterdam together, by the way. I did not
comment on them individually but they looked mostly reasonable from a
quick glance.

Re: [PATCH] pull: introduce a pull.rebase option to enable --rebase

[Sverre Rabbelier]

Heya,

On Sun, Nov 6, 2011 at 08:47, Junio C Hamano <gitster@pobox.com> wrote:
> I see many reviewed-by lines, but what kind of review did this patch have,
> exactly? It seem to break its own test (branch.to-rebase.rebase should
> override pull.rebase).

We looked at the patch the same way one would on list, and I assumed
that the tests passed. Ævar, did you sent a wrong version by accident
perhaps?

> Thanks for other topics from the Amsterdam together, by the way. I did not
> comment on them individually but they looked mostly reasonable from a
> quick glance.

Thanks.

-- 
Cheers,

Sverre Rabbelier

[PATCH v2] pull: introduce a pull.rebase option to enable --rebase

[Ævar Arnfjörð Bjarmason]

Currently we either need to set branch.<name>.rebase for existing
branches if we'd like "git pull" to mean "git pull --rebase", or have
the forethought of setting "branch.autosetuprebase" before we create
the branch.

But there's no way to globally configure "git pull" to mean "git pull
--rebase" for existing branches, introduce a "pull.rebase" option to
do that.

This option will be considered at a lower priority than
branch.<name>.rebase, i.e. we could set pull.rebase=true and
branch.<name>.rebase=false and the latter configuration option would
win.

Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
Reviewed-by: Eric Herman <eric@freesa.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Sun, Nov 6, 2011 at 08:47, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> This option will be considered at a lower priority than
>> branch.<name>.rebase, i.e. we could set pull.rebase=true and
>> branch.<name>.rebase=false and the latter configuration option would
>> win.
>>
>> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
>> Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
>> Reviewed-by: Eric Herman <eric@freesa.org>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> I see many reviewed-by lines, but what kind of review did this patch have,
> exactly? It seem to break its own test (branch.to-rebase.rebase should
> override pull.rebase).

We all stood behind my laptop while I explained what it did and
why. Sverre pointed out that I should use the test_when_finished()
function for unsetting the config variables, Eric and Fernando looked
it over as well.

> I think I've queued most (if not all) of the patches in flight except for
> Ram's sequencer reroll to 'pu' and pu^ passes the test but the tip of pu
> does not due to this topic.

Due to a trivial error of mine. I sent out the wrong patch as Sverre
thought, the problem was that I was trying to --unset a config
variable twice, that's now fixed and the tests pass with this patch.

 Documentation/config.txt   |   14 +++++++++++++-
 Documentation/git-pull.txt |    2 +-
 git-pull.sh                |    4 ++++
 t/t5520-pull.sh            |   23 +++++++++++++++++++++--
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..b2d7d92 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -677,7 +677,9 @@ branch.<name>.mergeoptions::
 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.
+	"git pull" is run. See "pull.rebase" for doing this in a non
+	branch-specific manner.
+
 	*NOTE*: this is a possibly dangerous operation; do *not* use
 	it unless you understand the implications (see linkgit:git-rebase[1]
 	for details).
@@ -1590,6 +1592,16 @@ 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.
+
+	*NOTE*: this is a possibly dangerous operation; do *not* use
+	it unless you understand the implications (see linkgit:git-rebase[1]
+	for details).
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index e1da468..0f18ec8 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -108,7 +108,7 @@ include::merge-options.txt[]
 	fetched, the rebase uses that information to avoid rebasing
 	non-local changes.
 +
-See `branch.<name>.rebase` and `branch.autosetuprebase` in
+See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `{litdd}rebase` instead of merging.
 +
diff --git a/git-pull.sh b/git-pull.sh
index 9868a0b..24b6b7c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -44,6 +44,10 @@ merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
+if test -z "$rebase"
+then 
+	rebase=$(git config --bool pull.rebase)
+fi
 dry_run=
 while :
 do
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 0e5eb67..35304b4 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -94,16 +94,35 @@ test_expect_success '--rebase' '
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
+test_expect_success 'pull.rebase' '
+	git reset --hard before-rebase &&
+	git config --bool pull.rebase true &&
+	test_when_finished "git config --unset pull.rebase" &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
+	test new = $(git show HEAD:file2)
+'
 
 test_expect_success 'branch.to-rebase.rebase' '
 	git reset --hard before-rebase &&
-	git config branch.to-rebase.rebase 1 &&
+	git config --bool branch.to-rebase.rebase true &&
+	test_when_finished "git config --unset branch.to-rebase.rebase" &&
 	git pull . copy &&
-	git config branch.to-rebase.rebase 0 &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
 
+test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
+	git reset --hard before-rebase &&
+	git config --bool pull.rebase true &&
+	test_when_finished "git config --unset pull.rebase" &&
+	git config --bool branch.to-rebase.rebase false &&
+	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
+	test new = $(git show HEAD:file2)
+'
+
 test_expect_success '--rebase with rebased upstream' '
 
 	git remote add -f me . &&
-- 
1.7.6.3

Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> We all stood behind my laptop while I explained what it did and
> why. Sverre pointed out that I should use the test_when_finished()
> function for unsetting the config variables, Eric and Fernando looked
> it over as well.

Sounds like fun ;-)

Will re-queue. Thanks.

Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase

[Johannes Sixt]

Am 06.11.2011 10:50, schrieb Ævar Arnfjörð Bjarmason:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 5a841da..b2d7d92 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -677,7 +677,9 @@ branch.<name>.mergeoptions::
>  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.
> +	"git pull" is run. See "pull.rebase" for doing this in a non
> +	branch-specific manner.
> +
>  	*NOTE*: this is a possibly dangerous operation; do *not* use
>  	it unless you understand the implications (see linkgit:git-rebase[1]
>  	for details).

When you continue an indented item in a separate paragraph, then you
must not use an empty line, but have line with '+' and un-indented
continuation paragraphs. See examples in this file.

-- Hannes

Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 6, 2011 at 20:53, Johannes Sixt <j6t@kdbg.org> wrote:
> When you continue an indented item in a separate paragraph, then you
> must not use an empty line, but have line with '+' and un-indented
> continuation paragraphs. See examples in this file.

Thanks for spotting that.

Junio: Should I spam you with another patch or is something you'd
prefer to just fix up?

Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Sun, Nov 6, 2011 at 20:53, Johannes Sixt <j6t@kdbg.org> wrote:
>> When you continue an indented item in a separate paragraph, then you
>> must not use an empty line, but have line with '+' and un-indented
>> continuation paragraphs. See examples in this file.
>
> Thanks for spotting that.
>
> Junio: Should I spam you with another patch or is something you'd
> prefer to just fix up?

It is about the same amount of work for me; I've just dedented the two
paragraphs that start with "*NOTE*:" and replaced the blank lines before
them with a single "+". Is that what you wanted to resend, or is there
anything else?

Re: [PATCH v2] pull: introduce a pull.rebase option to enable --rebase

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 7, 2011 at 17:44, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Sun, Nov 6, 2011 at 20:53, Johannes Sixt <j6t@kdbg.org> wrote:
>>> When you continue an indented item in a separate paragraph, then you
>>> must not use an empty line, but have line with '+' and un-indented
>>> continuation paragraphs. See examples in this file.
>>
>> Thanks for spotting that.
>>
>> Junio: Should I spam you with another patch or is something you'd
>> prefer to just fix up?
>
> It is about the same amount of work for me; I've just dedented the two
> paragraphs that start with "*NOTE*:" and replaced the blank lines before
> them with a single "+". Is that what you wanted to resend, or is there
> anything else?

That should cover it, thanks!