[BUG] git-svn parses --pretty=medium log output, fails when log.decorate is true

[BUG] git-svn parses --pretty=medium log output, fails when log.decorate is true

[Will Palmer]

I started receiving this error during "git svn dcommit" today:

Use of uninitialized value $hash in string eq
at /home/wpalmer/libexec/git-core/git-svn line 1534.

Examining that section reveals that git-svn is running 
"git log --no-color --first-parent --pretty=medium"

and parsing the output in order to find commit hashes and git-svn-id:
lines. This breaks when log.decorate is true.

This could be patched-up by adding "--no-decorate" to the options
git-svn passes, but that seems to me like it would just be adding to the
pile, as "--pretty=medium" is a moving target. I assume the correct
solution is to specify the format exactly as it is expected.

Re: [BUG] git-svn parses --pretty=medium log output, fails when log.decorate is true

[Mathias Lafeldt]

Will Palmer wrote:
> I started receiving this error during "git svn dcommit" today:
> 
> Use of uninitialized value $hash in string eq
> at /home/wpalmer/libexec/git-core/git-svn line 1534.
> 
> Examining that section reveals that git-svn is running 
> "git log --no-color --first-parent --pretty=medium"
> 
> and parsing the output in order to find commit hashes and git-svn-id:
> lines. This breaks when log.decorate is true.
> 
> This could be patched-up by adding "--no-decorate" to the options
> git-svn passes, but that seems to me like it would just be adding to the
> pile, as "--pretty=medium" is a moving target. I assume the correct
> solution is to specify the format exactly as it is expected.

I recently submitted a patch adding --no-decorate:
http://article.gmane.org/gmane.comp.version-control.git/156919

And it's already in Eric's git-svn branch:
git://git.bogomips.org/git-svn

-Mathias

Re: [BUG] git-svn parses --pretty=medium log output, fails when log.decorate is true

[Jeff King]

On Tue, Oct 05, 2010 at 11:18:46AM +0100, Will Palmer wrote:

> I started receiving this error during "git svn dcommit" today:
> 
> Use of uninitialized value $hash in string eq
> at /home/wpalmer/libexec/git-core/git-svn line 1534.
> 
> Examining that section reveals that git-svn is running 
> "git log --no-color --first-parent --pretty=medium"
> 
> and parsing the output in order to find commit hashes and git-svn-id:
> lines. This breaks when log.decorate is true.
> 
> This could be patched-up by adding "--no-decorate" to the options
> git-svn passes, but that seems to me like it would just be adding to the
> pile, as "--pretty=medium" is a moving target. I assume the correct
> solution is to specify the format exactly as it is expected.

The problem isn't necessarily --pretty=medium, but that we are using
"git log" instead of "git rev-list" (though because "git log" does have
some features that rev-list does not have, we do turn off most
configurable features for "git log --pretty=raw").

So I think the simplest thing is just the patch below (which has only
been lightly tested by me):

-- >8 --
Subject: [PATCH] git-svn: use rev-list instead of log

We are parsing the output, so we don't want user
configuration like color or decorations to appear in the
output. The simplest way to accomplish this is to use the
rev-list plumbing instead of the log porcelain.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-svn.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index d292224..03d93d8 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1514,7 +1514,7 @@ sub cmt_sha2rev_batch {
 
 sub working_head_info {
 	my ($head, $refs) = @_;
-	my @args = ('log', '--no-color', '--first-parent', '--pretty=medium');
+	my @args = ('rev-list', '--first-parent', '--pretty=medium');
 	my ($fh, $ctx) = command_output_pipe(@args, $head);
 	my $hash;
 	my %max;
-- 
1.7.3.1.158.g8f5ae

Re: [BUG] git-svn parses --pretty=medium log output, fails when log.decorate is true

[Mathias Lafeldt]

On 10/05/2010 06:58 PM, Jeff King wrote:
> On Tue, Oct 05, 2010 at 11:18:46AM +0100, Will Palmer wrote:
> 
>> I started receiving this error during "git svn dcommit" today:
>>
>> Use of uninitialized value $hash in string eq
>> at /home/wpalmer/libexec/git-core/git-svn line 1534.
>>
>> Examining that section reveals that git-svn is running 
>> "git log --no-color --first-parent --pretty=medium"
>>
>> and parsing the output in order to find commit hashes and git-svn-id:
>> lines. This breaks when log.decorate is true.
>>
>> This could be patched-up by adding "--no-decorate" to the options
>> git-svn passes, but that seems to me like it would just be adding to the
>> pile, as "--pretty=medium" is a moving target. I assume the correct
>> solution is to specify the format exactly as it is expected.
> 
> The problem isn't necessarily --pretty=medium, but that we are using
> "git log" instead of "git rev-list" (though because "git log" does have
> some features that rev-list does not have, we do turn off most
> configurable features for "git log --pretty=raw").
> 
> So I think the simplest thing is just the patch below (which has only
> been lightly tested by me):
> 
> -- >8 --
> Subject: [PATCH] git-svn: use rev-list instead of log
> 
> We are parsing the output, so we don't want user
> configuration like color or decorations to appear in the
> output. The simplest way to accomplish this is to use the
> rev-list plumbing instead of the log porcelain.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  git-svn.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index d292224..03d93d8 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1514,7 +1514,7 @@ sub cmt_sha2rev_batch {
>  
>  sub working_head_info {
>  	my ($head, $refs) = @_;
> -	my @args = ('log', '--no-color', '--first-parent', '--pretty=medium');
> +	my @args = ('rev-list', '--first-parent', '--pretty=medium');
>  	my ($fh, $ctx) = command_output_pipe(@args, $head);
>  	my $hash;
>  	my %max;

You're right. Using rev-list looks like a much cleaner solution.

In addition, I think we can replace the call to git-log in git_svn_log_cmd() too.

-Mathias

Re: [BUG] git-svn parses --pretty=medium log output, fails when log.decorate is true

[Jeff King]

On Tue, Oct 05, 2010 at 07:53:36PM +0200, Mathias Lafeldt wrote:

> > --- a/git-svn.perl
> > +++ b/git-svn.perl
> > @@ -1514,7 +1514,7 @@ sub cmt_sha2rev_batch {
> >  
> >  sub working_head_info {
> >  	my ($head, $refs) = @_;
> > -	my @args = ('log', '--no-color', '--first-parent', '--pretty=medium');
> > +	my @args = ('rev-list', '--first-parent', '--pretty=medium');
> >  	my ($fh, $ctx) = command_output_pipe(@args, $head);
> >  	my $hash;
> >  	my %max;
> 
> You're right. Using rev-list looks like a much cleaner solution.
> 
> In addition, I think we can replace the call to git-log in git_svn_log_cmd() too.

I don't think so. It uses diff options like "--raw --name-status" which
rev-list does not support. I think it is not buggy, though, as it uses
--format=raw.

-Peff