From: Brad King <hidden> Date: 2016-06-15 22:44:59
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 <redacted>
---
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(-)
From: Eric Wong <hidden> Date: 2016-06-15 22:45:02
Brad King [off-list ref] 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 <redacted>
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).
quoted hunk
+ 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
From: Brad King <hidden> Date: 2016-06-15 22:45:02
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 <redacted>
---
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
From: Eric Wong <hidden> Date: 2016-06-15 22:45:03
Brad King [off-list ref] 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 <redacted>
Thanks Brad,
Acked-by: Eric Wong <redacted>
---
Eric Wong wrote:
quoted
I like this patch.
Thanks.
quoted
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.
quoted
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.
quoted
Also, some minor nitpicks on whitespace/formatting inline below.
Addressed. I missed the wrong indentation before because my second patch
removed it.
quoted
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...
@@ -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.+subglob2pat{+my$globstr=shift;+my%patmap=('*'=>'.*','?'=>'.','['=>'[',']'=>']');+$globstr=~s{(.)} { $patmap{$1}||"\Q$1"}ge;+return'^'.$globstr.'$';+}++subcheck_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);+foreachmy$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);+}+}+}+}++subapply_autoprops{+my($self,$file,$fbat)=@_;+my$conf_t=${$self->{config}}{'config'};+nowarnings'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);+}+}+subA{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});}
From: Brad King <hidden> Date: 2016-06-15 22:45:03
Eric Wong wrote:
Brad King [off-list ref] wrote:
quoted
Signed-off-by: Brad King <redacted>
Acked-by: Eric Wong <redacted>
Great, thanks!
quoted
---
Eric Wong wrote:
quoted
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
From: Eric Wong <hidden> Date: 2016-06-15 22:45:06
Brad King [off-list ref] wrote:
Eric Wong wrote:
quoted
Brad King [off-list ref] wrote:
quoted
Eric Wong wrote:
quoted
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.
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
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
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 <redacted>
---
t/t9113-git-svn-dcommit-new-file.sh | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
From: Brad King <hidden> Date: 2016-06-15 22:45:06
Dmitry Potapov wrote:
quoted hunk
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 <redacted>
---
t/t9113-git-svn-dcommit-new-file.sh | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
From: Eric Wong <hidden> Date: 2016-06-15 22:45:06
Dmitry Potapov [off-list ref] 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 <redacted>
Brad and Dmitry: Thank you both very much.
Acked-by: Eric Wong <redacted>