[RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

Normally, if the current branch is up to date, the rebase is aborted.
However, it may be desirable to allow rebasing even if the current
branch is up to date, when using '-f' in combination with the
'--whitespace=fix' option for example.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

Say I have a bunch of new commits ready to submit to origin, but I want to fix
some whitespace damage, I could do something like this:

$ git checkout master
$ git branch -b rebase-me
$ git reset --hard origin/master
$ git commit --allow-empty "force rebase"
$ git checkout rebase-me
$ git rebase --whitespace=fix master
$ git rebase -i master # kick out the 'force rebase' commit
$ git checkout master
$ git reset --hard rebase-me
$ git branch -d rebase-me

The result would be that all commits in origin/master..master have any
whitespace errors fixed, but it seems a bit clumsy. That is, the need to create
a commit on master so that 'git rebase' won't bail out early makes the whole
process a lot more involved. This patch addresses simplifies the above process
to the following:

$ git rebase -f --whitespace=fix origin/master

That's a reduction of 9 commands, not too bad at all.

No tests included yet, will add them if there is any interest in the patch
from the list, otherwise I'll just keep it around locally :).

 git-rebase.sh |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 6d3eddb..55d0f63 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Junio C Hamano.
 #
 
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] [<upstream>|--root] [<branch>]'
+USAGE='[--interactive | -i] [-v] [--force-rebase | -f] [--onto <newbase>] [<upstream>|--root] [<branch>]'
 LONG_USAGE='git-rebase replaces <branch> with a new branch of the
 same name.  When the --onto option is provided the new branch starts
 out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
@@ -48,6 +48,7 @@ prec=4
 verbose=
 git_am_opt=
 rebase_root=
+force_rebase=
 
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
@@ -300,6 +301,9 @@ do
 		;;
 	--root)
 		rebase_root=t
+    ;;
+  -f|--f|--fo|--for|--forc|force|--force-r|--force-re|--force-reb|--force-reba|--force_rebas|--force-rebase)
+    force_rebase=t
 		;;
 	-*)
 		usage
@@ -419,10 +423,15 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 	# linear history?
 	! (git rev-list --parents "$onto".."$branch" | grep " .* ") > /dev/null
 then
-	# Lazily switch to the target branch if needed...
-	test -z "$switch_to" || git checkout "$switch_to"
-	echo >&2 "Current branch $branch_name is up to date."
-	exit 0
+	if test -z "$force_rebase"
+	then
+		# Lazily switch to the target branch if needed...
+		test -z "$switch_to" || git checkout "$switch_to"
+		echo >&2 "Current branch $branch_name is up to date."
+		exit 0
+	else
+		echo "Current branch $branch_name is up to date, rebase forced."
+  fi
 fi
 
 if test -n "$verbose"
-- 
1.6.2.rc0.205.g53b19b.dirty

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Johannes Schindelin]

Hi,

On Thu, 12 Feb 2009, Sverre Rabbelier wrote:

> The result would be that all commits in origin/master..master have any
> whitespace errors fixed, but it seems a bit clumsy.

FWIW I typically use 'git rebase --whitespace=fix $(git merge-base 
origin/master master)' for that, but that only works when there is a 
single merge base.

Ciao,
Dscho

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

Heya,

On Thu, Feb 12, 2009 at 21:28, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> FWIW I typically use 'git rebase --whitespace=fix $(git merge-base
> origin/master master)' for that, but that only works when there is a
> single merge base.

Hehe, shouldof known there is an easier way to do it currently. Why
does that work though? Also, any comments on the patch? ;)

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Johannes Schindelin]

Hi,

On Thu, 12 Feb 2009, Sverre Rabbelier wrote:

> On Thu, Feb 12, 2009 at 21:28, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > FWIW I typically use 'git rebase --whitespace=fix $(git merge-base
> > origin/master master)' for that, but that only works when there is a
> > single merge base.
> 
> Hehe, shouldof known there is an easier way to do it currently. Why
> does that work though?

It works because you are not rebasing onto 'master', but the merge base of 
'master' and your current branch.

So in contrast to the other situation, there are commits left to be 
rebased :-)

> Also, any comments on the patch? ;)

There is probably a thinko in it: if "master" already has your patches, 
then you cannot apply them on top of "master".  That should conflict 
rather horribly, and not change the commits that are already upstream.

Or I misunderstand something here.  Quite possible, I am pretty tired.

Ciao,
Dscho

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

On Thu, Feb 12, 2009 at 21:37, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> So in contrast to the other situation, there are commits left to be
> rebased :-)

Ah, I see, funky hack :).

>> Also, any comments on the patch? ;)
>
> There is probably a thinko in it: if "master" already has your patches,
> then you cannot apply them on top of "master".  That should conflict
> rather horribly, and not change the commits that are already upstream.

Hmmm, afaik there are two cases in which 'git rebase' will abort with
'already up to date':
1. onto == current
2. onto == current + some extra patches

A quick demo of 1:

$ git rebase origin/master
Current branch master is up to date.
$ git rebase -f origin/master
Current branch master is up to date, rebase forced.
First, rewinding head to replay your work on top of it...
Fast-forwarded master to origin/master.


A quick demo of 2:

$ git rebase origin/master
Current branch master is up to date.
$ git rebase -f origin/master
Current branch master is up to date, rebase forced.
First, rewinding head to replay your work on top of it...
Applying: Do not attempt to render a field if it is disabled
Applying: Add ToS agreement to org_admin application related forms.
Applying: Added mentor ToS

> Or I misunderstand something here.  Quite possible, I am pretty tired.

I hope that's the case, in my manual tests everything worked as
expected, but perhaps I didn't understand your concern correctly.

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Say I have a bunch of new commits ready to submit to origin, but I want to fix
> some whitespace damage, I could do something like this:
>
> $ git checkout master
> $ git branch -b rebase-me
> $ git reset --hard origin/master
> $ git commit --allow-empty "force rebase"
> $ git checkout rebase-me
> $ git rebase --whitespace=fix master
> $ git rebase -i master # kick out the 'force rebase' commit
> $ git checkout master
> $ git reset --hard rebase-me
> $ git branch -d rebase-me

For that, I would prefer to see:

	git format-patch --stdout origin >my.mbox
	git fetch origin
        git checkout origin             ;# yes, detach
        git am --whitespace=fix my.mbox
        make test
        git format-patch -o to-send-out origin
	git send-email ..options.. to-send-out

for two reasons.

It fixes whitespace breakages, but more importantly, the procedure makes
sure that what you will be sending out will apply cleanly to the origin
that may have progressed since you last looked at it.

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

On Thu, Feb 12, 2009 at 22:34, Junio C Hamano <gitster@pobox.com> wrote:
> For that, I would prefer to see:
>  (1)   git format-patch --stdout origin >my.mbox
>  (2)   git fetch origin
>  (3)   git checkout origin             ;# yes, detach
>  (4)   git am --whitespace=fix my.mbox
>  (5)   make test
>  (6)   git format-patch -o to-send-out origin
>  (7)   git send-email ..options.. to-send-out

If I understand things correctly 'git rebase -f --whitespace=fix
origin' does only 1 and 4, yes? In my workflow I do 2 and 5 as 'git
pull --rebase' before I push anything, and since I push rather then
send-email, I never use 6 and 7. That leaves 3, which I guess is
specific to your workflow? I am guessing you detach to make it easier
to easily test many different topic branches.

> It fixes whitespace breakages, but more importantly, the procedure makes
> sure that what you will be sending out will apply cleanly to the origin
> that may have progressed since you last looked at it.

Mhhh, would 'git fetch && git rebase -f whitespace=fix orgin' do the
same? Do you see any other problems with the patch?

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Thu, Feb 12, 2009 at 22:34, Junio C Hamano <gitster@pobox.com> wrote:
>> For that, I would prefer to see:
>>  (1)   git format-patch --stdout origin >my.mbox
>>  (2)   git fetch origin
>>  (3)   git checkout origin             ;# yes, detach
>>  (4)   git am --whitespace=fix my.mbox
>>  (5)   make test
>>  (6)   git format-patch -o to-send-out origin
>>  (7)   git send-email ..options.. to-send-out
>
> If I understand things correctly 'git rebase -f --whitespace=fix
> origin' does only 1 and 4, yes? In my workflow I do 2 and 5 as 'git
> pull --rebase' before I push anything, and since I push rather then
> send-email, I never use 6 and 7. That leaves 3, which I guess is
> specific to your workflow? I am guessing you detach to make it easier
> to easily test many different topic branches.
>
>> It fixes whitespace breakages, but more importantly, the procedure makes
>> sure that what you will be sending out will apply cleanly to the origin
>> that may have progressed since you last looked at it.
>
> Mhhh, would 'git fetch && git rebase -f whitespace=fix orgin' do the
> same? Do you see any other problems with the patch?

No, because I didn't read the patch text; I don't read patches that are
larger than 20 lines during the day-job hours.

I was only commenting on your "rebase does not work if I am up to date
with respect to origin and here is my workaround".

Obviously neither the "fetch origin and reapply" nor "pull --rebase" would
make any difference if you were indeed up to date.

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

On Fri, Feb 13, 2009 at 00:22, Junio C Hamano <gitster@pobox.com> wrote:
> No, because I didn't read the patch text; I don't read patches that are
> larger than 20 lines during the day-job hours.

Hehe, that sounds like a fair policy.

> I was only commenting on your "rebase does not work if I am up to date
> with respect to origin and here is my workaround".

Ok, thanks for the comment then, I'll wait for more comment and maybe
write some tests tomorrow.

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Fri, Feb 13, 2009 at 00:22, Junio C Hamano <gitster@pobox.com> wrote:
>> No, because I didn't read the patch text; I don't read patches that are
>> larger than 20 lines during the day-job hours.
>
> Hehe, that sounds like a fair policy.
>
>> I was only commenting on your "rebase does not work if I am up to date
>> with respect to origin and here is my workaround".
>
> Ok, thanks for the comment then, I'll wait for more comment and maybe
> write some tests tomorrow.

Ok, now it is past 5pm, I've read the patch.

I wonder if we can (and if so should) make this a bit more automatic
without having to say "rebase -f --whitespace=fix".

When you say "git rebase --whitespace=fix origin" from the command line,
you are explicitly saying "I want to *fix* breakages in the commits since
'origin'", and at that point, it stops mattering if origin stayed dormant
since you forked from it.  I'd say that the current behaviour is a bug
introduced by the commit that added --whitespace=fix option.

The logic is the same as "git rebase --interactive origin" that does not
refuse to work even if you are already ahead of the origin.

So my suggestion would be to add --force (or -f) like your patch does, and
also detect --whitespace=$option given from the command line, and if it is
fix (or its synonym "strip"), automatically enable --force, perhaps as a
follow up patch, or in the same patch.

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

On Fri, Feb 13, 2009 at 02:32, Junio C Hamano <gitster@pobox.com> wrote:
> I wonder if we can (and if so should) make this a bit more automatic
> without having to say "rebase -f --whitespace=fix".

I have been thinking of that, having whitespace=fix imply -f, but I
wasn't sure whether it makes sense for all workflows that might use
rebase together with whitespace=fix. I aliased 'fixwhitespace' to
'rebase -f --whitespace=fix origin' myself, so I figured I'd send the
patch as-is and see what people think :).

> The logic is the same as "git rebase --interactive origin" that does not
> refuse to work even if you are already ahead of the origin.

This makes sense, and after thinking about it some, I don't really see
any harm in running 'git rebase --whitespace=...' even if the branch
is up to date.

> So my suggestion would be to add --force (or -f) like your patch does, and
> also detect --whitespace=$option given from the command line, and if it is
> fix (or its synonym "strip"), automatically enable --force, perhaps as a
> follow up patch, or in the same patch.

The patch to do so would be fairly trivial I think? That is, add
'force_rebase=t' in the --whitespace=... part. Is that change small
enough to be a single patch, or should it be a follow-up since the
first patch is a-means-to-an-end for the second one?

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Fri, Feb 13, 2009 at 02:32, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> So my suggestion would be to add --force (or -f) like your patch does, and
>> also detect --whitespace=$option given from the command line, and if it is
>> fix (or its synonym "strip"), automatically enable --force, perhaps as a
>> follow up patch, or in the same patch.
>
> The patch to do so would be fairly trivial I think? That is, add
> 'force_rebase=t' in the --whitespace=... part. Is that change small
> enough to be a single patch, or should it be a follow-up since the
> first patch is a-means-to-an-end for the second one?

I thought I left it up to you ;-).

I do not think of a practical purpose of "git rebase -f" without other
options that actively modify the tree each commit records (e.g. the "fix
whitespace" option), perhaps other than to pretend that you committed them
later than you actually did.  A patch that implements --force alone is
harder to justify, because it is unclear what good it does.  It is
especially true if you make --whitespace=fix imply that behaviour.

One more thing.  I kept saying "detect --whitespace=fix (or its synonym
strip)" because people can have "apply.whitespace = fix" in their
configuration file for use with "git am", and countermand the
configuration with "git rebase --whitespace=warn".  Such a usage should
not imply --force.

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Sverre Rabbelier]

On Fri, Feb 13, 2009 at 07:22, Junio C Hamano <gitster@pobox.com> wrote:
> I thought I left it up to you ;-).

Ah, I failed to notice that, my bad.

> I can not think of a practical purpose of "git rebase -f" without other
> options that actively modify the tree each commit records (e.g. the "fix
> whitespace" option), perhaps other than to pretend that you committed them
> later than you actually did.  A patch that implements --force alone is
> harder to justify, because it is unclear what good it does.  It is
> especially true if you make --whitespace=fix imply that behaviour.

Agreed, thanks for the explanation.

> One more thing.  I kept saying "detect --whitespace=fix (or its synonym
> strip)" because people can have "apply.whitespace = fix" in their
> configuration file for use with "git am", and countermand the
> configuration with "git rebase --whitespace=warn".  Such a usage should
> not imply --force.

Ok, so having 'apply.whitespace = fix' in your config _should_ imply
-f, and '--whitespace=[no]warn' as commandline option should not
affect '-f'. This does mean though that anyone with 'apply.whitespace
= fix' in their git config that does a rebase on an up-to-date branch
will automagically have the whitespace fixed. Then again, that
behavior is probably desired for those with 'apply.whitespace = fix'
set.

I'll look into making --whitespace=fix/strip imply -f then, and this
time add some tests :).

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH] Teach rebase to rebase even if upstream is up to date with -f

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Fri, Feb 13, 2009 at 07:22, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> One more thing.  I kept saying "detect --whitespace=fix (or its synonym
>> strip)" because people can have "apply.whitespace = fix" in their
>> configuration file for use with "git am", and countermand the
>> configuration with "git rebase --whitespace=warn".  Such a usage should
>> not imply --force.
>
> Ok, so having 'apply.whitespace = fix' in your config _should_ imply
> -f, and '--whitespace=[no]warn' as commandline option should not
> affect '-f'.

Well, that is actually not quite what I meant.

The intention of the user who runs "git rebase" without an explicit option
(-f nor --whitespace) with such a config is ambiguous.

But "git rebase --whitespace=fix" *is* unambiguously "I'd like to fix it",
with or without a config.  It should imply --force.

Similarly "git rebase --whitespace=warn" *is* unambiguously "I do not want
the rebasing process to touch it, just warn, if necessary".  It should not
imply --force.

So I think it would be the most natural to look only at command line
option --whitespace=*, and make fix/strip imply --force, without looking
at config.