[PATCH 1/2] git-svn: teach dcommit about svn auto-props

[PATCH 1/2] git-svn: teach dcommit about svn auto-props

[Brad King]

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


Subversion repositories often require files to have properties such as
svn:mime-type and svn:eol-style set when they are added.  Users
typically set these properties automatically using the SVN auto-props
feature with 'svn add'.  This commit teaches dcommit to look at the user
SVN configuration and apply matching auto-props entries for files added
by a diff as it is applied to the SVN remote.  A later commit will make
this feature optional.

Signed-off-by: Brad King <brad.king@kitware.com>
---
This change honors the user's enable-auto-props svn config setting.
The next patch will configure this at the git level and add the
corresponding documentation.

I've tested this by hand on an real SVN repo that checks for mime type.
Unfortunately I'm unable to run the git-svn test suite because I get
the error reported here:

  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=486527

(even without my changes).

 git-svn.perl |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)






[-- Attachment #2: 31da8bfbf702861ed89b7c6fd307c99ebbd01165.diff --]
[-- Type: text/x-patch, Size: 2201 bytes --]

diff --git a/git-svn.perl b/git-svn.perl
index 2e0e552..1975b62 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3340,6 +3340,7 @@ sub new {
 	$self->{rm} = { };
 	$self->{path_prefix} = length $self->{svn_path} ?
 	                       "$self->{svn_path}/" : '';
+	$self->{config} = $opts->{config};
 	return $self;
 }
 
@@ -3528,6 +3529,51 @@ sub ensure_path {
 	return $bat->{$c};
 }
 
+# Subroutine to convert a globbing pattern to a regular expression.
+# From perl cookbook.
+sub glob2pat {
+	my $globstr = shift;
+	my %patmap = ('*' => '.*', '?' => '.', '[' => '[', ']' => ']');
+	$globstr =~ s{(.)} { $patmap{$1} || "\Q$1" }ge;
+	return '^' . $globstr . '$';
+}
+
+sub check_autoprop {
+	my ($self, $pattern, $properties, $file, $fbat) = @_;
+	# Convert the globbing pattern to a regular expression.
+	my $regex = glob2pat($pattern);
+	# Check if the pattern matches the file name.
+	if($file =~ m/($regex)/) {
+		# Parse the list of properties to set.
+		my @props = split(/;/, $properties);
+		foreach my $prop (@props) {
+			# Parse 'name=value' syntax and set the property.
+			if ($prop =~ /([^=]+)=(.*)/) {
+				$self->change_file_prop($fbat, $1, $2);
+			}
+		}
+	}
+}
+
+sub apply_autoprops {
+	my ($self, $file, $fbat) = @_;
+	my $conf_t = ${$self->{config}}{'config'};
+	no warnings 'once';
+	# Check [miscellany]/enable-auto-props in svn configuration.
+	if (SVN::_Core::svn_config_get_bool($conf_t,
+					    $SVN::_Core::SVN_CONFIG_SECTION_MISCELLANY,
+					    $SVN::_Core::SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS,
+					    0)) {
+		# Auto-props are enabled.  Enumerate them to look for matches.
+		my $callback = sub {
+			$self->check_autoprop($_[0], $_[1], $file, $fbat);
+		};
+		SVN::_Core::svn_config_enumerate($conf_t,
+						 $SVN::_Core::SVN_CONFIG_SECTION_AUTO_PROPS,
+						 $callback);
+	}
+}
+
 sub A {
 	my ($self, $m) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -3535,6 +3581,7 @@ sub A {
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
+        $self->apply_autoprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }

Re: [PATCH 1/2] git-svn: teach dcommit about svn auto-props

[Eric Wong]

Brad King <brad.king@kitware.com> wrote:
> 
> Subversion repositories often require files to have properties such as
> svn:mime-type and svn:eol-style set when they are added.  Users
> typically set these properties automatically using the SVN auto-props
> feature with 'svn add'.  This commit teaches dcommit to look at the user
> SVN configuration and apply matching auto-props entries for files added
> by a diff as it is applied to the SVN remote.  A later commit will make
> this feature optional.
> 
> Signed-off-by: Brad King <brad.king@kitware.com>

Hi Brad,

I like this patch.  Can we get an automated test of this functionality?
We can (and probably should) set $HOME for the test and ignore the
existing ~/.subversion/config of the user.

Also, some minor nitpicks on whitespace/formatting inline below.

Not sure if writing a new unit test will trigger that bug below for you.
It really shouldn't...

> ---
> This change honors the user's enable-auto-props svn config setting.
> The next patch will configure this at the git level and add the
> corresponding documentation.
> 
> I've tested this by hand on an real SVN repo that checks for mime type.
> Unfortunately I'm unable to run the git-svn test suite because I get
> the error reported here:
> 
>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=486527
>
> (even without my changes).

I haven't had the chance to look at this.   Can anybody else shed more
light on that bug?  It's really strange that the tests won't run because
of it.  Are you unable to run some git-svn tests or all of them?

<snip>

> +sub apply_autoprops {
> +	my ($self, $file, $fbat) = @_;
> +	my $conf_t = ${$self->{config}}{'config'};
> +	no warnings 'once';
> +	# Check [miscellany]/enable-auto-props in svn configuration.
> +	if (SVN::_Core::svn_config_get_bool($conf_t,
> +					    $SVN::_Core::SVN_CONFIG_SECTION_MISCELLANY,
> +					    $SVN::_Core::SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS,

Long lines here and below.  I'd rather just align to the left (tabs are
assumed to be 8 characters wide on screen).

> +					    0)) {
> +		# Auto-props are enabled.  Enumerate them to look for matches.
> +		my $callback = sub {
> +			$self->check_autoprop($_[0], $_[1], $file, $fbat);
> +		};
> +		SVN::_Core::svn_config_enumerate($conf_t,
> +						 $SVN::_Core::SVN_CONFIG_SECTION_AUTO_PROPS,
> +						 $callback);
> +	}
> +}
> +
>  sub A {
>  	my ($self, $m) = @_;
>  	my ($dir, $file) = split_path($m->{file_b});
> @@ -3535,6 +3581,7 @@ sub A {
>  	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
>  					undef, -1);
>  	print "\tA\t$m->{file_b}\n" unless $::_q;
> +        $self->apply_autoprops($file, $fbat);

Hard tabs are used for indentation.

Thanks!

-- 
Eric Wong

[PATCH] git-svn: teach dcommit about svn auto-props

[Brad King]

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


Subversion repositories often require files to have properties such as
svn:mime-type and svn:eol-style set when they are added.  Users
typically set these properties automatically using the SVN auto-props
feature with 'svn add'.  This commit teaches dcommit to look at the user
SVN configuration and apply matching auto-props entries for files added
by a diff as it is applied to the SVN remote.

Signed-off-by: Brad King <brad.king@kitware.com>
---
Eric Wong wrote:
> I like this patch.

Thanks.

> Can we get an automated test of this functionality?

This patch adds a test.  I also fixed the property name/value parsing
to remove leading and trailing whitespace.

> We can (and probably should) set $HOME for the test and ignore the
> existing ~/.subversion/config of the user.

I used the --config-dir option.

> Also, some minor nitpicks on whitespace/formatting inline below.

Addressed.  I missed the wrong indentation before because my second patch
removed it.

> I haven't had the chance to look at this.   Can anybody else shed more
> light on that bug?  It's really strange that the tests won't run because
> of it.  Are you unable to run some git-svn tests or all of them?

Just that one fails.  All others (including the one in the patch below) pass.

Thanks for reviewing,
-Brad

 git-svn.perl                          |   52 ++++++++++++++++++++
 t/t9124-git-svn-dcommit-auto-props.sh |   84 +++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 0 deletions(-)
 create mode 100755 t/t9124-git-svn-dcommit-auto-props.sh


[-- Attachment #2: a13a0b82dc4d8a4d45a94bdc58a60b605debe17a.diff --]
[-- Type: text/x-patch, Size: 4794 bytes --]

diff --git a/git-svn.perl b/git-svn.perl
index 2e0e552..0a8e907 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3340,6 +3340,7 @@ sub new {
 	$self->{rm} = { };
 	$self->{path_prefix} = length $self->{svn_path} ?
 	                       "$self->{svn_path}/" : '';
+	$self->{config} = $opts->{config};
 	return $self;
 }
 
@@ -3528,6 +3529,56 @@ sub ensure_path {
 	return $bat->{$c};
 }
 
+# Subroutine to convert a globbing pattern to a regular expression.
+# From perl cookbook.
+sub glob2pat {
+	my $globstr = shift;
+	my %patmap = ('*' => '.*', '?' => '.', '[' => '[', ']' => ']');
+	$globstr =~ s{(.)} { $patmap{$1} || "\Q$1" }ge;
+	return '^' . $globstr . '$';
+}
+
+sub check_autoprop {
+	my ($self, $pattern, $properties, $file, $fbat) = @_;
+	# Convert the globbing pattern to a regular expression.
+	my $regex = glob2pat($pattern);
+	# Check if the pattern matches the file name.
+	if($file =~ m/($regex)/) {
+		# Parse the list of properties to set.
+		my @props = split(/;/, $properties);
+		foreach my $prop (@props) {
+			# Parse 'name=value' syntax and set the property.
+			if ($prop =~ /([^=]+)=(.*)/) {
+				my ($n,$v) = ($1,$2);
+				$n =~ s/^\s+//; $n =~ s/\s+$//;
+				$v =~ s/^\s+//; $v =~ s/\s+$//;
+				$self->change_file_prop($fbat, $n, $v);
+			}
+		}
+	}
+}
+
+sub apply_autoprops {
+	my ($self, $file, $fbat) = @_;
+	my $conf_t = ${$self->{config}}{'config'};
+	no warnings 'once';
+	# Check [miscellany]/enable-auto-props in svn configuration.
+	if (SVN::_Core::svn_config_get_bool(
+		$conf_t,
+		$SVN::_Core::SVN_CONFIG_SECTION_MISCELLANY,
+		$SVN::_Core::SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS,
+		0)) {
+		# Auto-props are enabled.  Enumerate them to look for matches.
+		my $callback = sub {
+			$self->check_autoprop($_[0], $_[1], $file, $fbat);
+		};
+		SVN::_Core::svn_config_enumerate(
+			$conf_t,
+			$SVN::_Core::SVN_CONFIG_SECTION_AUTO_PROPS,
+			$callback);
+	}
+}
+
 sub A {
 	my ($self, $m) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -3535,6 +3586,7 @@ sub A {
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
+	$self->apply_autoprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
diff --git a/t/t9124-git-svn-dcommit-auto-props.sh b/t/t9124-git-svn-dcommit-auto-props.sh
new file mode 100755
index 0000000..beefbcc
--- /dev/null
+++ b/t/t9124-git-svn-dcommit-auto-props.sh
@@ -0,0 +1,84 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Brad King
+
+test_description='git-svn dcommit honors auto-props'
+
+. ./lib-git-svn.sh
+
+generate_auto_props() {
+cat << EOF
+[miscellany]
+enable-auto-props=$1
+[auto-props]
+*.sh  = svn:mime-type=application/x-shellscript; svn:eol-style=LF
+*.txt = svn:mime-type=text/plain; svn:eol-style = native
+EOF
+}
+
+test_expect_success 'initialize git-svn' '
+	mkdir import &&
+	cd import &&
+	echo foo > foo &&
+	svn import -m "import for git-svn" . "$svnrepo" >/dev/null &&
+	cd .. &&
+	rm -rf import &&
+	git-svn init "$svnrepo"
+	git-svn fetch'
+
+test_expect_success 'enable auto-props config' '
+	cd "$gittestrepo" &&
+	mkdir user &&
+	generate_auto_props yes > user/config
+	'
+
+test_expect_success 'add files matching auto-props' '
+	cd "$gittestrepo" &&
+	echo "#!/bin/sh" > exec1.sh &&
+	chmod +x exec1.sh &&
+	echo "hello" > hello.txt &&
+	echo bar > bar &&
+	git add exec1.sh hello.txt bar &&
+	git commit -m "files for enabled auto-props" &&
+	git svn dcommit --config-dir=user
+	'
+
+test_expect_success 'disable auto-props config' '
+	cd "$gittestrepo" &&
+	generate_auto_props no > user/config
+	'
+
+test_expect_success 'add files matching disabled auto-props' '
+	cd "$gittestrepo" &&
+	echo "#!/bin/sh" > exec2.sh &&
+	chmod +x exec2.sh &&
+	echo "world" > world.txt &&
+	echo zot > zot &&
+	git add exec2.sh world.txt zot &&
+	git commit -m "files for disabled auto-props" &&
+	git svn dcommit --config-dir=user
+	'
+
+test_expect_success 'check resulting svn repository' '
+	mkdir work &&
+	cd work &&
+	svn co "$svnrepo" &&
+	cd svnrepo &&
+
+	# Check properties from first commit.
+	test "x$(svn propget svn:executable exec1.sh)" = "x*" &&
+	test "x$(svn propget svn:mime-type exec1.sh)" = \
+	     "xapplication/x-shellscript" &&
+	test "x$(svn propget svn:mime-type hello.txt)" = "xtext/plain" &&
+	test "x$(svn propget svn:eol-style hello.txt)" = "xnative" &&
+	test "x$(svn propget svn:mime-type bar)" = "x" &&
+
+	# Check properties from second commit.
+	test "x$(svn propget svn:executable exec2.sh)" = "x*" &&
+	test "x$(svn propget svn:mime-type exec2.sh)" = "x" &&
+	test "x$(svn propget svn:mime-type world.txt)" = "x" &&
+	test "x$(svn propget svn:eol-style world.txt)" = "x" &&
+	test "x$(svn propget svn:mime-type zot)" = "x"
+	'
+
+test_done

Re: [PATCH] git-svn: teach dcommit about svn auto-props

[Eric Wong]

Brad King <brad.king@kitware.com> wrote:
> 
> Subversion repositories often require files to have properties such as
> svn:mime-type and svn:eol-style set when they are added.  Users
> typically set these properties automatically using the SVN auto-props
> feature with 'svn add'.  This commit teaches dcommit to look at the user
> SVN configuration and apply matching auto-props entries for files added
> by a diff as it is applied to the SVN remote.
> 
> Signed-off-by: Brad King <brad.king@kitware.com>

Thanks Brad,

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

> ---
> Eric Wong wrote:
> > I like this patch.
> 
> Thanks.
> 
> > Can we get an automated test of this functionality?
> 
> This patch adds a test.  I also fixed the property name/value parsing
> to remove leading and trailing whitespace.
> 
> > We can (and probably should) set $HOME for the test and ignore the
> > existing ~/.subversion/config of the user.
> 
> I used the --config-dir option.
> 
> > Also, some minor nitpicks on whitespace/formatting inline below.
> 
> Addressed.  I missed the wrong indentation before because my second patch
> removed it.
> 
> > I haven't had the chance to look at this.   Can anybody else shed more
> > light on that bug?  It's really strange that the tests won't run because
> > of it.  Are you unable to run some git-svn tests or all of them?
> 
> Just that one fails.  All others (including the one in the patch below) pass.

Exactly which test fails for you?  Perhaps it's some setting in your
~/.subversion/config that's causing it to fail.  Maybe we should set
$HOME and use a clean ~/.subversion/config for git-svn tests regardless
if that turns out to be the case...

> Thanks for reviewing,
> -Brad
> 
>  git-svn.perl                          |   52 ++++++++++++++++++++
>  t/t9124-git-svn-dcommit-auto-props.sh |   84 +++++++++++++++++++++++++++++++++
>  2 files changed, 136 insertions(+), 0 deletions(-)
>  create mode 100755 t/t9124-git-svn-dcommit-auto-props.sh
> 

> diff --git a/git-svn.perl b/git-svn.perl
> index 2e0e552..0a8e907 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3340,6 +3340,7 @@ sub new {
>  	$self->{rm} = { };
>  	$self->{path_prefix} = length $self->{svn_path} ?
>  	                       "$self->{svn_path}/" : '';
> +	$self->{config} = $opts->{config};
>  	return $self;
>  }
>  
> @@ -3528,6 +3529,56 @@ sub ensure_path {
>  	return $bat->{$c};
>  }
>  
> +# Subroutine to convert a globbing pattern to a regular expression.
> +# From perl cookbook.
> +sub glob2pat {
> +	my $globstr = shift;
> +	my %patmap = ('*' => '.*', '?' => '.', '[' => '[', ']' => ']');
> +	$globstr =~ s{(.)} { $patmap{$1} || "\Q$1" }ge;
> +	return '^' . $globstr . '$';
> +}
> +
> +sub check_autoprop {
> +	my ($self, $pattern, $properties, $file, $fbat) = @_;
> +	# Convert the globbing pattern to a regular expression.
> +	my $regex = glob2pat($pattern);
> +	# Check if the pattern matches the file name.
> +	if($file =~ m/($regex)/) {
> +		# Parse the list of properties to set.
> +		my @props = split(/;/, $properties);
> +		foreach my $prop (@props) {
> +			# Parse 'name=value' syntax and set the property.
> +			if ($prop =~ /([^=]+)=(.*)/) {
> +				my ($n,$v) = ($1,$2);
> +				$n =~ s/^\s+//; $n =~ s/\s+$//;
> +				$v =~ s/^\s+//; $v =~ s/\s+$//;
> +				$self->change_file_prop($fbat, $n, $v);
> +			}
> +		}
> +	}
> +}
> +
> +sub apply_autoprops {
> +	my ($self, $file, $fbat) = @_;
> +	my $conf_t = ${$self->{config}}{'config'};
> +	no warnings 'once';
> +	# Check [miscellany]/enable-auto-props in svn configuration.
> +	if (SVN::_Core::svn_config_get_bool(
> +		$conf_t,
> +		$SVN::_Core::SVN_CONFIG_SECTION_MISCELLANY,
> +		$SVN::_Core::SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS,
> +		0)) {
> +		# Auto-props are enabled.  Enumerate them to look for matches.
> +		my $callback = sub {
> +			$self->check_autoprop($_[0], $_[1], $file, $fbat);
> +		};
> +		SVN::_Core::svn_config_enumerate(
> +			$conf_t,
> +			$SVN::_Core::SVN_CONFIG_SECTION_AUTO_PROPS,
> +			$callback);
> +	}
> +}
> +
>  sub A {
>  	my ($self, $m) = @_;
>  	my ($dir, $file) = split_path($m->{file_b});
> @@ -3535,6 +3586,7 @@ sub A {
>  	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
>  					undef, -1);
>  	print "\tA\t$m->{file_b}\n" unless $::_q;
> +	$self->apply_autoprops($file, $fbat);
>  	$self->chg_file($fbat, $m);
>  	$self->close_file($fbat,undef,$self->{pool});
>  }
> diff --git a/t/t9124-git-svn-dcommit-auto-props.sh b/t/t9124-git-svn-dcommit-auto-props.sh
> new file mode 100755
> index 0000000..beefbcc
> --- /dev/null
> +++ b/t/t9124-git-svn-dcommit-auto-props.sh
> @@ -0,0 +1,84 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Brad King
> +
> +test_description='git-svn dcommit honors auto-props'
> +
> +. ./lib-git-svn.sh
> +
> +generate_auto_props() {
> +cat << EOF
> +[miscellany]
> +enable-auto-props=$1
> +[auto-props]
> +*.sh  = svn:mime-type=application/x-shellscript; svn:eol-style=LF
> +*.txt = svn:mime-type=text/plain; svn:eol-style = native
> +EOF
> +}
> +
> +test_expect_success 'initialize git-svn' '
> +	mkdir import &&
> +	cd import &&
> +	echo foo > foo &&
> +	svn import -m "import for git-svn" . "$svnrepo" >/dev/null &&
> +	cd .. &&
> +	rm -rf import &&
> +	git-svn init "$svnrepo"
> +	git-svn fetch'
> +
> +test_expect_success 'enable auto-props config' '
> +	cd "$gittestrepo" &&
> +	mkdir user &&
> +	generate_auto_props yes > user/config
> +	'
> +
> +test_expect_success 'add files matching auto-props' '
> +	cd "$gittestrepo" &&
> +	echo "#!/bin/sh" > exec1.sh &&
> +	chmod +x exec1.sh &&
> +	echo "hello" > hello.txt &&
> +	echo bar > bar &&
> +	git add exec1.sh hello.txt bar &&
> +	git commit -m "files for enabled auto-props" &&
> +	git svn dcommit --config-dir=user
> +	'
> +
> +test_expect_success 'disable auto-props config' '
> +	cd "$gittestrepo" &&
> +	generate_auto_props no > user/config
> +	'
> +
> +test_expect_success 'add files matching disabled auto-props' '
> +	cd "$gittestrepo" &&
> +	echo "#!/bin/sh" > exec2.sh &&
> +	chmod +x exec2.sh &&
> +	echo "world" > world.txt &&
> +	echo zot > zot &&
> +	git add exec2.sh world.txt zot &&
> +	git commit -m "files for disabled auto-props" &&
> +	git svn dcommit --config-dir=user
> +	'
> +
> +test_expect_success 'check resulting svn repository' '
> +	mkdir work &&
> +	cd work &&
> +	svn co "$svnrepo" &&
> +	cd svnrepo &&
> +
> +	# Check properties from first commit.
> +	test "x$(svn propget svn:executable exec1.sh)" = "x*" &&
> +	test "x$(svn propget svn:mime-type exec1.sh)" = \
> +	     "xapplication/x-shellscript" &&
> +	test "x$(svn propget svn:mime-type hello.txt)" = "xtext/plain" &&
> +	test "x$(svn propget svn:eol-style hello.txt)" = "xnative" &&
> +	test "x$(svn propget svn:mime-type bar)" = "x" &&
> +
> +	# Check properties from second commit.
> +	test "x$(svn propget svn:executable exec2.sh)" = "x*" &&
> +	test "x$(svn propget svn:mime-type exec2.sh)" = "x" &&
> +	test "x$(svn propget svn:mime-type world.txt)" = "x" &&
> +	test "x$(svn propget svn:eol-style world.txt)" = "x" &&
> +	test "x$(svn propget svn:mime-type zot)" = "x"
> +	'
> +
> +test_done

-- 
Eric Wong

Re: [PATCH] git-svn: teach dcommit about svn auto-props

[Brad King]

Eric Wong wrote:
> Brad King <brad.king@kitware.com> wrote:
>> Signed-off-by: Brad King <brad.king@kitware.com>
> 
> Acked-by: Eric Wong <normalperson@yhbt.net>

Great, thanks!

>> ---
>> Eric Wong wrote:
>>> I haven't had the chance to look at this.   Can anybody else shed more
>>> light on that bug?  It's really strange that the tests won't run because
>>> of it.  Are you unable to run some git-svn tests or all of them?
>> Just that one fails.  All others (including the one in the patch below) pass.
> 
> Exactly which test fails for you?  Perhaps it's some setting in your
> ~/.subversion/config that's causing it to fail.  Maybe we should set
> $HOME and use a clean ~/.subversion/config for git-svn tests regardless
> if that turns out to be the case...

$ cd $gitsrc/t
$ export SVNSERVE_PORT=5432
$ ./t9113-git-svn-dcommit-new-file.sh
*   ok 1: start tracking an empty repo
* FAIL 2: create files in new directory with dcommit

                mkdir git-new-dir &&
                echo hello > git-new-dir/world &&
                git update-index --add git-new-dir/world &&
                git commit -m hello &&
                start_svnserve &&
                git svn dcommit

* failed 1 among 2 test(s)

I hacked the test script to log the dcommit output to a file, and I see
this:

Committing to svn://127.0.0.1:5432 ...
Use of uninitialized value in concatenation (.) or string at
/usr/lib/perl5/SVN/Core.pm line 584.
Authorization failed:  at $gitsrc/t/../git-svn line 3329

(I replaced my git source dir full path with $gitsrc).

The version of libsvn-perl is:  1.5.0dfsg1-4

Please let me know if you need more info.

I tried moving my ~/.subversion/config out of the way but it makes no
difference.  However, I agree we should block the user's home svn config
when running other dcommit tests now that we have auto-props.  Perhaps
just using the --config-dir option with an empty directory would be enough.

-Brad

[RFH] - git-svn auth bug (possibly SVN 1.5.0-related)

[Eric Wong]

Brad King <brad.king@kitware.com> wrote:
> Eric Wong wrote:
> > Brad King <brad.king@kitware.com> wrote:
> >> Eric Wong wrote:
> >>> I haven't had the chance to look at this.   Can anybody else shed more
> >>> light on that bug?  It's really strange that the tests won't run because
> >>> of it.  Are you unable to run some git-svn tests or all of them?
> >> Just that one fails.  All others (including the one in the patch below) pass.
> > 
> > Exactly which test fails for you?  Perhaps it's some setting in your
> > ~/.subversion/config that's causing it to fail.  Maybe we should set
> > $HOME and use a clean ~/.subversion/config for git-svn tests regardless
> > if that turns out to be the case...
> 
> $ cd $gitsrc/t
> $ export SVNSERVE_PORT=5432
> $ ./t9113-git-svn-dcommit-new-file.sh
> *   ok 1: start tracking an empty repo
> * FAIL 2: create files in new directory with dcommit
> 
>                 mkdir git-new-dir &&
>                 echo hello > git-new-dir/world &&
>                 git update-index --add git-new-dir/world &&
>                 git commit -m hello &&
>                 start_svnserve &&
>                 git svn dcommit
> 
> * failed 1 among 2 test(s)
> 
> I hacked the test script to log the dcommit output to a file, and I see
> this:
> 
> Committing to svn://127.0.0.1:5432 ...
> Use of uninitialized value in concatenation (.) or string at
> /usr/lib/perl5/SVN/Core.pm line 584.
> Authorization failed:  at $gitsrc/t/../git-svn line 3329
> 
> (I replaced my git source dir full path with $gitsrc).
> 
> The version of libsvn-perl is:  1.5.0dfsg1-4

It could be another incompatibility introduced in SVN 1.5.0.

I'll try to dist-upgrade a machine to Lenny sometime in the next two
weeks so I can test; I'm pretty busy these days but if anybody else
wants to figure this out in the meantime, please do :)

> Please let me know if you need more info.
> 
> I tried moving my ~/.subversion/config out of the way but it makes no
> difference.  However, I agree we should block the user's home svn config
> when running other dcommit tests now that we have auto-props.  Perhaps
> just using the --config-dir option with an empty directory would be enough.

Thanks for the additional info!

-- 
Eric Wong

Re: [RFH] - git-svn auth bug (possibly SVN 1.5.0-related)

[Dmitry Potapov]

On Sun, Aug 03, 2008 at 03:02:51PM -0700, Eric Wong wrote:
> 
> It could be another incompatibility introduced in SVN 1.5.0.
> 
> I'll try to dist-upgrade a machine to Lenny sometime in the next two
> weeks so I can test; I'm pretty busy these days but if anybody else
> wants to figure this out in the meantime, please do :)

The problem happens only if you use FS format 3 regardless what version
of SVN perl binding you use. Also, there is no problem with using SVN
1.5 (I tried 1.5.1) as long as you use FS format 2 (I tested git-svn
with libsvn-perl 1.4.2 and 1.5.1 works fine) but if your repository is
initialized to use FS format 3 (which is the default in SVN 1.5) then
this problem happens with all versions of SVN perl binding, and it
happens exactly in the same place:

 /home/dpotapov/git/git-svn:3333:    my @ce  = $opts->{ra}->get_commit_editor($opts->{log},
sub get_commit_editor {
 /home/dpotapov/git/git-svn:3909:    my ($self, $log, $cb, $pool) = @_;
 /home/dpotapov/git/git-svn:3910:    my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
 /home/dpotapov/git/git-svn:3911:    $self->SUPER::get_commit_editor($log, $cb, @lock, $pool);
sub AUTOLOAD {
 /usr/lib/perl5/SVN/Ra.pm:74:     my $class = ref($_[0]);
 /usr/lib/perl5/SVN/Ra.pm:75:     my $method = $AUTOLOAD;
 /usr/lib/perl5/SVN/Ra.pm:76:     $method =~ s/.*:://;
 /usr/lib/perl5/SVN/Ra.pm:77:     return unless $method =~ m/[^A-Z]/;
 /usr/lib/perl5/SVN/Ra.pm:79:     my $self = shift;
 /usr/lib/perl5/SVN/Ra.pm:82:     my $func = $self->{session}->can ($method)
 /usr/lib/perl5/SVN/Ra.pm:85:     my @ret = $func->($self->{session}, @_);

Numbers of lines may be different for different versions of libsvn-perl,
but the effect is exactly the same. Instead of going to the next line
and completing AUTOLOAD, if you use FS format 3 then you end up in
croak_on_error(), which uses some uninitialized value in string
concatenation (which produces an additional warning) and then calls
croak(). End of the story :(


Dmitry

Re: [RFH] - git-svn auth bug (possibly SVN 1.5.0-related)

[Dmitry Potapov]

On Mon, Aug 04, 2008 at 06:18:20PM +0400, Dmitry Potapov wrote:
> Numbers of lines may be different for different versions of libsvn-perl,
> but the effect is exactly the same. Instead of going to the next line
> and completing AUTOLOAD, if you use FS format 3 then you end up in
> croak_on_error(), which uses some uninitialized value in string
> concatenation (which produces an additional warning) and then calls
> croak(). End of the story :(

I think I have figured out that is wrong. It is a bug in initialization
of SVN database. Before, there was only one [general] section in the
conf/svnserve.conf file and the procedure of initialization apparently
copied a template and added the following string to the end of file:

anon-access = write

but now there are two sections: [general] and [sasl]

as result "anon-access = write" is added to the wrong section,
and there is no anonymous access anymore. So, the test fails.


Dmitry

[PATCH] correct access right for git-svn-dcommit test

[Dmitry Potapov]

The tests requires anonymous write access. Therefore, "anon-access =
write" is added to conf/svnserve.conf. But because it was added to
the end of the file, it is impossible to guarantee in what section
it will be located. It turned out that on SVN 1.5, it was placed in
the wrong section and as result the test failed.

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
 t/t9113-git-svn-dcommit-new-file.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/t/t9113-git-svn-dcommit-new-file.sh b/t/t9113-git-svn-dcommit-new-file.sh
index 8da8ce5..ae78e33 100755
--- a/t/t9113-git-svn-dcommit-new-file.sh
+++ b/t/t9113-git-svn-dcommit-new-file.sh
@@ -28,6 +28,7 @@ start_svnserve () {
 
 test_expect_success 'start tracking an empty repo' '
 	svn mkdir -m "empty dir" "$svnrepo"/empty-dir &&
+	echo "[general]" > "$rawsvnrepo"/conf/svnserve.conf &&
 	echo anon-access = write >> "$rawsvnrepo"/conf/svnserve.conf &&
 	start_svnserve &&
 	git svn init svn://127.0.0.1:$SVNSERVE_PORT &&
-- 
1.6.0.rc1.74.g1e94

Re: [PATCH] correct access right for git-svn-dcommit test

[Brad King]

Dmitry Potapov wrote:
> The tests requires anonymous write access. Therefore, "anon-access =
> write" is added to conf/svnserve.conf. But because it was added to
> the end of the file, it is impossible to guarantee in what section
> it will be located. It turned out that on SVN 1.5, it was placed in
> the wrong section and as result the test failed.
> 
> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
> ---
>  t/t9113-git-svn-dcommit-new-file.sh |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/t/t9113-git-svn-dcommit-new-file.sh b/t/t9113-git-svn-dcommit-new-file.sh
> index 8da8ce5..ae78e33 100755
> --- a/t/t9113-git-svn-dcommit-new-file.sh
> +++ b/t/t9113-git-svn-dcommit-new-file.sh
> @@ -28,6 +28,7 @@ start_svnserve () {
>  
>  test_expect_success 'start tracking an empty repo' '
>  	svn mkdir -m "empty dir" "$svnrepo"/empty-dir &&
> +	echo "[general]" > "$rawsvnrepo"/conf/svnserve.conf &&
>  	echo anon-access = write >> "$rawsvnrepo"/conf/svnserve.conf &&
>  	start_svnserve &&
>  	git svn init svn://127.0.0.1:$SVNSERVE_PORT &&

I just tried this patch and it does fix the test on my system.

Thanks,
-Brad

Re: [PATCH] correct access right for git-svn-dcommit test

[Eric Wong]

Dmitry Potapov <dpotapov@gmail.com> wrote:
> The tests requires anonymous write access. Therefore, "anon-access =
> write" is added to conf/svnserve.conf. But because it was added to
> the end of the file, it is impossible to guarantee in what section
> it will be located. It turned out that on SVN 1.5, it was placed in
> the wrong section and as result the test failed.
> 
> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>

Brad and Dmitry: Thank you both very much.

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

> ---
>  t/t9113-git-svn-dcommit-new-file.sh |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/t/t9113-git-svn-dcommit-new-file.sh b/t/t9113-git-svn-dcommit-new-file.sh
> index 8da8ce5..ae78e33 100755
> --- a/t/t9113-git-svn-dcommit-new-file.sh
> +++ b/t/t9113-git-svn-dcommit-new-file.sh
> @@ -28,6 +28,7 @@ start_svnserve () {
>  
>  test_expect_success 'start tracking an empty repo' '
>  	svn mkdir -m "empty dir" "$svnrepo"/empty-dir &&
> +	echo "[general]" > "$rawsvnrepo"/conf/svnserve.conf &&
>  	echo anon-access = write >> "$rawsvnrepo"/conf/svnserve.conf &&
>  	start_svnserve &&
>  	git svn init svn://127.0.0.1:$SVNSERVE_PORT &&
> -- 
> 1.6.0.rc1.74.g1e94