[PATCH] rebase --fix: interactive fixup mode

[PATCH] rebase --fix: interactive fixup mode

[Clemens Buchacher]

Interactive rebase is frequently used not to rebase history, but to
manipulate recent commits. This is typically done using the following
command:

 git rebase -i HEAD~N

Where N has to be large enough such that the the range HEAD~N..HEAD
contains the desired commits. At the same time, it should be small
enough such that the range HEAD~N..HEAD does not include published
commits or a merge commit. Otherwise, the user may accidentally change
published history. Rebasing a merge commit can also have the generally
undesirable effect of linearizing the merge history.

In order to determine a suitable range automatically, it is a reasonable
heuristic to rebase onto the most recent merge commit. It does not
guarantee that published commits are not included -- indeed there is no
way to do that. But, the range is usually large enough to contain the
desired commits. Also, this mechanism works regardless of whether or not
branch tracking has been configured.

So instead of the above command, one can instead use the following:

 git rebase --fix

By default, the range is limited to a maximum of 20 commits. This can be
changed by passing a different number to --fix, e.g.:

 git rebase --fix=50

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

Also on branch cb/rebase-fix at https://github.com/drizzd/git .

 Documentation/git-rebase.txt |    9 +++++++++
 git-rebase.sh                |   37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 504945c..b1eac16 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -332,6 +332,15 @@ link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for details).
 	user edit that list before rebasing.  This mode can also be used to
 	split commits (see SPLITTING COMMITS below).
 
+--fix=<n>::
+	Searches commit history backwards from the current commit until the
+	most recent merge commit, or until a maximum of <n> preceding commits
+	(default: 20), and runs rebase -i <commit>^. The resulting range is
+	typically large enough to contain recent commits which the user might
+	want to edit, while avoiding the usually undesirable effects of
+	rebasing a merge commit, which obviates the need to find a suitable
+	base commit manually.
+
 -p::
 --preserve-merges::
 	Instead of ignoring merges, try to recreate them.
diff --git a/git-rebase.sh b/git-rebase.sh
index 00ca7b9..e95b57f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -38,6 +38,7 @@ git-rebase [-i] --continue | --abort | --skip
 v,verbose!         display a diffstat of what changed upstream
 q,quiet!           be quiet. implies --no-stat
 onto=!             rebase onto given branch instead of upstream
+fix?!              interactive rebase onto last merge commit
 p,preserve-merges! try to recreate merges instead of ignoring them
 s,strategy=!       use the given merge strategy
 no-ff!             cherry-pick all commits, even if unchanged
@@ -95,6 +96,7 @@ type=
 state_dir=
 # One of {'', continue, skip, abort}, as parsed from command line
 action=
+rebase_fix=
 preserve_merges=
 autosquash=
 test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
@@ -178,6 +180,22 @@ run_pre_rebase_hook () {
 	fi
 }
 
+latest_merge_commit()
+{
+	max_nr_commits=$1
+
+	latest_merge=$(git rev-list -1 --merges HEAD)
+	if test -z "$latest_merge"
+	then
+		range=HEAD
+	else
+		range=$latest_merge..HEAD
+	fi
+
+	range_start=$(git rev-list -"$max_nr_commits" "$range" | tail -1)
+	echo $(git rev-parse $range_start^)
+}
+
 test -f "$apply_dir"/applying &&
 	die 'It looks like git-am is in progress. Cannot rebase.'
 
@@ -220,6 +238,20 @@ do
 	-i)
 		interactive_rebase=explicit
 		;;
+	--fix)
+		interactive_rebase=explicit
+		rebase_fix=20
+		# Parse optional argument.
+		if test "${2#-}" = "$2"
+		then
+			if ! expr "$2" : "^[0-9]\+$" >/dev/null
+			then
+				die "Invalid argument to rebase --fix: $2"
+			fi
+			rebase_fix=$2
+			shift
+		fi
+		;;
 	-p)
 		preserve_merges=t
 		test -z "$interactive_rebase" && interactive_rebase=implied
@@ -375,7 +407,10 @@ if test -z "$rebase_root"
 then
 	case "$#" in
 	0)
-		if ! upstream_name=$(git rev-parse --symbolic-full-name \
+		if test -n "$rebase_fix"
+		then
+			upstream_name=$(latest_merge_commit $rebase_fix)
+		elif ! upstream_name=$(git rev-parse --symbolic-full-name \
 			--verify -q @{upstream} 2>/dev/null)
 		then
 			. git-parse-remote
-- 
1.7.8

Re: [PATCH] rebase --fix: interactive fixup mode

[Jakub Narebski]

Clemens Buchacher <drizzd@aon.at> writes:

[...]
> In order to determine a suitable range automatically, it is a reasonable
> heuristic to rebase onto the most recent merge commit.

Why not additionally / instead take into account remote-tracking
branches for "push" remotes?

> It does not
> guarantee that published commits are not included -- indeed there is no
> way to do that. But, the range is usually large enough to contain the
> desired commits. Also, this mechanism works regardless of whether or not
> branch tracking has been configured.
> 
> So instead of the above command, one can instead use the following:
> 
>  git rebase --fix
> 
> By default, the range is limited to a maximum of 20 commits. This can be
> changed by passing a different number to --fix, e.g.:
> 
>  git rebase --fix=50
> 
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>

Nice idea!

-- 
Jakub Narebski
Poland

Re: [PATCH] rebase --fix: interactive fixup mode

[Clemens Buchacher]

On Sun, Jan 08, 2012 at 01:57:03PM -0800, Jakub Narebski wrote:
> 
> > In order to determine a suitable range automatically, it is a reasonable
> > heuristic to rebase onto the most recent merge commit.
> 
> Why not additionally / instead take into account remote-tracking
> branches for "push" remotes?

For me personally, remote-tracking does not work. I frequently branch
locally, and even if I do branch from a remote branch, it's often not
from a public branch, but rather my own private branch that I
synchronize between repos and machines. So my remote-tracking
configuration is usually an awful mess, and it does not feel like fixing
it up manually would be worth the trouble.

As a result, I don't trust remote-tracking and I do not use any of the
features associated with it. For my uses of rebase --fix it would
therefore be counter-productive to consider remote-tracking information.

What I did consider was adding a comment to the list of "pick <commit>"
that interactive rebase offers saying:

# older commits are already contained in the current upstream branch

Also, I often rewrite commits that are also contained in other branches.
That typically happens when I am reworking a topic that has already been
tested extensively. In that case I like to keep the original branch
around for reference, even if I end up not using it eventually.

Re: [PATCH] rebase --fix: interactive fixup mode

[Jonathan Nieder]

Clemens Buchacher wrote:

> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -332,6 +332,15 @@ link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for details).
>  	user edit that list before rebasing.  This mode can also be used to
>  	split commits (see SPLITTING COMMITS below).
>  
> +--fix=<n>::
> +	Searches commit history backwards from the current commit until the
> +	most recent merge commit, or until a maximum of <n> preceding commits
> +	(default: 20), and runs rebase -i <commit>^. The resulting range is
> +	typically large enough to contain recent commits which the user might
> +	want to edit, while avoiding the usually undesirable effects of
> +	rebasing a merge commit, which obviates the need to find a suitable
> +	base commit manually.

Funny. :)  I wonder if this is possible to generalize, to something like

	git rebase -i foo^{last-merge}

or even something like

	git rebase -i foo^{first:--merges}

(where "<commit>^{first:<rev-list args>}" would mean something like
"the first commit listed by "git rev-list <rev-list args> <commit>").
What do you think?

Re: [PATCH] rebase --fix: interactive fixup mode

[Clemens Buchacher]

On Sun, Jan 08, 2012 at 04:01:27PM -0600, Jonathan Nieder wrote:
> 
> Funny. :)  I wonder if this is possible to generalize, to something like
> 
> 	git rebase -i foo^{last-merge}
> 
> What do you think?

I suppose if the history has no merges, I would return the root commit?

Uh, and now I realize I have a bug. In a repo with only linear history:

$ git rebase --fix
fatal: ambiguous argument '4ccf67b9fa2ae247e55b86648d650cb368f286c2^': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
fatal: Needed a single revision
invalid upstream 4ccf67b9fa2ae247e55b86648d650cb368f286c2^

Re: [PATCH] rebase --fix: interactive fixup mode

[Nguyen Thai Ngoc Duy]

2012/1/9 Jonathan Nieder <jrnieder@gmail.com>:
> Funny. :)  I wonder if this is possible to generalize, to something like
>
>        git rebase -i foo^{last-merge}
>
> or even something like
>
>        git rebase -i foo^{first:--merges}
>
> (where "<commit>^{first:<rev-list args>}" would mean something like
> "the first commit listed by "git rev-list <rev-list args> <commit>").
> What do you think?

Is something like this over-generalized?

http://kerneltrap.org/mailarchive/git/2010/12/24/47502

A good thing I see from having a specific option for "-i HEAD~n" is
that it's potentially shorter to type. For someone who does rebase a
lot and has CapsLock turned to Ctrl, it helps. Maybe "rebase -I" ==
"rebase -i HEAD^{last-merge}" (or "rebase -i
<the-revision-used-last-time>") and "rebase -I <n>" == "rebase -i
HEAD~<n>"?
-- 
Duy

Re: [PATCH] rebase --fix: interactive fixup mode

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> Is something like this over-generalized?
>
> http://kerneltrap.org/mailarchive/git/2010/12/24/47502

Yes, I suspect that at the moment (i.e., in the absence of a large
collection of examples to show their utility), both your ^{~custom}
and my ^{first:rev-list args} are overengineered, and that they do
something that is more clearly expressed using the shell's command
substitution feature:

	git rebase -i $(git rev-list --merges HEAD | head -1)

So why did I suggest it?

I guess I was reacting to the implementation of the
rebase-recent-commits command.  I understand that it was a sketch, but
it felt a little ad hoc.  If it could be expressed as a clean
two-liner, I would be more comfortable since the burden of maintaining
it would be less.

Thanks for clarifying.

Sincerely,
Jonathan

Re: [PATCH] rebase --fix: interactive fixup mode

[Junio C Hamano]

Clemens Buchacher <drizzd@aon.at> writes:

> Interactive rebase is frequently used not to rebase history, but to
> manipulate recent commits. This is typically done using the following
> command:
>
>  git rebase -i HEAD~N
>
> Where N has to be large enough such that the the range HEAD~N..HEAD
> contains the desired commits. At the same time, it should be small
> enough such that the range HEAD~N..HEAD does not include published
> commits or a merge commit. Otherwise, the user may accidentally change
> published history. Rebasing a merge commit can also have the generally
> undesirable effect of linearizing the merge history.
>
> In order to determine a suitable range automatically, it is a reasonable
> heuristic to rebase onto the most recent merge commit.

I understand the problem you are trying to solve, but I am not sure if
this is a good idea from the UI point of view for two reasons.

 - "We want to limit the extent of the operation to commits since the last
   merge" is by itself a reasonable thing to ask, and I do not think it
   should be limited to "rebase". If we had an extended SHA-1 syntax to
   express it, for example, you may want to say "I want to see what I did
   since the last merge" and run "git log $last_merge_before_HEAD..".
   Perhaps HEAD~{merge} or something?

 - If your "rebase --fix" is to "fix" things, what is "rebase -i" about?
   Isn't it also about fixing? I imagine that ordinary people expect a
   "fix" option that takes a parameter would take a commit to be fixed,
   and drive the rebase machinery to quickly fix it; in other words,

	$ git rebase --fix=':/^reword the greeting message'

   may internally run "rebase -i $(git rev-parse ':/^rewor...')^", with
   the insn sheet already prepared to "edit" the first commit in it, and
   may even return the control back to the user without showing the insn
   sheet in the editor.

Hmm?

   

Re: [PATCH] rebase --fix: interactive fixup mode

[Clemens Buchacher]

On Sun, Jan 08, 2012 at 02:58:42PM -0800, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> >
> > In order to determine a suitable range automatically, it is a reasonable
> > heuristic to rebase onto the most recent merge commit.
> 
> I understand the problem you are trying to solve, but I am not sure if
> this is a good idea from the UI point of view for two reasons.
> 
>  - "We want to limit the extent of the operation to commits since the last
>    merge" is by itself a reasonable thing to ask, and I do not think it
>    should be limited to "rebase". If we had an extended SHA-1 syntax to
>    express it, for example, you may want to say "I want to see what I did
>    since the last merge" and run "git log $last_merge_before_HEAD..".
>    Perhaps HEAD~{merge} or something?

Ok, sounds reasonable.

I am not sure what to do if the history has no merges, though.  If it's
just rev-parse HEAD~{merge} I suppose I could return nothing, or an
error. But what about the HEAD~{merge}..HEAD range? I think it would be
useful if that were not an error but the entire history.

>  - If your "rebase --fix" is to "fix" things, what is "rebase -i" about?

I would have suggested this to be the default behavior for rebase -i
without an <upstream> argument, but unfortunately we already handle this
case using @{upstream}.

Re: [PATCH] rebase --fix: interactive fixup mode

[Michael Haggerty]

On 01/08/2012 10:31 PM, Clemens Buchacher wrote:
> Interactive rebase is frequently used not to rebase history, but to
> manipulate recent commits. This is typically done using the following
> command:
> 
>  git rebase -i HEAD~N
> 
> Where N has to be large enough such that the the range HEAD~N..HEAD
> contains the desired commits. At the same time, it should be small
> enough such that the range HEAD~N..HEAD does not include published
> commits or a merge commit. Otherwise, the user may accidentally change
> published history. Rebasing a merge commit can also have the generally
> undesirable effect of linearizing the merge history.
> 
> In order to determine a suitable range automatically, it is a reasonable
> heuristic to rebase onto the most recent merge commit. It does not
> guarantee that published commits are not included -- indeed there is no
> way to do that. But, the range is usually large enough to contain the
> desired commits. Also, this mechanism works regardless of whether or not
> branch tracking has been configured.
> 
> So instead of the above command, one can instead use the following:
> 
>  git rebase --fix

Two comments:

* The name "--fix" might be confusing because of its similarity to the
"fixup" command that can be specified in the interactive instructions file.

* I agree with you that "interactive rebase is frequently used not to
rebase history, but to manipulate recent commits".  In fact, I use
interactive rebase *only* for manipulating recent commits and
non-interactive rebase *only* for changing commits' ancestry.  I think
it is a good idea to make these two uses more distinct.  For example, it
makes me nervous that I might mis-type the <upstream> parameter when I
am trying to touch up commits and end up inadvertently rebasing the
commits onto a new parent.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH] rebase --fix: interactive fixup mode

[Neal Kreitzinger]

On 1/9/2012 2:40 AM, Michael Haggerty wrote:
>
> Two comments:
>
> * The name "--fix" might be confusing because of its similarity to the
> "fixup" command that can be specified in the interactive instructions file.
>
> * I agree with you that "interactive rebase is frequently used not to
> rebase history, but to manipulate recent commits".  In fact, I use
> interactive rebase *only* for manipulating recent commits and
> non-interactive rebase *only* for changing commits' ancestry.  I think
> it is a good idea to make these two uses more distinct.  For example, it
> makes me nervous that I might mis-type the<upstream>  parameter when I
> am trying to touch up commits and end up inadvertently rebasing the
> commits onto a new parent.
>
He could all it --touchup like you did above.

v/r,
neal

Re: [PATCH] rebase --fix: interactive fixup mode

[Thomas Rast]

Clemens Buchacher <drizzd@aon.at> writes:

> Interactive rebase is frequently used not to rebase history, but to
> manipulate recent commits. This is typically done using the following
> command:
>
>  git rebase -i HEAD~N
>
> Where N has to be large enough such that the the range HEAD~N..HEAD
> contains the desired commits. At the same time, it should be small
> enough such that the range HEAD~N..HEAD does not include published
> commits or a merge commit.
[...]
>  git rebase --fix
>
> By default, the range is limited to a maximum of 20 commits.

Given the name I would expect --fix to rebase far enough to make recent
fixup!/squash! commits take effect.  Perhaps name it --recent?

(And I also think that the 20 is rather arbitrary...)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] rebase --fix: interactive fixup mode

[Clemens Buchacher]

On Mon, Jan 09, 2012 at 10:13:33AM +0100, Thomas Rast wrote:
> 
> Given the name I would expect --fix to rebase far enough to make recent
> fixup!/squash! commits take effect.  Perhaps name it --recent?

Sure, I am not particular about the name.

> (And I also think that the 20 is rather arbitrary...)

Well, I need some kind of limit. Otherwise we will list the entire
history up to the root commit in case of repos with only linear history.
And that can be rather slow.

Clemens