Re: [RFC/PATCH] git-svn: add support for --first-parent

From: Lars Hjemli <hjemli@gmail.com>
Date: Sep 6, 2007, 08:34 UTC
Message-ID: <8c5c35580709060134x22ed48d2nc9123e234fc6cf70@mail.gmail.com>
In-Reply-To: <20070906075104.GA10192@hand.yhbt.net>

On 9/6/07, Eric Wong <normalperson@yhbt.net> wrote:

Show 24 quoted lines

> Lars Hjemli <hjemli@gmail.com> wrote:
> > On 9/5/07, Eric Wong <normalperson@yhbt.net> wrote:
> > > Lars Hjemli <hjemli@gmail.com> wrote:
> > > > When git-svn uses git-log to find embedded 'git-svn-id'-lines in commit
> > > > messages, it can get confused when local history contains merges with
> > > > other git-svn branches. But if --first-parent is supplied to git-log,
> > > > working_head_info() will only see 'branch-local' commits and thus the
> > > > first commit containing a 'git-svn-id' line should refer to the correct
> > > > subversion branch.
> > >
> > > Ideally, we'd probably stop, say something and give the user the choice
> > > of branches if multiple parents available.
> >
> > Could you elaborate? (I don't understand how following the first
> > parent of a merge could end up at the wrong svn branch)
>
> Well, if the user didn't know about --first-parent (like me yesterday
> :), they could still end up miscommitting to any branch.  That's been a
> complaint of users for a while now.  Thinking more about it,
> --first-parent should probably be the default.
>
> But, if they want to commit a different branch instead of the one they
> merged into (so the second/third/fourth parent), --first-parent would
> never give them that chance.

Ok, so maybe '--first-parent' should be the default unless the user specified something like '--upstream <revspec>', like this:

diff --git a/git-svn.perl b/git-svn.perl
index d21eb7f..0590299 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -819,8 +819,13 @@ sub cmt_metadata {
 sub working_head_info {
        my ($head, $refs) = @_;
        my @args = ('log', '--no-color');
-       push @args, '--first-parent' if $_first_parent;
-       my ($fh, $ctx) = command_output_pipe(@args, $head);
+       if ($_revspec) {
+           push @args, $_revspec;
+       } else {
+           push @args, '--first-parent';
+           push @args, $head;
+       }
+       my ($fh, $ctx) = command_output_pipe(@args);
        my $hash;
        my %max;
        while (<$fh>) {

This should give the user plenty of rope ;-)

--
larsh