[PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Ævar Arnfjörð Bjarmason]

Change git-svn to parse --no-abbrev-commit --no-decorate to git-log
since it's trying to parse the log output, and the parsing will fail
completely if log.abbrevCommit is set to true in the configuration,
also set --no-decorate so we won't fail if we're processing a commit
that has a tag pointing to it.

Without this patch the code will fail to parse a SHA1, and then just
spew a bunch of "Use of uninitialized value $hash in string eq"
warnings at "if ($c && $c eq $hash) { ..." and never do anything
useful.

A real fix would be to make git-svn not use any porcelain utilities,
but just work around it for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-svn.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index eeb83d3..570fe9c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1879,7 +1879,8 @@ sub cmt_sha2rev_batch {
 sub working_head_info {
 	my ($head, $refs) = @_;
 	my @args = qw/log --no-color --no-decorate --first-parent
-	              --pretty=medium/;
+	              --pretty=medium --no-abbrev-commit --no-decorate/;
+
 	my ($fh, $ctx) = command_output_pipe(@args, $head);
 	my $hash;
 	my %max;
-- 
1.7.8.3

Re: [PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1879,7 +1879,8 @@ sub cmt_sha2rev_batch {
>  sub working_head_info {
>  	my ($head, $refs) = @_;
>  	my @args = qw/log --no-color --no-decorate --first-parent
> -	              --pretty=medium/;
> +	              --pretty=medium --no-abbrev-commit --no-decorate/;
> +

Oh, dear.  Wouldn't

	my @args = qw/rev-list --first-parent --pretty=medium/;

be simpler and more robust as a quick fix?

Of course something that takes plain rev-list --first-parent output
and pipes it through to cat-file --batch might be more intuitive, but
rev-list --pretty should at least work. ;-)

Re: [PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Ævar Arnfjörð Bjarmason]

On Sun, Jan 29, 2012 at 03:59, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -1879,7 +1879,8 @@ sub cmt_sha2rev_batch {
>>  sub working_head_info {
>>       my ($head, $refs) = @_;
>>       my @args = qw/log --no-color --no-decorate --first-parent
>> -                   --pretty=medium/;
>> +                   --pretty=medium --no-abbrev-commit --no-decorate/;
>> +
>
> Oh, dear.  Wouldn't
>
>        my @args = qw/rev-list --first-parent --pretty=medium/;
>
> be simpler and more robust as a quick fix?
>
> Of course something that takes plain rev-list --first-parent output
> and pipes it through to cat-file --batch might be more intuitive, but
> rev-list --pretty should at least work. ;-)

That sounds like a better fix. I forgot that rev-list could emulate
git-log like that.

Re: [PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Dan Johnson]

On Sat, Jan 28, 2012 at 9:11 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Change git-svn to parse --no-abbrev-commit --no-decorate to git-log
Did you mean _pass_ --no-abbrev-commit here?

-- 
-Dan

Re: [PATCH] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Ævar Arnfjörð Bjarmason]

On Mon, Jan 30, 2012 at 21:41, Dan Johnson <computerdruid@gmail.com> wrote:
> On Sat, Jan 28, 2012 at 9:11 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Change git-svn to parse --no-abbrev-commit --no-decorate to git-log
> Did you mean _pass_ --no-abbrev-commit here?

Yup.

I'l submit another patch fixing this and using rev-list.

[PATCH v2 1/2] git-svn: remove redundant porcelain option to rev-list

[Ævar Arnfjörð Bjarmason]

Change an invocation of git-rev-list(1) to not use --no-color,
git-rev-list(1) will always ignore that option and the --color option,
so there's no need to pass it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-svn.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index eeb83d3..712eeeb 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3920,7 +3920,7 @@ sub rebuild {
 	my ($base_rev, $head) = ($partial ? $self->rev_map_max_norebuild(1) :
 		(undef, undef));
 	my ($log, $ctx) =
-	    command_output_pipe(qw/rev-list --pretty=raw --no-color --reverse/,
+	    command_output_pipe(qw/rev-list --pretty=raw --reverse/,
 				($head ? "$head.." : "") . $self->refname,
 				'--');
 	my $metadata_url = $self->metadata_url;
-- 
1.7.9

[PATCH v2 2/2] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Ævar Arnfjörð Bjarmason]

Change git-svn to use git-rev-list(1) instead of git-log(1) since the
latter is porcelain that'll cause "git svn rebase" to fail completely
if log.abbrevCommit is set to true in the configuration.

Without this patch the code will fail to parse a SHA1, and then just
spew a bunch of "Use of uninitialized value $hash in string eq"
warnings at "if ($c && $c eq $hash) { ..." and never do anything
useful.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
---
 git-svn.perl |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 712eeeb..bebe38b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1878,8 +1878,7 @@ sub cmt_sha2rev_batch {
 
 sub working_head_info {
 	my ($head, $refs) = @_;
-	my @args = qw/log --no-color --no-decorate --first-parent
-	              --pretty=medium/;
+	my @args = qw/rev-list --first-parent --pretty=medium/;
 	my ($fh, $ctx) = command_output_pipe(@args, $head);
 	my $hash;
 	my %max;
-- 
1.7.9

Re: [PATCH v2 2/2] git-svn: un-break "git svn rebase" when log.abbrevCommit=true

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Change git-svn to use git-rev-list(1) instead of git-log(1) since the
> latter is porcelain that'll cause "git svn rebase" to fail completely
> if log.abbrevCommit is set to true in the configuration.
[...]
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1878,8 +1878,7 @@ sub cmt_sha2rev_batch {
>  
>  sub working_head_info {
>  	my ($head, $refs) = @_;
> -	my @args = qw/log --no-color --no-decorate --first-parent
> -	              --pretty=medium/;
> +	my @args = qw/rev-list --first-parent --pretty=medium/;

Thanks!  The other caller to "git log" in this script uses
--pretty=raw and should be safe.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Re: [PATCH v2 1/2] git-svn: remove redundant porcelain option to rev-list

[Eric Wong]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Change an invocation of git-rev-list(1) to not use --no-color,
> git-rev-list(1) will always ignore that option and the --color option,
> so there's no need to pass it.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

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

Thanks, will push this series.