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

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

[Lars Hjemli]

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.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

This passes the test-suite and I've used it to correctly dcommit against a
real-life repository after merging subversion-branches in git ('dcommit -n'
reported the wrong subversion branch while 'dcommit -n --first-parent' got
it right, so I did 'dcommit --first-parent' and then inspected the logs and
diffs in the subversion repo and everything looked swell).

But I'm not a perl person, my understanding of git-svn is limited and the
use of --first-parent may not be the right solution to the problem...

 Documentation/git-svn.txt |   10 ++++++++++
 git-svn.perl              |   17 +++++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index be2e34e..42d7b82 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -317,6 +317,16 @@ This is only used with the 'dcommit' command.
 Print out the series of git arguments that would show
 which diffs would be committed to SVN.
 
+--first-parent::
+
+This is only used with the 'dcommit', 'rebase', 'log', 'find-rev' and
+'show-ignore' commands.
+
+These commands tries to detect the upstream subversion branch by means of
+the embedded 'git-svn-id' line in commit messages. When --first-parent is
+specified, git-svn only follows the first parent of each commit, effectively
+ignoring commits brought into the current branch through merge-operations.
+
 --
 
 ADVANCED OPTIONS
diff --git a/git-svn.perl b/git-svn.perl
index d3c8cd0..d21eb7f 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -59,7 +59,7 @@ my ($_stdin, $_help, $_edit,
 	$_template, $_shared,
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
-	$_prefix, $_no_checkout, $_verbose);
+	$_prefix, $_no_checkout, $_verbose, $_first_parent);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -119,12 +119,15 @@ my %cmd = (
 			  'dry-run|n' => \$_dry_run,
 			  'fetch-all|all' => \$_fetch_all,
 			  'no-rebase' => \$_no_rebase,
+			  'first-parent' => \$_first_parent,
 			%cmt_opts, %fc_opts } ],
 	'set-tree' => [ \&cmd_set_tree,
 	                "Set an SVN repository to a git tree-ish",
 			{ 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
 	'show-ignore' => [ \&cmd_show_ignore, "Show svn:ignore listings",
-			{ 'revision|r=i' => \$_revision } ],
+			{ 'revision|r=i' => \$_revision,
+			  'first-parent' => \$_first_parent
+			} ],
 	'multi-fetch' => [ \&cmd_multi_fetch,
 	                   "Deprecated alias for $0 fetch --all",
 			   { 'revision|r=s' => \$_revision, %fc_opts } ],
@@ -145,15 +148,19 @@ my %cmd = (
 			  'authors-file|A=s' => \$_authors,
 			  'color' => \$Git::SVN::Log::color,
 			  'pager=s' => \$Git::SVN::Log::pager,
+			  'first-parent' => \$_first_parent
 			} ],
 	'find-rev' => [ \&cmd_find_rev, "Translate between SVN revision numbers and tree-ish",
-			{ } ],
+			{
+			  'first-parent' => \$_first_parent
+			} ],
 	'rebase' => [ \&cmd_rebase, "Fetch and rebase your working directory",
 			{ 'merge|m|M' => \$_merge,
 			  'verbose|v' => \$_verbose,
 			  'strategy|s=s' => \$_strategy,
 			  'local|l' => \$_local,
 			  'fetch-all|all' => \$_fetch_all,
+			  'first-parent' => \$_first_parent,
 			  %fc_opts } ],
 	'commit-diff' => [ \&cmd_commit_diff,
 	                   'Commit a diff between two trees',
@@ -811,7 +818,9 @@ sub cmt_metadata {
 
 sub working_head_info {
 	my ($head, $refs) = @_;
-	my ($fh, $ctx) = command_output_pipe('log', '--no-color', $head);
+	my @args = ('log', '--no-color');
+	push @args, '--first-parent' if $_first_parent;
+	my ($fh, $ctx) = command_output_pipe(@args, $head);
 	my $hash;
 	my %max;
 	while (<$fh>) {
-- 
1.5.3.1.g7e90d-dirty

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

[Eric Wong]

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.
> 
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> ---
> 
> This passes the test-suite and I've used it to correctly dcommit against a
> real-life repository after merging subversion-branches in git ('dcommit -n'
> reported the wrong subversion branch while 'dcommit -n --first-parent' got
> it right, so I did 'dcommit --first-parent' and then inspected the logs and
> diffs in the subversion repo and everything looked swell).
> 
> But I'm not a perl person, my understanding of git-svn is limited and the
> use of --first-parent may not be the right solution to the problem...

At first glance this seems reasonable.  I didn't know about git-log
--first-parent until now, but it seems like a good 80% solution.

Ideally, we'd probably stop, say something and give the user the choice
of branches if multiple parents available.

Acked-by-for-next-or-pu: Eric Wong <normalperson@yhbt.net>

>  Documentation/git-svn.txt |   10 ++++++++++
>  git-svn.perl              |   17 +++++++++++++----
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index be2e34e..42d7b82 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -317,6 +317,16 @@ This is only used with the 'dcommit' command.
>  Print out the series of git arguments that would show
>  which diffs would be committed to SVN.
>  
> +--first-parent::
> +
> +This is only used with the 'dcommit', 'rebase', 'log', 'find-rev' and
> +'show-ignore' commands.
> +
> +These commands tries to detect the upstream subversion branch by means of
> +the embedded 'git-svn-id' line in commit messages. When --first-parent is
> +specified, git-svn only follows the first parent of each commit, effectively
> +ignoring commits brought into the current branch through merge-operations.
> +
>  --
>  
>  ADVANCED OPTIONS
> diff --git a/git-svn.perl b/git-svn.perl
> index d3c8cd0..d21eb7f 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -59,7 +59,7 @@ my ($_stdin, $_help, $_edit,
>  	$_template, $_shared,
>  	$_version, $_fetch_all, $_no_rebase,
>  	$_merge, $_strategy, $_dry_run, $_local,
> -	$_prefix, $_no_checkout, $_verbose);
> +	$_prefix, $_no_checkout, $_verbose, $_first_parent);
>  $Git::SVN::_follow_parent = 1;
>  my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
>                      'config-dir=s' => \$Git::SVN::Ra::config_dir,
> @@ -119,12 +119,15 @@ my %cmd = (
>  			  'dry-run|n' => \$_dry_run,
>  			  'fetch-all|all' => \$_fetch_all,
>  			  'no-rebase' => \$_no_rebase,
> +			  'first-parent' => \$_first_parent,
>  			%cmt_opts, %fc_opts } ],
>  	'set-tree' => [ \&cmd_set_tree,
>  	                "Set an SVN repository to a git tree-ish",
>  			{ 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
>  	'show-ignore' => [ \&cmd_show_ignore, "Show svn:ignore listings",
> -			{ 'revision|r=i' => \$_revision } ],
> +			{ 'revision|r=i' => \$_revision,
> +			  'first-parent' => \$_first_parent
> +			} ],
>  	'multi-fetch' => [ \&cmd_multi_fetch,
>  	                   "Deprecated alias for $0 fetch --all",
>  			   { 'revision|r=s' => \$_revision, %fc_opts } ],
> @@ -145,15 +148,19 @@ my %cmd = (
>  			  'authors-file|A=s' => \$_authors,
>  			  'color' => \$Git::SVN::Log::color,
>  			  'pager=s' => \$Git::SVN::Log::pager,
> +			  'first-parent' => \$_first_parent
>  			} ],
>  	'find-rev' => [ \&cmd_find_rev, "Translate between SVN revision numbers and tree-ish",
> -			{ } ],
> +			{
> +			  'first-parent' => \$_first_parent
> +			} ],
>  	'rebase' => [ \&cmd_rebase, "Fetch and rebase your working directory",
>  			{ 'merge|m|M' => \$_merge,
>  			  'verbose|v' => \$_verbose,
>  			  'strategy|s=s' => \$_strategy,
>  			  'local|l' => \$_local,
>  			  'fetch-all|all' => \$_fetch_all,
> +			  'first-parent' => \$_first_parent,
>  			  %fc_opts } ],
>  	'commit-diff' => [ \&cmd_commit_diff,
>  	                   'Commit a diff between two trees',
> @@ -811,7 +818,9 @@ sub cmt_metadata {
>  
>  sub working_head_info {
>  	my ($head, $refs) = @_;
> -	my ($fh, $ctx) = command_output_pipe('log', '--no-color', $head);
> +	my @args = ('log', '--no-color');
> +	push @args, '--first-parent' if $_first_parent;
> +	my ($fh, $ctx) = command_output_pipe(@args, $head);
>  	my $hash;
>  	my %max;
>  	while (<$fh>) {

-- 
Eric Wong

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

[Lars Hjemli]

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)

--
larsh

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

[Eric Wong]

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.

-- 
Eric Wong

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

[David Kastrup]

Eric Wong <normalperson@yhbt.net> writes:

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

If they want to commit to a different branch, they can b****y well
check out that branch and merge to _that_.  That is the way all the
rest of git works and nobody whines that this is a restriction.

Quite the opposite: that git-svn does not feel constrained to actually
commit to the base of the checked-out branch is a complete nuisance.
_Why_ would anybody want to dcommit to a Subversion branch not
associated with the current branch?

Just yesterday, I _again_ had git-svn dcommit to a nonsensical branch
(this time, the commit was to branch instead of the trunk, while more
often the catastrophes happen the other way round).  And it is not
like git-svn gives you a chance to override this bad decision: you
have to rebase -i all of your commit messages from potential
cherrypicks or whatever else and remove the git-svn taglines manually
for every commit before git-svn will deign to dcommit to the same
branch as before.

Eric, this is not "flexibility".  This is madness.  If you think that
it is essential to dcommit to a branch unrelated to the current HEAD,
then create a sane command line interface for it that takes effect at
the time of dcommit.  But the current interface is just throwing dice
as far as the user is concerned, since the addition of git-svn tags to
commit messages happens behind his back, and any merge or cherrypick
or rebase 5 revisions back might come to bite you.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

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

[Lars Hjemli]

On 9/6/07, Eric Wong <normalperson@yhbt.net> wrote:
> 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

[PATCH] git-svn: remove --first-parent, add --upstream

[Lars Hjemli]

This makes git-svn always issue the --first-parent option to git-log when
trying to establish the "base" subversion branch, so the --first-parent
option to git-svn is no longer needed. Instead a new option, --upstream
<revspec>, is introduced. When this is specified the search for embedded
git-svn-id lines in commit messages starts at the specified revision, if
not specified the search starts at HEAD.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/git-svn.txt |   10 +++++-----
 git-svn.perl              |   18 +++++++++---------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 42d7b82..2903777 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -317,15 +317,15 @@ This is only used with the 'dcommit' command.
 Print out the series of git arguments that would show
 which diffs would be committed to SVN.
 
---first-parent::
+--upstream=<revspec>::
 
 This is only used with the 'dcommit', 'rebase', 'log', 'find-rev' and
 'show-ignore' commands.
 
-These commands tries to detect the upstream subversion branch by means of
-the embedded 'git-svn-id' line in commit messages. When --first-parent is
-specified, git-svn only follows the first parent of each commit, effectively
-ignoring commits brought into the current branch through merge-operations.
+These commands tries to detect the upstream subversion branch by traversing
+the first parent of each commit (starting at HEAD), looking for an embedded
+'git-svn-id' line in the commit messages. When --upstream is specified,
+git-svn starts the traversal at the specified commit instead of HEAD.
 
 --
 
diff --git a/git-svn.perl b/git-svn.perl
index d21eb7f..947a944 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -59,7 +59,7 @@ my ($_stdin, $_help, $_edit,
 	$_template, $_shared,
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
-	$_prefix, $_no_checkout, $_verbose, $_first_parent);
+	$_prefix, $_no_checkout, $_verbose, $_upstream);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -119,14 +119,14 @@ my %cmd = (
 			  'dry-run|n' => \$_dry_run,
 			  'fetch-all|all' => \$_fetch_all,
 			  'no-rebase' => \$_no_rebase,
-			  'first-parent' => \$_first_parent,
+			  'upstream=s' => \$_upstream,
 			%cmt_opts, %fc_opts } ],
 	'set-tree' => [ \&cmd_set_tree,
 	                "Set an SVN repository to a git tree-ish",
 			{ 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
 	'show-ignore' => [ \&cmd_show_ignore, "Show svn:ignore listings",
 			{ 'revision|r=i' => \$_revision,
-			  'first-parent' => \$_first_parent
+			  'upstream=s' => \$_upstream
 			} ],
 	'multi-fetch' => [ \&cmd_multi_fetch,
 	                   "Deprecated alias for $0 fetch --all",
@@ -148,11 +148,11 @@ my %cmd = (
 			  'authors-file|A=s' => \$_authors,
 			  'color' => \$Git::SVN::Log::color,
 			  'pager=s' => \$Git::SVN::Log::pager,
-			  'first-parent' => \$_first_parent
+			  'upstream=s' => \$_upstream
 			} ],
 	'find-rev' => [ \&cmd_find_rev, "Translate between SVN revision numbers and tree-ish",
 			{
-			  'first-parent' => \$_first_parent
+			  'upstream=s' => \$_upstream
 			} ],
 	'rebase' => [ \&cmd_rebase, "Fetch and rebase your working directory",
 			{ 'merge|m|M' => \$_merge,
@@ -160,7 +160,7 @@ my %cmd = (
 			  'strategy|s=s' => \$_strategy,
 			  'local|l' => \$_local,
 			  'fetch-all|all' => \$_fetch_all,
-			  'first-parent' => \$_first_parent,
+			  'upstream=s' => \$_upstream,
 			  %fc_opts } ],
 	'commit-diff' => [ \&cmd_commit_diff,
 	                   'Commit a diff between two trees',
@@ -818,9 +818,9 @@ 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);
+	my @args = ('log', '--no-color', '--first-parent');
+	push @args, ($_upstream ? $_upstream : $head);
+	my ($fh, $ctx) = command_output_pipe(@args);
 	my $hash;
 	my %max;
 	while (<$fh>) {
-- 
1.5.3.1.g0e33-dirty

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Steven Grimm]

Lars Hjemli wrote:
> This makes git-svn always issue the --first-parent option to git-log when
> trying to establish the "base" subversion branch, so the --first-parent
> option to git-svn is no longer needed. Instead a new option, --upstream
> <revspec>, is introduced. When this is specified the search for embedded
> git-svn-id lines in commit messages starts at the specified revision, if
> not specified the search starts at HEAD.
>   

This is a much better solution -- I can't personally imagine a scenario 
where I'd want anything other than the --first-parent behavior, so 
making it the default is much more convenient. Thanks.

-Steve

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Eric Wong]

Lars Hjemli <hjemli@gmail.com> wrote:
> This makes git-svn always issue the --first-parent option to git-log when
> trying to establish the "base" subversion branch, so the --first-parent
> option to git-svn is no longer needed. Instead a new option, --upstream
> <revspec>, is introduced. When this is specified the search for embedded
> git-svn-id lines in commit messages starts at the specified revision, if
> not specified the search starts at HEAD.
> 
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>

Looks good to me.

Acked-by: Eric Wong <normalperson@yhbt.net>

> ---
>  Documentation/git-svn.txt |   10 +++++-----
>  git-svn.perl              |   18 +++++++++---------
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 42d7b82..2903777 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -317,15 +317,15 @@ This is only used with the 'dcommit' command.
>  Print out the series of git arguments that would show
>  which diffs would be committed to SVN.
>  
> ---first-parent::
> +--upstream=<revspec>::
>  
>  This is only used with the 'dcommit', 'rebase', 'log', 'find-rev' and
>  'show-ignore' commands.
>  
> -These commands tries to detect the upstream subversion branch by means of
> -the embedded 'git-svn-id' line in commit messages. When --first-parent is
> -specified, git-svn only follows the first parent of each commit, effectively
> -ignoring commits brought into the current branch through merge-operations.
> +These commands tries to detect the upstream subversion branch by traversing
> +the first parent of each commit (starting at HEAD), looking for an embedded
> +'git-svn-id' line in the commit messages. When --upstream is specified,
> +git-svn starts the traversal at the specified commit instead of HEAD.
>  
>  --
>  
> diff --git a/git-svn.perl b/git-svn.perl
> index d21eb7f..947a944 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -59,7 +59,7 @@ my ($_stdin, $_help, $_edit,
>  	$_template, $_shared,
>  	$_version, $_fetch_all, $_no_rebase,
>  	$_merge, $_strategy, $_dry_run, $_local,
> -	$_prefix, $_no_checkout, $_verbose, $_first_parent);
> +	$_prefix, $_no_checkout, $_verbose, $_upstream);
>  $Git::SVN::_follow_parent = 1;
>  my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
>                      'config-dir=s' => \$Git::SVN::Ra::config_dir,
> @@ -119,14 +119,14 @@ my %cmd = (
>  			  'dry-run|n' => \$_dry_run,
>  			  'fetch-all|all' => \$_fetch_all,
>  			  'no-rebase' => \$_no_rebase,
> -			  'first-parent' => \$_first_parent,
> +			  'upstream=s' => \$_upstream,
>  			%cmt_opts, %fc_opts } ],
>  	'set-tree' => [ \&cmd_set_tree,
>  	                "Set an SVN repository to a git tree-ish",
>  			{ 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
>  	'show-ignore' => [ \&cmd_show_ignore, "Show svn:ignore listings",
>  			{ 'revision|r=i' => \$_revision,
> -			  'first-parent' => \$_first_parent
> +			  'upstream=s' => \$_upstream
>  			} ],
>  	'multi-fetch' => [ \&cmd_multi_fetch,
>  	                   "Deprecated alias for $0 fetch --all",
> @@ -148,11 +148,11 @@ my %cmd = (
>  			  'authors-file|A=s' => \$_authors,
>  			  'color' => \$Git::SVN::Log::color,
>  			  'pager=s' => \$Git::SVN::Log::pager,
> -			  'first-parent' => \$_first_parent
> +			  'upstream=s' => \$_upstream
>  			} ],
>  	'find-rev' => [ \&cmd_find_rev, "Translate between SVN revision numbers and tree-ish",
>  			{
> -			  'first-parent' => \$_first_parent
> +			  'upstream=s' => \$_upstream
>  			} ],
>  	'rebase' => [ \&cmd_rebase, "Fetch and rebase your working directory",
>  			{ 'merge|m|M' => \$_merge,
> @@ -160,7 +160,7 @@ my %cmd = (
>  			  'strategy|s=s' => \$_strategy,
>  			  'local|l' => \$_local,
>  			  'fetch-all|all' => \$_fetch_all,
> -			  'first-parent' => \$_first_parent,
> +			  'upstream=s' => \$_upstream,
>  			  %fc_opts } ],
>  	'commit-diff' => [ \&cmd_commit_diff,
>  	                   'Commit a diff between two trees',
> @@ -818,9 +818,9 @@ 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);
> +	my @args = ('log', '--no-color', '--first-parent');
> +	push @args, ($_upstream ? $_upstream : $head);
> +	my ($fh, $ctx) = command_output_pipe(@args);
>  	my $hash;
>  	my %max;
>  	while (<$fh>) {
> -- 
> 1.5.3.1.g0e33-dirty
> 

-- 
Eric Wong

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Lars Hjemli <hjemli@gmail.com> wrote:
> > This makes git-svn always issue the --first-parent option to git-log when
> > trying to establish the "base" subversion branch, so the --first-parent
> > option to git-svn is no longer needed. Instead a new option, --upstream
> > <revspec>, is introduced. When this is specified the search for embedded
> > git-svn-id lines in commit messages starts at the specified revision, if
> > not specified the search starts at HEAD.
> > 
> > Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> 
> Looks good to me.
> 
> Acked-by: Eric Wong <normalperson@yhbt.net>

Wait, actually.  --upstream won't ever populate the refs array in
working_head_info for dcommit (but --first-parent should be great for
that).

But the rest of the commands is fine.  I also just noticed cmd_find_rev
passes the @refs argument to working_head_info() unnecessarily.

-- 
Eric Wong

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Lars Hjemli]

On 9/6/07, Eric Wong <normalperson@yhbt.net> wrote:
> Wait, actually.  --upstream won't ever populate the refs array in
> working_head_info for dcommit

Sorry, I didn't realize that working_head_info() collected commit-ids
later used by dcommit.  But to implement --upstream we could maybe do
something like this:

sub working_head_info {
  my ($head, $refs) = @_;

  if (defined $_upstream) {
    working_head_info_traverse($head, \$refs);
    return working_head_info_traverse($_upstream, undef);
  }

  return working_head_info_traverse($head, \$refs);
}

sub working_head_info_traverse {
  my ($head, $refs) = @_;
  my ($fh, $ctx) = command_output_pipe('log', '--no-color',
'--first-parent', $head);
  ...


(This was written straight into firefox, late at night, by a perl
illiterate. Please be gentle...)

-- 
larsh

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Peter Baumann]

On Fri, Sep 07, 2007 at 12:14:30AM +0200, Lars Hjemli wrote:
> On 9/6/07, Eric Wong <normalperson@yhbt.net> wrote:
> > Wait, actually.  --upstream won't ever populate the refs array in
> > working_head_info for dcommit
> 
> Sorry, I didn't realize that working_head_info() collected commit-ids
> later used by dcommit.  But to implement --upstream we could maybe do
> something like this:
> 
> sub working_head_info {
>   my ($head, $refs) = @_;
> 
>   if (defined $_upstream) {
>     working_head_info_traverse($head, \$refs);
>     return working_head_info_traverse($_upstream, undef);
>   }
> 
>   return working_head_info_traverse($head, \$refs);
> }
> 
> sub working_head_info_traverse {
>   my ($head, $refs) = @_;
>   my ($fh, $ctx) = command_output_pipe('log', '--no-color',
> '--first-parent', $head);
>   ...
> 
> 
> (This was written straight into firefox, late at night, by a perl
> illiterate. Please be gentle...)
> 

Sorry, but isn't --upstream just the wrong way to do what you want?
Why should I specify a GIT commit to leat git-svn figure out on what
upstream SVN branch I want to commit? To me, this seems a little
backwards. Wouldn't it be much more pleasant to say something like

	git-svn dcommit --on the_branch

whereas 'the_branch' is the name of the upstream branch as specified
in the fetch/branch section in the git config? If I do a dcommit I know
*exactly* on which svn branch it should go, so why can't I specify it on
the cmdline? ...  or did I miss something obvious?

-Peter

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Lars Hjemli]

On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> Wouldn't it be much more pleasant to say something like
>
>         git-svn dcommit --on the_branch
>
> whereas 'the_branch' is the name of the upstream branch as specified
> in the fetch/branch section in the git config?

Well, git-svn extracts the svn url, revision and repo uuid from the
commit message, while your proposal only specifies the url. But I'm
still not certain that there is a need for --upstream or anything
similar if git-svn always uses 'git log --first-parent' (see
http://article.gmane.org/gmane.comp.version-control.git/57951).

--
larsh

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Peter Baumann]

On Fri, Sep 07, 2007 at 02:23:58AM +0200, Lars Hjemli wrote:
> On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > Wouldn't it be much more pleasant to say something like
> >
> >         git-svn dcommit --on the_branch
> >
> > whereas 'the_branch' is the name of the upstream branch as specified
> > in the fetch/branch section in the git config?
> 
> Well, git-svn extracts the svn url, revision and repo uuid from the
> commit message, while your proposal only specifies the url. But I'm
> still not certain that there is a need for --upstream or anything
> similar if git-svn always uses 'git log --first-parent' (see
> http://article.gmane.org/gmane.comp.version-control.git/57951).
> 

First parent is a heuristic (and a good one, me thinks).

If you did something like this:

(1) Start state:

       a-b-c-d-e    trunk	(both trunk and branch1 are imported
          \			 from SVN)
	   \-x-y    branch1

(2) Hm. My Branch 'branch1' should be ready to be merged to 'trunk', so
   lets do it (not yet dcommited)

       a-b-c-d-e- m trunk
          \	 /
	   \ -x-y   branch1

(3) ARGH. I just discovered a serious bug in 'branch1' and can't just merge
   it into 'trunk', yet. But the merge was painfull enough so I don't want to
   redo it again, so lets reset 'trunk' to its state before the merge and
   'branch1' to the merge commit, before fixing the bug in 'branch1'.

       a-b-c-d-e    trunk
          \	 \
	   \ -x-y m branch1

Notice that this DAG is identical to the one in (2), but just the branch
labels stick to different commits. And if you now want to commit the
merge 'm' to 'branch1' before fixing the bug you are screwed, because
--first-parent will give you 'e' instead of 'y'.

Yes, I know that this example isn't something happening every day, but
at least it shows that --first-parent could *only* be a heuristic and
not something you would rely 100% on. And if you imagine several people
who are sharing their git commits for codereview with pulling/pushing,
it isn't obvious what branch got merged into the other, because it is
possible that the other person did the merge.

Don't get me wrong, --first-parent *is* an improvement over the current
behaviour, but I think it is simply not the *best* we can do.

-Peter

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Lars Hjemli]

On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
>    lets reset 'trunk' to its state before the merge and
>    'branch1' to the merge commit, before fixing the bug in 'branch1'.
>
>        a-b-c-d-e    trunk
>           \      \
>            \ -x-y m branch1

Yeah, this would certainly not be handled correctly by dcommit using
--first-parent (but it could be handled by (a correct implementation
of) --upstream).

Thanks for the example, I had a feeling something like this could
occur. So I guess I'll have another go at --upstream this weekend.

[btw: could you please stop messing with 'Mail-Followup-To:'? When
replying to your mail, I don't want everyone _except_ you in the 'To:'
header...]

--
larsh

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Peter Baumann]

On Fri, Sep 07, 2007 at 12:13:23PM +0200, Lars Hjemli wrote:
> On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> >    lets reset 'trunk' to its state before the merge and
> >    'branch1' to the merge commit, before fixing the bug in 'branch1'.
> >
> >        a-b-c-d-e    trunk
> >           \      \
> >            \ -x-y m branch1
> 
> Yeah, this would certainly not be handled correctly by dcommit using
> --first-parent (but it could be handled by (a correct implementation
> of) --upstream).
> 
> Thanks for the example, I had a feeling something like this could
> occur. So I guess I'll have another go at --upstream this weekend.
> 
> [btw: could you please stop messing with 'Mail-Followup-To:'? When
> replying to your mail, I don't want everyone _except_ you in the 'To:'
> header...]
> 

Sorry, I wasn't aware of that.

I had a 'subscribe git@vger.kernel.org' in my muttrc and just pressed 'g'
for group reply. Reading the docs suggested to 'set followup_to=no' (as
I did before sending this message). Per default it is set to 'yes'.

Could anyone more experienced with mutt correct me if this was the wrong
fix for this problem (or even point me to the right documentation)?

-Peter

Configure mutt to be used in git and lkml mailing lists (was: Re: [PATCH] git-svn: remove --first-parent, add --upstream)

[Fernando J. Pereda]

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On Fri, Sep 07, 2007 at 01:51:30PM +0200, Peter Baumann wrote:
> > [btw: could you please stop messing with 'Mail-Followup-To:'? When
> > replying to your mail, I don't want everyone _except_ you in the 'To:'
> > header...]
> > 
> 
> Sorry, I wasn't aware of that.
> 
> I had a 'subscribe git@vger.kernel.org' in my muttrc and just pressed 'g'
> for group reply. Reading the docs suggested to 'set followup_to=no' (as
> I did before sending this message). Per default it is set to 'yes'.
> 
> Could anyone more experienced with mutt correct me if this was the wrong
> fix for this problem (or even point me to the right documentation)?
> 

[I'm sending this to the list too so it ends up in the archives. I cut
the rest of the people from the To: list because this is not directly
relevant to them.]

Just don't use 'subscribe list' and mutt won't mess up with
mail-followup-to. Instead of using 'l' to reply-to-list keep using 'g'
to reply to everyone involved in the subthread and everything should be
fine :>

- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Eric Wong]

Peter Baumann <waste.manager@gmx.de> wrote:
> On Fri, Sep 07, 2007 at 12:13:23PM +0200, Lars Hjemli wrote:
> > [btw: could you please stop messing with 'Mail-Followup-To:'? When
> > replying to your mail, I don't want everyone _except_ you in the 'To:'
> > header...]
> > 
> 
> Sorry, I wasn't aware of that.
> 
> I had a 'subscribe git@vger.kernel.org' in my muttrc and just pressed 'g'
> for group reply. Reading the docs suggested to 'set followup_to=no' (as
> I did before sending this message). Per default it is set to 'yes'.

I don't have any "subscribe <address>" lines in my muttrc nor do I
have any followup_to settings in it.  It's just default, and 'g' has
always managed to work fine.

> Could anyone more experienced with mutt correct me if this was the wrong
> fix for this problem (or even point me to the right documentation)?

man 5 muttrc

However, whatever you did seems to have worked as replying to this
email didn't drop your name from this.

-- 
Eric Wong

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Lars Hjemli]

On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> >    lets reset 'trunk' to its state before the merge and
> >    'branch1' to the merge commit, before fixing the bug in 'branch1'.
> >
> >        a-b-c-d-e    trunk
> >           \      \
> >            \ -x-y m branch1
>
> Yeah, this would certainly not be handled correctly by dcommit using
> --first-parent (but it could be handled by (a correct implementation
> of) --upstream).

Actually, I don't think there's any way to handle this correctly. The
current git-svn will do the right thing except in cases like the one
you described, and in these cases it can be _forced_ to do the right
thing by editing the grafts file, so I'll drop the whole --upstream
idea.

--
larsh

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Peter Baumann]

On Sat, Sep 15, 2007 at 04:08:31PM +0200, Lars Hjemli wrote:
> On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > >    lets reset 'trunk' to its state before the merge and
> > >    'branch1' to the merge commit, before fixing the bug in 'branch1'.
> > >
> > >        a-b-c-d-e    trunk
> > >           \      \
> > >            \ -x-y m branch1
> >
> > Yeah, this would certainly not be handled correctly by dcommit using
> > --first-parent (but it could be handled by (a correct implementation
> > of) --upstream).
> 
> Actually, I don't think there's any way to handle this correctly. The
> current git-svn will do the right thing except in cases like the one
> you described, and in these cases it can be _forced_ to do the right
> thing by editing the grafts file, so I'll drop the whole --upstream
> idea.
> 

What do you mean by editing the graft file? Remove (the wrong) parent
from the merge commit by a graft? This will help you commit on the right
branch, but 7b02b85a66fee6b357e02f9e70dd0baa0fd24308 removes the
possibility to get the same graph back, because now git-svn really
honors the graft in its rebase phase.

So you would get something like this back:

       a-b - c - d - e    trunk
          \
           \ -x'-y'- m branch1

Notice that you've lost the merge information :-(

-Peter

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Lars Hjemli]

On 9/15/07, Peter Baumann <waste.manager@gmx.de> wrote:
> On Sat, Sep 15, 2007 at 04:08:31PM +0200, Lars Hjemli wrote:
> > On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > > On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > > >    lets reset 'trunk' to its state before the merge and
> > > >    'branch1' to the merge commit, before fixing the bug in 'branch1'.
> > > >
> > > >        a-b-c-d-e    trunk
> > > >           \      \
> > > >            \ -x-y m branch1
> > >
> > > Yeah, this would certainly not be handled correctly by dcommit using
> > > --first-parent (but it could be handled by (a correct implementation
> > > of) --upstream).
> >
> > Actually, I don't think there's any way to handle this correctly. The
> > current git-svn will do the right thing except in cases like the one
> > you described, and in these cases it can be _forced_ to do the right
> > thing by editing the grafts file, so I'll drop the whole --upstream
> > idea.
> >
>
> What do you mean by editing the graft file? Remove (the wrong) parent
> from the merge commit by a graft?

I imagined just changing the order of the parents.

-- 
larsh

Re: [PATCH] git-svn: remove --first-parent, add --upstream

[Peter Baumann]

On Sat, Sep 15, 2007 at 05:24:02PM +0200, Lars Hjemli wrote:
> On 9/15/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > On Sat, Sep 15, 2007 at 04:08:31PM +0200, Lars Hjemli wrote:
> > > On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > > > On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > > > >    lets reset 'trunk' to its state before the merge and
> > > > >    'branch1' to the merge commit, before fixing the bug in 'branch1'.
> > > > >
> > > > >        a-b-c-d-e    trunk
> > > > >           \      \
> > > > >            \ -x-y m branch1
> > > >
> > > > Yeah, this would certainly not be handled correctly by dcommit using
> > > > --first-parent (but it could be handled by (a correct implementation
> > > > of) --upstream).
> > >
> > > Actually, I don't think there's any way to handle this correctly. The
> > > current git-svn will do the right thing except in cases like the one
> > > you described, and in these cases it can be _forced_ to do the right
> > > thing by editing the grafts file, so I'll drop the whole --upstream
> > > idea.
> > >
> >
> > What do you mean by editing the graft file? Remove (the wrong) parent
> > from the merge commit by a graft?
> 
> I imagined just changing the order of the parents.
> 

Doh. I missed the obvious.

-Peter