threads / patch / 29317

patchrebase --fix: interactive fixup mode

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

## tl;dr

13 messages between Jan 8, 2012 and Jan 10, 2012. Diffs are folded; open one to read it.

replies: 12people: 8as markdown or json

Clemens Buchacher· Jan 8, 2012, 21:31 UTC · lore

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(-)
Show changes to 2 files +45 −1

Documentation/git-rebase.txt, git-rebase.sh

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
Jakub Narebski· Jan 8, 2012, 21:57 UTC · re: Clemens Buchacher · lore

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

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?

Show 16 quoted lines
> 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
Clemens Buchacher· Jan 8, 2012, 22:19 UTC · re: Jakub Narebski · lore

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

On Sun, Jan 08, 2012 at 01:57:03PM -0800, Jakub Narebski wrote:
Show 6 quoted lines
> 
> > 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.

Jonathan Nieder· Jan 8, 2012, 22:01 UTC · re: Clemens Buchacher · lore

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

Clemens Buchacher wrote:
Show 14 quoted lines
> --- 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?

Clemens Buchacher· Jan 8, 2012, 22:25 UTC · re: Jonathan Nieder · lore

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

On Sun, Jan 08, 2012 at 04:01:27PM -0600, Jonathan Nieder wrote:
Show 6 quoted lines
> 
> 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^

Nguyen Thai Ngoc Duy· Jan 9, 2012, 01:44 UTC · re: Jonathan Nieder · lore

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

2012/1/9 Jonathan Nieder <jrnieder@gmail.com>:
Show 11 quoted lines
> 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
Jonathan Nieder· Jan 9, 2012, 08:43 UTC · re: Nguyen Thai Ngoc Duy · lore

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

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

Junio C Hamano· Jan 8, 2012, 22:58 UTC · re: Clemens Buchacher · lore

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

Clemens Buchacher <drizzd@aon.at> writes:
Show 15 quoted lines
> 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?
   
Clemens Buchacher· Jan 9, 2012, 20:33 UTC · re: Junio C Hamano · lore

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

On Sun, Jan 08, 2012 at 02:58:42PM -0800, Junio C Hamano wrote:
Show 14 quoted lines
> 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}.

Michael Haggerty· Jan 9, 2012, 08:40 UTC · re: Clemens Buchacher · lore

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

On 01/08/2012 10:31 PM, Clemens Buchacher wrote:
Show 23 quoted lines
> 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/
Neal Kreitzinger· Jan 10, 2012, 19:58 UTC · re: Michael Haggerty · lore

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

On 1/9/2012 2:40 AM, Michael Haggerty wrote:
Show 15 quoted lines
>
> 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

Thomas Rast· Jan 9, 2012, 09:13 UTC · re: Clemens Buchacher · lore

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

Clemens Buchacher <drizzd@aon.at> writes:
Show 10 quoted lines
> 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
Clemens Buchacher· Jan 9, 2012, 20:16 UTC · re: Thomas Rast · lore

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

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

← back to recent threads