[RFC/PATCH] rebase: add -x option to record original commit name

[RFC/PATCH] rebase: add -x option to record original commit name

[Jay Soffian]

It is often more convenient to use rebase --onto than cherry-pick when
relocating a range of commits, as cherry-pick only supports a single commit at a
time.

This commit teaches rebase the "-x" to record the original commit name, in the
same way as cherry-pick's "-x" option.
---

Note that I explictly did not add support for anything but the simple
format-patch + am codepath. I think it's unlikely to want to record the
original commit name in cases where "-m/-p" are needed. 

However, this option might be useful when using "--interactive", but I wanted
to get feedback first as rebase--interactive is a little scarier to patch. :-)

Thoughts?

 Documentation/git-rebase.txt |    5 +++++
 git-am.sh                    |   14 ++++++++++++++
 git-rebase.sh                |    9 +++++++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..2fb0e96 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -315,6 +315,11 @@ which makes little sense.
 	so that the commit marked for squashing comes right after the
 	commit to be modified, and change the action of the moved
 	commit from `pick` to `squash` (or `fixup`).
+
+-x::
+	Append to the original commit message a note that indicates which commit
+	this change originated from, similar to cherry-pick's -x option. This
+	option is not compatible with -m, -p or --interactive.
 +
 This option is only valid when '--interactive' option is used.
 
diff --git a/git-am.sh b/git-am.sh
index c8b9cbb..83805eb 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -31,6 +31,7 @@ abort           restore the original branch and abort the patching operation.
 committer-date-is-author-date    lie about committer date
 ignore-date     use current timestamp for author date
 rerere-autoupdate update the index with reused conflict resolution if possible
+add-commit-name* (internal use for git-rebase)
 rebasing*       (internal use for git-rebase)"
 
 . git-sh-setup
@@ -296,6 +297,7 @@ git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
 allow_rerere_autoupdate=
+add_commit_name=
 
 while test $# != 0
 do
@@ -347,6 +349,8 @@ do
 		allow_rerere_autoupdate="$1" ;;
 	-q|--quiet)
 		GIT_QUIET=t ;;
+	--add-commit-name)
+		add_commit_name=t ;;
 	--)
 		shift; break ;;
 	*)
@@ -454,6 +458,7 @@ else
 	echo "$keep" >"$dotest/keep"
 	echo "$scissors" >"$dotest/scissors"
 	echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
+	echo "$add_commit_name" > "$dotest/add_commit_name"
 	echo "$GIT_QUIET" >"$dotest/quiet"
 	echo 1 >"$dotest/next"
 	if test -n "$rebasing"
@@ -507,6 +512,10 @@ then
 else
 	no_inbody_headers=
 fi
+if test "$(cat "$dotest/add_commit_name")" = t
+then
+	add_commit_name=t
+fi
 if test "$(cat "$dotest/quiet")" = t
 then
 	GIT_QUIET=t
@@ -630,6 +639,11 @@ do
 		then
 			echo "$ADD_SIGNOFF"
 		fi
+		if test "$add_commit_name" = t
+		then
+			test -z "$commit" && die "Internal error: expecting a commit name."
+			echo "(cherry picked from commit $commit)"
+		fi
 	    } >"$dotest/final-commit"
 	    ;;
 	*)
diff --git a/git-rebase.sh b/git-rebase.sh
index fb4fef7..511599e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -53,6 +53,7 @@ git_am_opt=
 rebase_root=
 force_rebase=
 allow_rerere_autoupdate=
+add_commit_name=
 
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
@@ -353,6 +354,10 @@ do
 	--rerere-autoupdate|--no-rerere-autoupdate)
 		allow_rerere_autoupdate="$1"
 		;;
+	-x)
+		git_am_opt="$git_am_opt --add-commit-name"
+		add_commit_name=t
+		;;
 	-*)
 		usage
 		;;
@@ -386,6 +391,10 @@ else
 		die "previous rebase directory $dotest still exists." \
 			'Try git rebase (--continue | --abort | --skip)'
 	fi
+	if test -n "$add_commit_name"
+	then
+		die "-x not compatible with --merge"
+	fi
 fi
 
 # The tree must be really really clean.
-- 
1.7.0.rc1.200.g9c1f9

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> Thoughts?

In the longer term, we would rather deprecate -x from cherry-pick, so that
we won't contaminate the commit log message.  So in that sense, I would
give a mildly negative response to this patch.

We might instead want to add a hook that is called from cherry-pick (and
rebase will get a similar one) immediately after the command creates a new
commit out of another commit, so that people can record the correspondence
in notes namespace if they choose to.

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Jay Soffian]

On Fri, Feb 5, 2010 at 8:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> In the longer term, we would rather deprecate -x from cherry-pick, so that
> we won't contaminate the commit log message.

Well now that's interesting. I am in the process of back porting a
large amount of development, and it seems very useful to record this
information in the log message. The subject messages are not always
unique, so it's provides a very easy to find the original commit.

I know you can use "git cherry" to look for identical commits, but I'm
actually adding the "(cherry picked from ...)" especially for when the
new commit is not identical, so that I can then easily emit
interdiffs. I want to make sure that the backported work is as true to
the original branch as possible, and it's much easier to compare
commit by commit than the end product.

> So in that sense, I would give a mildly negative response to this patch.
>
> We might instead want to add a hook that is called from cherry-pick (and
> rebase will get a similar one) immediately after the command creates a new
> commit out of another commit, so that people can record the correspondence
> in notes namespace if they choose to.

Hmfph. So I know some folks don't like polluting log messages (e.g.,
the git-svn-id footer, or apparently, the cherry-pick -x message), but
I actually prefer having this data embedded there than hidden in
notes.

j.

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Jay Soffian]

On Fri, Feb 5, 2010 at 8:58 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Well now that's interesting. I am in the process of back porting a
> large amount of development, and it seems very useful to record this
> information in the log message. The subject messages are not always
> unique, so it's provides a very easy to find the original commit.

Sorry, that wasn't clear. I meant, it's a more convenient way to find
the original commit than looking through the original branch for a
commit with the same subject.

j.

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

>> We might instead want to add a hook that is called from cherry-pick (and
>> rebase will get a similar one) immediately after the command creates a new
>> commit out of another commit, so that people can record the correspondence
>> in notes namespace if they choose to.
>
> Hmfph. So I know some folks don't like polluting log messages (e.g.,
> the git-svn-id footer, or apparently, the cherry-pick -x message),...

That reminds me of a slightly related topic.  I've been running git-svn to
follow (but never build) a project without metadata, exactly because I do
not want the log message contamination. I am having a hard time mapping
the commit object name back to the upstream subversion serial number.

I ended up using this hacky script for that.  It finds the svn uuid from
the metadata file, and then finds ".rev_map.$uuid" files from all over the
place to see if any of them contains a record that points at the git
commit I am interested in.

I really wish "git svn" has a built-in way to do something like that;
perhaps I didn't look hard enough.

-- >8 --

#!/usr/bin/perl -w

use strict;
use Carp qw/croak/;
use Fcntl qw/:DEFAULT :seek/;
use constant rev_map_fmt => 'NH40';
use File::Find;

my $uuid = `git config -f .git/svn/.metadata svn-remote.svn.uuid`;
chomp($uuid);

# remotes/trunk

sub find_version {
	my ($name, $sha1) = @_;
	my ($fh, $size);
	if (!sysopen($fh, "$name/.rev_map.$uuid", O_RDONLY)) {
		return undef;
	}
	if (!binmode $fh) {
		close($fh);
		return undef;
	}
	$size = (stat($fh))[7];
	if (($size % 24) != 0) {
		close($fh);
		return undef;
	}

	while (sysread($fh, my $buf, 24) == 24) {
		my ($rev, $commit) = unpack(rev_map_fmt, $buf);
		if ($sha1 eq $commit) {
			close($fh);
			return $rev;
		}
	}
	close($fh);
	return undef;
}

my $top = ".git/svn/refs";
my $commit = $ARGV[0] || "HEAD";
my $sha1 = `git rev-parse $commit`;
chomp($sha1);
my $found;

sub match_svn_revision {
	if (-f "$_/.rev_map.$uuid") {
		my $it = "$File::Find::dir/$_";
		$it =~ s|^\Q$top\E/||;
		my $v = find_version($_, $sha1);
		if ($v) {
			$found = [$it, $v];
		}
		$File::Find::prune = 1;
	}
	if ($found) {
		$File::Find::prune = 1;
	}
}

File::Find::find(\&match_svn_revision, $top);

if ($found) {
	my ($origin, $rev) = @$found;
	$origin =~ s|^remotes/||;
	$origin =~ s|/|-|g;
	if ($origin eq 'trunk') {
		$origin = '';
	} else {
		$origin = "-$origin";
	}
	print "$rev$origin\n";
}

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Jay Soffian]

On Fri, Feb 5, 2010 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Hmfph. So I know some folks don't like polluting log messages (e.g.,
>> the git-svn-id footer, or apparently, the cherry-pick -x message),...
>
> That reminds me of a slightly related topic.  I've been running git-svn to
> follow (but never build) a project without metadata, exactly because I do
> not want the log message contamination. I am having a hard time mapping
> the commit object name back to the upstream subversion serial number.

Since there is a difference of opinion here, how about this proposal:

1) We keep -x in cherry-pick

2) I convince you to add my -x patch to rebase.sh (hmm, how to do that?) :-)

3) We add a -X option to both cherry-pick and rebase.sh that records
in the notes instead of in the log message.

> I ended up using this hacky script for that.  It finds the svn uuid from
> the metadata file, and then finds ".rev_map.$uuid" files from all over the
> place to see if any of them contains a record that points at the git
> commit I am interested in.
>
> I really wish "git svn" has a built-in way to do something like that;
> perhaps I didn't look hard enough.

Is git svn find-rev not what you want? (Caveat, it doesn't seem to
work for me, but it's claimed description seems to be what you're
asking for.)

j.

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> Since there is a difference of opinion here, how about this proposal:
>
> 1) We keep -x in cherry-pick
>
> 2) I convince you to add my -x patch to rebase.sh (hmm, how to do that?) :-)
>
> 3) We add a -X option to both cherry-pick and rebase.sh that records
> in the notes instead of in the log message.

How could that be a counterproposal to a deprecation of -x and adding
hooks as a replacement, as a solution to more general issues?

>> I ended up using this hacky script for that.  It finds the svn uuid from
>> the metadata file, and then finds ".rev_map.$uuid" files from all over the
>> place to see if any of them contains a record that points at the git
>> commit I am interested in.
>>
>> I really wish "git svn" has a built-in way to do something like that;
>> perhaps I didn't look hard enough.
>
> Is git svn find-rev not what you want? (Caveat, it doesn't seem to
> work for me, but it's claimed description seems to be what you're
> asking for.)

Maybe, but it calls cmt_metadata() which is "grep ^git-svn-id:" of the
commit object, so we know why it doesn't X-<.

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Jay Soffian]

On Fri, Feb 5, 2010 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> How could that be a counterproposal to a deprecation of -x and adding
> hooks as a replacement, as a solution to more general issues?

Sorry, it's been a long week and I wasn't thinking clearly.

Yes, adding post-cherry/rebase hook(s) would keep all parties happy. :-)

> Maybe, but it calls cmt_metadata() which is "grep ^git-svn-id:" of the
> commit object, so we know why it doesn't X-<.

Bah, that's too bad. :-(

j.

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[tytso]

On Sat, Feb 06, 2010 at 12:19:02AM -0500, Jay Soffian wrote:
> On Fri, Feb 5, 2010 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > How could that be a counterproposal to a deprecation of -x and adding
> > hooks as a replacement, as a solution to more general issues?
> 
> Sorry, it's been a long week and I wasn't thinking clearly.
> 
> Yes, adding post-cherry/rebase hook(s) would keep all parties happy. :-)

I really don't think it's a contamination of the log when it's used to
record a cherry pick of patch from the dev stream to an older
maintainance branch, but I'll grudgingly accept doing via a hook; but
can there be an easy way to control whether or not the hook is
actually executed?  

*Normally* I don't want cherry-pick -x, but some of the time I do want
it, and manually enabling and disabling the hook by having to do some
kind of "chmod -x .git/hooks/cherry-pick" command.  I suppose I could
create an alias for cherry-pick-record which does a "chmod a+x `git
rev-parse --git-dir`/hooks/cherry-pick; git cherry-pick $*; chmod a-x
`git rev-parse --git-dir`/hooks/cherry-pick", but that seems really
hacky.  :-(

So it would be nice if there was some kind of command-line option to
cherry-pick which could be passed to the hook script.

					- Ted

Re: [RFC/PATCH] rebase: add -x option to record original commit name

[Junio C Hamano]

tytso@mit.edu writes:

> I really don't think it's a contamination of the log when it's used to
> record a cherry pick of patch from the dev stream to an older
> maintainance branch,...

Ok, then I'll happily scrap my earlier comment and accept -x to rebase,
too.

Thanks for a feedback and injection of some sanity.