RFC patchrebase: add -x option to record original commit name

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(-)

Jay Soffian <jaysoffian@gmail.com> writes:

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.

On Fri, Feb 5, 2010 at 8:35 PM, Junio C Hamano <gitster@pobox.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.

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.

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.

On Fri, Feb 5, 2010 at 8:58 PM, Jay Soffian <jaysoffian@gmail.com> wrote:

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.

Jay Soffian <jaysoffian@gmail.com> writes:

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";
}

On Fri, Feb 5, 2010 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:

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.

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.

Jay Soffian <jaysoffian@gmail.com> writes:

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

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

On Fri, Feb 5, 2010 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:

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. :-)

Bah, that's too bad. :-(

j.

On Sat, Feb 06, 2010 at 12:19:02AM -0500, Jay Soffian wrote:

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

tytso@mit.edu writes:

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

Thanks for a feedback and injection of some sanity.