threads / patch / 29462

patchgit-svn: un-break "git svn rebase" when log.abbrevCommit=true

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

## tl;dr

9 messages between Jan 29, 2012 and Feb 12, 2012. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Ævar Arnfjörð Bjarmason· Jan 29, 2012, 02:11 UTC · lore

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

Show changes to git-svn.perl +2 −1

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

Jonathan Nieder· Jan 29, 2012, 02:59 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

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

Ævar Arnfjörð Bjarmason· Jan 29, 2012, 03:12 UTC · re: Jonathan Nieder · lore

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

On Sun, Jan 29, 2012 at 03:59, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 21 quoted lines

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

Dan Johnson· Jan 30, 2012, 20:41 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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

Ævar Arnfjörð Bjarmason· Jan 30, 2012, 21:18 UTC · re: Dan Johnson · lore

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

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.

Ævar Arnfjörð Bjarmason· Feb 12, 2012, 00:23 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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

Show changes to git-svn.perl +1 −1

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

Ævar Arnfjörð Bjarmason· Feb 12, 2012, 00:23 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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.

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

Show changes to git-svn.perl +1 −2

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

Jonathan Nieder· Feb 12, 2012, 00:31 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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

[...]

Show 9 quoted lines

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

Eric Wong· Feb 12, 2012, 06:49 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 5 quoted lines

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