Re: [PATCH] git-svn: Support for git-svn propset
> On Dec 8, 2014, at 1:36 PM, Eric Wong wrote:
>
> Alfred Perlstein wrote:
>> Appearing here:
>> http://marc.info/?l=git&m=125259772625008&w=2
>
> Probably better to use a mid URL here, too
>
> http://mid.gmane.org/1927112650.1281253084529659.javamail.r...@klofta.sjsoft.com
>
> such a long URL, though...
>
>> --- a/perl/Git/SVN/Editor.pm
>> +++ b/perl/Git/SVN/Editor.pm
>> @@ -288,6 +288,44 @@ sub apply_autoprops {
>>}
>> }
>>
>> +sub check_attr {
>> +my ($attr,$path) = @_;
>> +my $fh = command_output_pipe("check-attr", $attr, "--", $path);
>> +return undef if (!$fh);
>> +
>> +my $val = <$fh>;
>> +close $fh;
>> +if ($val) { $val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/; }
>> +return $val;
>> +}
>
> I just noticed command_output_pipe didn't use a corresponding
> command_close_pipe to check for errors, but command_oneline is even
> better. I'll squash the following:
>
> --- a/perl/Git/SVN/Editor.pm
> +++ b/perl/Git/SVN/Editor.pm
> @@ -290,11 +290,7 @@ sub apply_autoprops {
>
> sub check_attr {
>my ($attr,$path) = @_;
> -my $fh = command_output_pipe("check-attr", $attr, "--", $path);
> -return undef if (!$fh);
> -
> -my $val = <$fh>;
> -close $fh;
> +my $val = command_oneline("check-attr", $attr, "--", $path);
>if ($val) { $val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/; }
>return $val;
> }
>
> In your test, "local" isn't portable, unfortunately, but tests seem to
> work fine without local so I've removed them:
>
> --- a/t/t9148-git-svn-propset.sh
> +++ b/t/t9148-git-svn-propset.sh
> @@ -29,10 +29,9 @@ test_expect_success 'fetch revisions from svn' '
>git svn fetch
>'
>
> -set_props()
> -{
> -local subdir="$1"
> -local file="$2"
> +set_props () {
> +subdir="$1"
> +file="$2"
>shift;shift;
>(cd "$subdir" &&
>while [ $# -gt 0 ] ; do
> @@ -43,10 +42,9 @@ set_props()
>git commit -m "testing propset" "$file")
> }
>
> -confirm_props()
> -{
> -local subdir="$1"
> -local file="$2"
> +confirm_props () {
> +subdir="$1"
> +file="$2"
>shift;shift;
>(set -e ; cd "svn_project/$subdir" &&
>while [ $# -gt 0 ] ; do
>
> Unless there's other improvements we missed, I'll push out your v3 with
> my changes squashed in for Junio to pull in a day or two. Thank you
> again for working on this!
>
Eric,
All looks good to me.
Thank you all very much for the feedback and help. It's made this a very
rewarding endeavor.
-Alfred. --
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Support for git-svn propset
Alfred Perlstein wrote:
> Appearing here:
> http://marc.info/?l=git&m=125259772625008&w=2
Probably better to use a mid URL here, too
http://mid.gmane.org/1927112650.1281253084529659.javamail.r...@klofta.sjsoft.com
such a long URL, though...
> --- a/perl/Git/SVN/Editor.pm
> +++ b/perl/Git/SVN/Editor.pm
> @@ -288,6 +288,44 @@ sub apply_autoprops {
> }
> }
>
> +sub check_attr {
> + my ($attr,$path) = @_;
> + my $fh = command_output_pipe("check-attr", $attr, "--", $path);
> + return undef if (!$fh);
> +
> + my $val = <$fh>;
> + close $fh;
> + if ($val) { $val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/; }
> + return $val;
> +}
I just noticed command_output_pipe didn't use a corresponding
command_close_pipe to check for errors, but command_oneline is even
better. I'll squash the following:
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -290,11 +290,7 @@ sub apply_autoprops {
sub check_attr {
my ($attr,$path) = @_;
- my $fh = command_output_pipe("check-attr", $attr, "--", $path);
- return undef if (!$fh);
-
- my $val = <$fh>;
- close $fh;
+ my $val = command_oneline("check-attr", $attr, "--", $path);
if ($val) { $val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/; }
return $val;
}
In your test, "local" isn't portable, unfortunately, but tests seem to
work fine without local so I've removed them:
--- a/t/t9148-git-svn-propset.sh
+++ b/t/t9148-git-svn-propset.sh
@@ -29,10 +29,9 @@ test_expect_success 'fetch revisions from svn' '
git svn fetch
'
-set_props()
-{
- local subdir="$1"
- local file="$2"
+set_props () {
+ subdir="$1"
+ file="$2"
shift;shift;
(cd "$subdir" &&
while [ $# -gt 0 ] ; do
@@ -43,10 +42,9 @@ set_props()
git commit -m "testing propset" "$file")
}
-confirm_props()
-{
- local subdir="$1"
- local file="$2"
+confirm_props () {
+ subdir="$1"
+ file="$2"
shift;shift;
(set -e ; cd "svn_project/$subdir" &&
while [ $# -gt 0 ] ; do
Unless there's other improvements we missed, I'll push out your v3 with
my changes squashed in for Junio to pull in a day or two. Thank you
again for working on this!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Support for git-svn propset
On 12/6/14, 9:42 PM, Eric Wong wrote:
Alfred Perlstein wrote:
This change allows git-svn to support setting subversion properties.
Very useful for manually setting properties when committing to a
subversion repo that *requires* properties to be set without requiring
moving your changeset to separate subversion checkout in order to
set props.
This change is initially from David Fraser
No point to obfuscate email addresses in commit messages (especially
it's also in the Signed-off-by :).
Appearing here:
http://marc.info/?l=git&m=125259772625008&w=2
They are now forward ported to most recent git along with fixes to
deal with files in subdirectories.
Style and functional changes from Eric Wong have been taken
in thier entirety from:
s/thier/their/
http://marc.info/?l=git&m=141742735608544&w=2
Fwiw, I prefer equivalent mid.gmane.org links since the message-ID
remains useful if the web server ever goes away. e.g.:
http://mid.gmane.org/20141201094911.ga13...@dcvr.yhbt.net
Reviewed-by: Eric Wong
Signed-off-by: Alfred Perlstein
Signed-off-by: David Fraser
I'd like to squash in the following changes (in order of importance):
- use && to chain commands throughout tests
- use svn_cmd wrapper throughout tests
- show $! in die messages
- favor $(...) over `...` in tests
- make new_props an array simplify building the final list
- wrap long comments (help output still needs fixing)
- remove unnecessary FIXME comment
No need to resend if you're OK with these things. Thanks again.
Hmm, I refactored tests because it bothered me that I had done all that
cut and pasting, in doing so I found and fixed a bug with uninitialized
variables in the check_attr() function.
Let me send my final diff, I will try to properly incorporate your
changes as well in that diff.
-Alfred
diff --git a/git-svn.perl b/git-svn.perl
index 5cdbf39..ec5cee4 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1392,9 +1392,9 @@ sub cmd_propset {
my $file = basename($path);
my $dn = dirname($path);
my $cur_props = Git::SVN::Editor::check_attr( "svn-properties", $path );
- my $new_props = "";
+ my @new_props;
if ($cur_props eq "unset" || $cur_props eq "" || $cur_props eq "set") {
- $new_props = "$propname=$propval";
+ push @new_props, "$propname=$propval";
} else {
# TODO: handle combining properties better
my @props = split(/;/, $cur_props);
@@ -1403,24 +1403,24 @@ sub cmd_propset {
# Parse 'name=value' syntax and set the property.
if ($prop =~ /([^=]+)=(.*)/) {
my ($n,$v) = ($1,$2);
- if ($n eq $propname)
- {
+ if ($n eq $propname) {
$v = $propval;
$replaced_prop = 1;
}
- if ($new_props eq "") { $new_props="$n=$v"; }
- else { $new_props="$new_props;$n=$v"; }
+ push @new_props, "$n=$v";
}
}
if (!$replaced_prop) {
- $new_props = "$new_props;$propname=$propval";
+ push @new_props, "$propname=$propval";
}
}
my $attrfile = "$dn/.gitattributes";
open my $attrfh, '>>', $attrfile or die "Can't open $attrfile: $!\n";
# TODO: don't simply append here if $file already has svn-properties
- print $attrfh "$file svn-properties=$new_props\n" or die "write to
$attrfile";
- close $attrfh or die "close $attrfile";
+ my $new_props = join(';', @new_props);
+ print $attrfh "$file svn-properties=$new_props\n" or
+ die "write to $attrfile: $!\n";
+ close $attrfh or die "close $attrfile: $!\n";
}
# cmd_proplist (PATH)
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index dd15318..8bed2d9 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -288,8 +288,7 @@ sub apply_autoprops {
}
}
-sub check_attr
-{
+sub check_attr {
my ($attr,$path) = @_;
my $fh = command_output_pipe("check-attr", $attr, "--", $path);
return undef if (!$fh);
@@ -306,10 +305,12 @@ sub apply_manualprops {
if ($pending_properties eq "") { return; }
# Parse the list of properties to set.
my @props = split(/;/, $pending_properties);
- # TODO: get existing properties to compare to - this fails for add so
currently not done
+ # TODO: get existing properties to compare to
+ # - this fails for add so currently not done
# my $existing_props = ::get_svnprops($file);
my $existing_props = {};
- # TODO: caching svn properties or storing them in .gitattributes would
make that fast
Re: [PATCH] git-svn: Support for git-svn propset
On Sun, Dec 7, 2014 at 3:00 AM, Torsten Bögershausen wrote:
> On 2014-12-07 06.45, Torsten Bögershausen wrote:
> []
>>> +
>>> +test_expect_success 'add multiple props' '
>>> +git svn propset svn:keywords "FreeBSD=%H" foo &&
>>> +git svn propset fbsd:nokeywords yes foo &&
>>> +echo hello >> foo &&
>>> +git commit -m "testing propset" foo &&
>>> +git svn dcommit
>>> +svn_cmd co "$svnrepo" svn_project &&
>>> +(cd svn_project && test "`svn propget svn:keywords foo`" =
>>> "FreeBSD=%H") &&
>>> +(cd svn_project && test "`svn propget fbsd:nokeywords foo`" = "yes") &&
>>> +(cd svn_project && test "`svn proplist -q foo | wc -l`" -eq 2) &&
>>> +rm -rf svn_project
>>> +'
>>> +
>> Ah, another small thing:
>> the "wc -l" will not work under Mac OS X.
>> Please see test_line_count() in t/test-lib-functions.sh
>>
> My excuse:
> I think I am wrong here and I need to correct myself after having looked at
> other TC's.
> The "wc -l" should work under Mac OS X.
More specifically: On Mac OS X, "wc -l" outputs "{whitespace}number"
which won't match "2" with the string '=' operator, however, this case
works because the '-eq' operator coerces the output of "wc -l" to a
number, which can match the 2.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Support for git-svn propset
On 2014-12-07 06.45, Torsten Bögershausen wrote: [] >> + >> +test_expect_success 'add multiple props' ' >> +git svn propset svn:keywords "FreeBSD=%H" foo && >> +git svn propset fbsd:nokeywords yes foo && >> +echo hello >> foo && >> +git commit -m "testing propset" foo && >> +git svn dcommit >> +svn_cmd co "$svnrepo" svn_project && >> +(cd svn_project && test "`svn propget svn:keywords foo`" = >> "FreeBSD=%H") && >> +(cd svn_project && test "`svn propget fbsd:nokeywords foo`" = "yes") && >> +(cd svn_project && test "`svn proplist -q foo | wc -l`" -eq 2) && >> +rm -rf svn_project >> +' >> + > Ah, another small thing: > the "wc -l" will not work under Mac OS X. > Please see test_line_count() in t/test-lib-functions.sh > My excuse: I think I am wrong here and I need to correct myself after having looked at other TC's. The "wc -l" should work under Mac OS X. Another small nit: This "`svn propget svn:keywords foo`" = "FreeBSD=%H") can be written as "$(svn propget svn:keywords foo)" = "FreeBSD=%H") (if you want to use the "Git style" for command substitution) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Support for git-svn propset
> diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
> new file mode 100755
> index 000..b36a8a2
> --- /dev/null
> +++ b/t/t9148-git-svn-propset.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2014 Alfred Perlstein
> +#
> +
> +test_description='git svn propset tests'
> +
> +. ./lib-git-svn.sh
> +
> +foo_subdir2="subdir/subdir2/foo_subdir2"
> +
In case something goes wrong (for whatever reason):
do we need a && chain here ?
> +mkdir import
> +(cd import
> + mkdir subdir
> + mkdir subdir/subdir2
> + touch foo # for 'add props top level'
"touch foo" can be written shorter:
>foo
> + touch subdir/foo_subdir # for 'add props relative'
> + touch "$foo_subdir2"# for 'add props subdir'
> + svn_cmd import -m 'import for git svn' . "$svnrepo" >/dev/null
> +)
> +rm -rf import
> +
> +test_expect_success 'initialize git svn' 'git svn init "$svnrepo"'
> +test_expect_success 'fetch revisions from svn' 'git svn fetch'
This may look a little bit strange, 2 times test_expect_success in a row,
is the indentention OK ?
> +
> +# There is a bogus feature about svn propset which means that it will only
> +# be taken as a delta for svn dcommit iff the file is also modified.
> +# That is fine for now.
"there is a bogus feature ?"
Small typo: s/iff/if/
How about this:
#The current implementation has a restriction:
#svn propset will be taken as a delta for svn dcommit only
#if the file content is also modified
> +test_expect_success 'add props top level' '
> + git svn propset svn:keywords "FreeBSD=%H" foo &&
> + echo hello >> foo &&
> + git commit -m "testing propset" foo &&
> + git svn dcommit
> + svn_cmd co "$svnrepo" svn_project &&
> + (cd svn_project && test "`svn propget svn:keywords foo`" =
> "FreeBSD=%H") &&
> + rm -rf svn_project
> + '
Is there a reason why there is no "&&" after "git svn dcommit" ?
If yes, it could be better to make this really clear to the readers and write
(This idea is stolen from Peff)
{ git svn dcommit || true } &&
> +
> +test_expect_success 'add multiple props' '
> + git svn propset svn:keywords "FreeBSD=%H" foo &&
> + git svn propset fbsd:nokeywords yes foo &&
> + echo hello >> foo &&
> + git commit -m "testing propset" foo &&
> + git svn dcommit
> + svn_cmd co "$svnrepo" svn_project &&
> + (cd svn_project && test "`svn propget svn:keywords foo`" =
> "FreeBSD=%H") &&
> + (cd svn_project && test "`svn propget fbsd:nokeywords foo`" = "yes") &&
> + (cd svn_project && test "`svn proplist -q foo | wc -l`" -eq 2) &&
> + rm -rf svn_project
> + '
> +
Ah, another small thing:
the "wc -l" will not work under Mac OS X.
Please see test_line_count() in t/test-lib-functions.sh
And thanks for improving Git
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Support for git-svn propset
Alfred Perlstein wrote:
> This change allows git-svn to support setting subversion properties.
>
> Very useful for manually setting properties when committing to a
> subversion repo that *requires* properties to be set without requiring
> moving your changeset to separate subversion checkout in order to
> set props.
>
> This change is initially from David Fraser
No point to obfuscate email addresses in commit messages (especially
it's also in the Signed-off-by :).
> Appearing here:
> http://marc.info/?l=git&m=125259772625008&w=2
>
> They are now forward ported to most recent git along with fixes to
> deal with files in subdirectories.
>
> Style and functional changes from Eric Wong have been taken
> in thier entirety from:
s/thier/their/
> http://marc.info/?l=git&m=141742735608544&w=2
Fwiw, I prefer equivalent mid.gmane.org links since the message-ID
remains useful if the web server ever goes away. e.g.:
http://mid.gmane.org/20141201094911.ga13...@dcvr.yhbt.net
> Reviewed-by: Eric Wong
> Signed-off-by: Alfred Perlstein
> Signed-off-by: David Fraser
I'd like to squash in the following changes (in order of importance):
- use && to chain commands throughout tests
- use svn_cmd wrapper throughout tests
- show $! in die messages
- favor $(...) over `...` in tests
- make new_props an array simplify building the final list
- wrap long comments (help output still needs fixing)
- remove unnecessary FIXME comment
No need to resend if you're OK with these things. Thanks again.
diff --git a/git-svn.perl b/git-svn.perl
index 5cdbf39..ec5cee4 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1392,9 +1392,9 @@ sub cmd_propset {
my $file = basename($path);
my $dn = dirname($path);
my $cur_props = Git::SVN::Editor::check_attr( "svn-properties", $path );
- my $new_props = "";
+ my @new_props;
if ($cur_props eq "unset" || $cur_props eq "" || $cur_props eq "set") {
- $new_props = "$propname=$propval";
+ push @new_props, "$propname=$propval";
} else {
# TODO: handle combining properties better
my @props = split(/;/, $cur_props);
@@ -1403,24 +1403,24 @@ sub cmd_propset {
# Parse 'name=value' syntax and set the property.
if ($prop =~ /([^=]+)=(.*)/) {
my ($n,$v) = ($1,$2);
- if ($n eq $propname)
- {
+ if ($n eq $propname) {
$v = $propval;
$replaced_prop = 1;
}
- if ($new_props eq "") { $new_props="$n=$v"; }
- else { $new_props="$new_props;$n=$v"; }
+ push @new_props, "$n=$v";
}
}
if (!$replaced_prop) {
- $new_props = "$new_props;$propname=$propval";
+ push @new_props, "$propname=$propval";
}
}
my $attrfile = "$dn/.gitattributes";
open my $attrfh, '>>', $attrfile or die "Can't open $attrfile: $!\n";
# TODO: don't simply append here if $file already has svn-properties
- print $attrfh "$file svn-properties=$new_props\n" or die "write to
$attrfile";
- close $attrfh or die "close $attrfile";
+ my $new_props = join(';', @new_props);
+ print $attrfh "$file svn-properties=$new_props\n" or
+ die "write to $attrfile: $!\n";
+ close $attrfh or die "close $attrfile: $!\n";
}
# cmd_proplist (PATH)
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index dd15318..8bed2d9 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -288,8 +288,7 @@ sub apply_autoprops {
}
}
-sub check_attr
-{
+sub check_attr {
my ($attr,$path) = @_;
my $fh = command_output_pipe("check-attr", $attr, "--", $path);
return undef if (!$fh);
@@ -306,10 +305,12 @@ sub apply_manualprops {
if ($pending_properties eq "") { return; }
# Parse the list of properties to set.
my @props = split(/;/, $pending_properties);
- # TODO: get existing properties to compare to - this fails for add so
currently not done
+ # TODO: get existing properties to compare to
+ # - this fails for add so currently not done
# my $existing_props = ::get_svnprops($file);
my $existing_props = {};
- # TODO: caching svn properties or storing them in .gitattributes would
make that faster
+ # TODO: caching svn properties or storing them in .gitattributes
+ # would make that faster
foreach my $prop (@props) {
# Parse 'name=value' syntax and set the property.
if ($prop =~ /([^=]+)=(.*)/) {
@@ -317,8 +318,6 @@ sub apply_manualprops {
Re: [PATCH] git-svn: Support for git-svn propset
Alfred Perlstein wrote:
> This change allows git-svn to support setting subversion properties.
>
> Very useful for manually setting properties when committing to a
> subversion repo that *requires* properties to be set without requiring
> moving your changeset to separate subversion checkout in order to
> set props.
Thanks Alfred, that's a good reason for supporting this feature
(something I wasn't convinced of with the original).
> This change is initially from David Fraser
> Appearing here: http://marc.info/?l=git&m=125259772625008&w=2
I've added David to the Cc: (but I never heard back from him regarding
comments from his original patch).
Alfred: I had some comments on David's original patch here that
were never addressed:
http://mid.gmane.org/20090923085812.ga20...@dcvr.yhbt.net
I suspect my concerns about .gitattributes in the source tree from
2009 are less relevant now in 2014 as git-svn seems more socially
acceptable in SVN-using projects.
Some of my other concerns about mimicking existing Perl style still
apply, and we have a Perl section in Documenting/CodingGuidelines
nowadays.
> They are now forward ported to most recent git along with fixes to
> deal with files in subdirectories.
>
> Developer's Certificate of Origin 1.1
no need for the whole DCO in the commit message.
just the S-o-b:
> Signed-off-by: Alfred Perlstein
Since this was originally written by David, his sign-off from the
original email should be here (Cc: for bigger changes)
> ---
> git-svn.perl | 50
> +-
> perl/Git/SVN/Editor.pm | 47 +++
We need a test case written for this new feature.
Most folks (including myself) who were ever involved with git-svn
rarely use it anymore, and this will likely be rarely-used.
> 2 files changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index b6e2186..91423a8 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -115,7 +115,7 @@ my ($_stdin, $_help, $_edit,
> $_before, $_after,
> $_merge, $_strategy, $_preserve_merges, $_dry_run, $_parents, $_local,
> $_prefix, $_no_checkout, $_url, $_verbose,
> - $_commit_url, $_tag, $_merge_info, $_interactive);
> + $_commit_url, $_tag, $_merge_info, $_interactive, $_set_svn_props);
>
> # This is a refactoring artifact so Git::SVN can get at this git-svn switch.
> sub opt_prefix { return $_prefix || '' }
> @@ -193,6 +193,7 @@ my %cmd = (
> 'dry-run|n' => \$_dry_run,
> 'fetch-all|all' => \$_fetch_all,
> 'commit-url=s' => \$_commit_url,
> + 'set-svn-props=s' => \$_set_svn_props,
> 'revision|r=i' => \$_revision,
> 'no-rebase' => \$_no_rebase,
> 'mergeinfo=s' => \$_merge_info,
> @@ -228,6 +229,9 @@ my %cmd = (
> 'propget' => [ \&cmd_propget,
> 'Print the value of a property on a file or directory',
> { 'revision|r=i' => \$_revision } ],
> +'propset' => [ \&cmd_propset,
> +'Set the value of a property on a file or directory -
> will be set on commit',
> +{} ],
> 'proplist' => [ \&cmd_proplist,
> 'List all properties of a file or directory',
> { 'revision|r=i' => \$_revision } ],
> @@ -1376,6 +1380,50 @@ sub cmd_propget {
> print $props->{$prop} . "\n";
> }
>
> +# cmd_propset (PROPNAME, PROPVAL, PATH)
> +#
> +# Adjust the SVN property PROPNAME to PROPVAL for PATH.
> +sub cmd_propset {
> + my ($propname, $propval, $path) = @_;
> + $path = '.' if not defined $path;
> + $path = $cmd_dir_prefix . $path;
> + usage(1) if not defined $propname;
> + usage(1) if not defined $propval;
> + my $file = basename($path);
> + my $dn = dirname($path);
> + # diff has check_attr locally, so just call direct
> + #my $current_properties = check_attr( "svn-properties", $path );
I prefer leaving out dead code entirely instead of leaving it commented out.
> + my $current_properties = Git::SVN::Editor::check_attr(
> "svn-properties", $path );
> + my $new_properties = "";
Since some lines are too long, local variables can be shortened
to cur_props, new_props without being any less descriptive.
> + if ($current_properties eq "unset" || $current_properties eq "" ||
> $current_properties eq "set") {
> + $new_properties = "$propname=$propval";
> + } else {
> + # TODO: handle combining properties better
> + my @props = split(/;/, $current_properties);
> + my $replaced_prop = 0;
> + foreach my $prop (@props) {
> + # Parse 'name=value' syntax and set the property.
> + if ($prop =~ /([^=]+)=(.*)/) {
> +
