threads / patch / 14581

patch, 2 partsgit-svn: teach dcommit about svn auto-props

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

## tl;dr

11 messages between Jul 21, 2008 and Aug 5, 2008. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Brad King· Jul 21, 2008, 21:40 UTC · lore

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(-)
Show changes to git-svn.perl +47 −0
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});
 }
Eric Wong· Jul 25, 2008, 06:00 UTC · re: Brad King · lore

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

Brad King <brad.king@kitware.com> wrote:
Show 10 quoted lines
> 
> 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...

Show 12 quoted lines
> ---
> 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>
Show 8 quoted lines
> +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).

Show 19 quoted lines
> +					    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
Brad King· Jul 25, 2008, 15:32 UTC · re: Eric Wong · lore

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

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
Show changes to 2 files +136 −0

git-svn.perl, 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· Jul 26, 2008, 05:45 UTC · re: Brad King · lore

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

Brad King <brad.king@kitware.com> wrote:
Show 9 quoted lines
> 
> 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>
Show 26 quoted lines
> ---
> 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...

Show 8 quoted lines
> 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
> 
Show 167 quoted lines
> 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
Brad King· Jul 26, 2008, 14:08 UTC · re: Eric Wong · lore

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

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!
Show 11 quoted lines
>> ---
>> 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
Eric Wong· Aug 3, 2008, 22:02 UTC · re: Brad King · lore

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

Brad King <brad.king@kitware.com> wrote:
Show 39 quoted lines
> 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 :)

Show 6 quoted lines
> 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
Dmitry Potapov· Aug 4, 2008, 14:18 UTC · re: Eric Wong · lore

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

On Sun, Aug 03, 2008 at 03:02:51PM -0700, Eric Wong wrote:
Show 6 quoted lines
> 
> 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
Dmitry Potapov· Aug 4, 2008, 15:14 UTC · re: Dmitry Potapov · lore

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

On Mon, Aug 04, 2008 at 06:18:20PM +0400, Dmitry Potapov wrote:
Show 6 quoted lines
> 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
Dmitry Potapov· Aug 4, 2008, 15:30 UTC · re: Dmitry Potapov · lore

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

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(-)
Show changes to t/t9113-git-svn-dcommit-new-file.sh +1 −0
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
Brad King· Aug 4, 2008, 15:44 UTC · re: Dmitry Potapov · lore

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

Dmitry Potapov wrote:
Show 23 quoted lines
> 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

Eric Wong· Aug 5, 2008, 03:18 UTC · re: Dmitry Potapov · lore

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

Dmitry Potapov <dpotapov@gmail.com> wrote:
Show 7 quoted lines
> 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>
Show 18 quoted lines
> ---
>  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

← back to recent threads