git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* Re: git svn propset and multi-line svn:externals property
  2017-03-22 23:29 98% git svn propset and multi-line svn:externals property Craig McQueen
@ 2017-03-23  3:01 98% ` Eric Wong
  0 siblings, 0 replies; 17+ results
From: Eric Wong @ 2017-03-23  3:01 UTC (permalink / raw)
  To: Craig McQueen; +Cc: git

Craig McQueen <craig.mcqueen@innerrange.com> wrote:
> Is it possible to set multi-line SVN properties somehow? Or
> could this be a future enhancement?

I'm not sure.  I don't use this feature, but it seems tied
to gitattributes(5), and I'm not sure if gitattributes supports
multi-line values, either...

^ permalink raw reply	[relevance 98%]

* git svn propset and multi-line svn:externals property
@ 2017-03-22 23:29 98% Craig McQueen
  2017-03-23  3:01 98% ` Eric Wong
  0 siblings, 1 reply; 17+ results
From: Craig McQueen @ 2017-03-22 23:29 UTC (permalink / raw)
  To: git@vger.kernel.org

I'd like to be able to set the svn:externals property using "git svn propset". However, that property can be multi-line. I've not had any success in setting this property because git svn doesn't seem to handle the newlines.

Is it possible to set multi-line SVN properties somehow? Or could this be a future enhancement?

-- 
Craig McQueen


^ permalink raw reply	[relevance 98%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-08 21:36 92% ` Eric Wong
@ 2014-12-08 23:43 99%   ` Alfred Perlstein
  0 siblings, 0 replies; 17+ results
From: Alfred Perlstein @ 2014-12-08 23:43 UTC (permalink / raw)
  To: Eric Wong
  Cc: Alfred Perlstein, git@vger.kernel.org, Jonathan Nieder,
	Junio C Hamano, Michael G. Schwern, David Fraser



> On Dec 8, 2014, at 1:36 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 
> Alfred Perlstein <alfred@freebsd.org> 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.root@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. 

^ permalink raw reply	[relevance 99%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-07 10:47 56% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
@ 2014-12-08 21:36 92% ` Eric Wong
  2014-12-08 23:43 99%   ` Alfred Perlstein
  0 siblings, 1 reply; 17+ results
From: Eric Wong @ 2014-12-08 21:36 UTC (permalink / raw)
  To: Alfred Perlstein
  Cc: git, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

Alfred Perlstein <alfred@freebsd.org> 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.root@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!

^ permalink raw reply	[relevance 92%]

* [PATCH] git-svn: Support for git-svn propset
@ 2014-12-07 10:47 56% Alfred Perlstein
  2014-12-08 21:36 92% ` Eric Wong
  0 siblings, 1 reply; 17+ results
From: Alfred Perlstein @ 2014-12-07 10:47 UTC (permalink / raw)
  To: git
  Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

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
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 their entirety from:
  http://mid.gmane.org/20141201094911.GA13931@dcvr.yhbt.net

There is a nit that I want to point out.  The code does not support
adding props unless there are also content changes to the files as
well.  You can see this in the testcase.

Reviewed-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Alfred Perlstein <alfred@freebsd.org>
Signed-off-by: David Fraser <davidf@sjsoft.com>
---
 git-svn.perl               | 49 ++++++++++++++++++++++-
 perl/Git/SVN/Editor.pm     | 42 ++++++++++++++++++++
 t/t9148-git-svn-propset.sh | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+), 1 deletion(-)
 create mode 100755 t/t9148-git-svn-propset.sh

diff --git a/git-svn.perl b/git-svn.perl
index b6e2186..60f8814 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,49 @@ 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);
+	my $cur_props = Git::SVN::Editor::check_attr( "svn-properties", $path );
+	my @new_props;
+	if (!$cur_props || $cur_props eq "unset" || $cur_props eq "" || $cur_props eq "set") {
+		push @new_props, "$propname=$propval";
+	} else {
+		# TODO: handle combining properties better
+		my @props = split(/;/, $cur_props);
+		my $replaced_prop;
+		foreach my $prop (@props) {
+			# Parse 'name=value' syntax and set the property.
+			if ($prop =~ /([^=]+)=(.*)/) {
+				my ($n,$v) = ($1,$2);
+				if ($n eq $propname) {
+					$v = $propval;
+					$replaced_prop = 1;
+				}
+				push @new_props, "$n=$v";
+			}
+		}
+		if (!$replaced_prop) {
+			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
+	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)
 # -------------------
 # Print the list of SVN properties for PATH.
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 34e8af9..b84ce13 100644
--- 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;
+}
+
+sub apply_manualprops {
+	my ($self, $file, $fbat) = @_;
+	my $pending_properties = check_attr( "svn-properties", $file );
+	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
+	# my $existing_props = ::get_svnprops($file);
+	my $existing_props = {};
+	# 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 =~ /([^=]+)=(.*)/) {
+			my ($n,$v) = ($1,$2);
+			for ($n, $v) {
+				s/^\s+//; s/\s+$//;
+			}
+			my $existing = $existing_props->{$n};
+			if (!defined($existing) || $existing ne $v) {
+			    $self->change_file_prop($fbat, $n, $v);
+			}
+		}
+	}
+}
+
 sub A {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -296,6 +334,7 @@ sub A {
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -311,6 +350,7 @@ sub C {
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 				$upa, $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -333,6 +373,7 @@ sub R {
 				$upa, $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 
@@ -348,6 +389,7 @@ sub M {
 	my $fbat = $self->open_file($self->repo_path($m->{file_b}),
 				$pbat,$self->{r},$self->{pool});
 	print "\t$m->{chg}\t$m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
new file mode 100755
index 0000000..584f1c5
--- /dev/null
+++ b/t/t9148-git-svn-propset.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Alfred Perlstein
+#
+
+test_description='git svn propset tests'
+
+. ./lib-git-svn.sh
+
+foo_subdir2="subdir/subdir2/foo_subdir2"
+
+set -e
+mkdir import &&
+(set -e ; cd import
+	mkdir subdir
+	mkdir subdir/subdir2
+	touch foo 		# for 'add props top level'
+	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
+	'
+
+set_props()
+{
+	local subdir="$1"
+	local file="$2"
+	shift;shift;
+	(cd "$subdir" &&
+		while [ $# -gt 0 ] ; do
+			git svn propset "$1" "$2" "$file" || exit 1
+			shift;shift;
+		done &&
+		echo hello >> "$file" &&
+		git commit -m "testing propset" "$file")
+}
+
+confirm_props()
+{
+	local subdir="$1"
+	local file="$2"
+	shift;shift;
+	(set -e ; cd "svn_project/$subdir" &&
+		while [ $# -gt 0 ] ; do
+			test "$(svn_cmd propget "$1" "$file")" = "$2" || exit 1
+			shift;shift;
+		done)
+}
+
+
+#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' '
+	set_props "." "foo" "svn:keywords" "FreeBSD=%H" &&
+	git svn dcommit &&
+	svn_cmd co "$svnrepo" svn_project &&
+	confirm_props "." "foo" "svn:keywords" "FreeBSD=%H" &&
+	rm -rf svn_project
+	'
+
+test_expect_success 'add multiple props' '
+	set_props "." "foo" \
+		"svn:keywords" "FreeBSD=%H" fbsd:nokeywords yes &&
+	git svn dcommit &&
+	svn_cmd co "$svnrepo" svn_project &&
+	confirm_props "." "foo" \
+		"svn:keywords" "FreeBSD=%H" fbsd:nokeywords yes &&
+	rm -rf svn_project
+	'
+
+test_expect_success 'add props subdir' '
+	set_props "." "$foo_subdir2" svn:keywords "FreeBSD=%H" &&
+	git svn dcommit &&
+	svn_cmd co "$svnrepo" svn_project &&
+	confirm_props "." "$foo_subdir2" "svn:keywords" "FreeBSD=%H" &&
+	rm -rf svn_project
+	'
+
+test_expect_success 'add props relative' '
+	set_props "subdir/subdir2" "../foo_subdir" \
+		svn:keywords "FreeBSD=%H" &&
+	git svn dcommit &&
+	svn_cmd co "$svnrepo" svn_project &&
+	confirm_props "subdir/subdir2" "../foo_subdir" \
+		svn:keywords "FreeBSD=%H" &&
+	rm -rf svn_project
+	'
+test_done
-- 
2.1.2

^ permalink raw reply related	[relevance 56%]

* [PATCH] git-svn: propset support v3
@ 2014-12-07 10:46 84% Alfred Perlstein
  0 siblings, 0 replies; 17+ results
From: Alfred Perlstein @ 2014-12-07 10:46 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 1536 bytes --]

Added:

Eric Sunshine's:
- suggestion to include the comment about propset only
  working on files with content changes.

Eric Wong's:
- 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
- commit message improvements.

Torsten Bögershausen:
- Fixes for test cases.

Finally, I have refactored the test cases to reduce duplicate code
such that further fixes and improvements can be done in one place.


JFYI, branches are here:
1st review changes:
  https://github.com/splbio/git/compare/master-git-svn-propset-upstream?expand=1
Squashed into:
  https://github.com/splbio/git/compare/master-git-svn-propset-upstream-one-diff?expand=1

Current review in different commits:
  https://github.com/splbio/git/compare/master-git-svn-propset-upstream2?expand=1
Squashed into:
  https://github.com/splbio/git/compare/master-git-svn-propset-upstream-one-diff2?expand=1
  
Thank you very much.


Alfred Perlstein (1):
  git-svn: Support for git-svn propset

 git-svn.perl               | 49 ++++++++++++++++++++++-
 perl/Git/SVN/Editor.pm     | 42 ++++++++++++++++++++
 t/t9148-git-svn-propset.sh | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+), 1 deletion(-)
 create mode 100755 t/t9148-git-svn-propset.sh

-- 
2.1.2

^ permalink raw reply	[relevance 84%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-07  5:42 63% ` Eric Wong
@ 2014-12-07  9:52 99%   ` Alfred Perlstein
  0 siblings, 0 replies; 17+ results
From: Alfred Perlstein @ 2014-12-07  9:52 UTC (permalink / raw)
  To: Eric Wong
  Cc: git, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser


On 12/6/14, 9:42 PM, Eric Wong wrote:
> Alfred Perlstein <alfred@freebsd.org> 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 <davidf () sjsoft ! com>
> 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.GA13931@dcvr.yhbt.net
>
>> Reviewed-by: Eric Wong <normalperson@yhbt.net>
>> Signed-off-by: Alfred Perlstein <alfred@freebsd.org>
>> Signed-off-by: David Fraser <davidf@sjsoft.com>
> 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 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 {
>   			for ($n, $v) {
>   				s/^\s+//; s/\s+$//;
>   			}
> -			# FIXME: clearly I don't know perl and couldn't work
> -			# out how to evaluate this better
>   			my $existing = $existing_props->{$n};
>   			if (!defined($existing) || $existing ne $v) {
>   			    $self->change_file_prop($fbat, $n, $v);
> diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
> index b36a8a2..6973e8d 100755
> --- a/t/t9148-git-svn-propset.sh
> +++ b/t/t9148-git-svn-propset.sh
> @@ -9,16 +9,14 @@ test_description='git svn propset tests'
>   
>   foo_subdir2="subdir/subdir2/foo_subdir2"
>   
> -mkdir import
> -(cd import
> -	mkdir subdir
> -	mkdir subdir/subdir2
> -	touch foo		# for 'add props top level'
> -	touch subdir/foo_subdir # for 'add props relative'
> -	touch "$foo_subdir2"	# for 'add props subdir'
> +(mkdir import && cd import &&
> +	mkdir subdir &&
> +	mkdir subdir/subdir2 &&
> +	touch foo && : "for 'add props top level'" &&
> +	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
> +) && rm -rf import
>   
>   test_expect_success 'initialize git svn' 'git svn init "$svnrepo"'
>   test_expect_success 'fetch revisions from svn' 'git svn fetch'
> @@ -30,9 +28,10 @@ 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
> +	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_cmd propget svn:keywords foo)" = "FreeBSD=%H") &&
>   	rm -rf svn_project
>   	'
>   
> @@ -41,11 +40,12 @@ test_expect_success 'add multiple props' '
>   	git svn propset fbsd:nokeywords yes foo &&
>   	echo hello >> foo &&
>   	git commit -m "testing propset" foo &&
> -	git svn dcommit
> +	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) &&
> +	(cd svn_project &&
> +		test "$(svn_cmd propget svn:keywords foo)" = "FreeBSD=%H" &&
> +		test "$(svn_cmd propget fbsd:nokeywords foo)" = "yes" &&
> +		test "$(svn_cmd proplist -q foo | wc -l)" -eq 2) &&
>   	rm -rf svn_project
>   	'
>   
> @@ -53,19 +53,24 @@ test_expect_success 'add props subdir' '
>   	git svn propset svn:keywords "FreeBSD=%H" "$foo_subdir2" &&
>   	echo hello >> "$foo_subdir2" &&
>   	git commit -m "testing propset" "$foo_subdir2" &&
> -	git svn dcommit
> +	git svn dcommit &&
>   	svn_cmd co "$svnrepo" svn_project &&
> -	(cd svn_project && test "`svn propget svn:keywords "$foo_subdir2"`" = "FreeBSD=%H") &&
> +	(cd svn_project &&
> +	 test "$(svn_cmd propget svn:keywords "$foo_subdir2")" = "FreeBSD=%H"
> +	) &&
>   	rm -rf svn_project
>   	'
>   
>   test_expect_success 'add props relative' '
> -	(cd subdir/subdir2 && git svn propset svn:keywords "FreeBSD=%H" ../foo_subdir ) &&
> +	(cd subdir/subdir2 &&
> +	 git svn propset svn:keywords "FreeBSD=%H" ../foo_subdir ) &&
>   	echo hello >> subdir/foo_subdir &&
>   	git commit -m "testing propset" subdir/foo_subdir &&
> -	git svn dcommit
> +	git svn dcommit &&
>   	svn_cmd co "$svnrepo" svn_project &&
> -	(cd svn_project && test "`svn propget svn:keywords subdir/foo_subdir`" = "FreeBSD=%H") &&
> +	(cd svn_project &&
> +	 test "$(svn_cmd propget svn:keywords subdir/foo_subdir)" = "FreeBSD=%H"
> +	) &&
>   	rm -rf svn_project
>   	'
>   test_done
>

^ permalink raw reply	[relevance 99%]

* Re: [PATCH] git-svn: propset support v2
  2014-12-06 22:29 86% [PATCH] git-svn: propset support v2 Alfred Perlstein
@ 2014-12-07  9:25 91% ` Eric Sunshine
  0 siblings, 0 replies; 17+ results
From: Eric Sunshine @ 2014-12-07  9:25 UTC (permalink / raw)
  To: Alfred Perlstein
  Cc: Git List, Eric Wong, Jonathan Nieder, Junio C Hamano,
	Michael G. Schwern, David Fraser

On Sat, Dec 6, 2014 at 5:29 PM, Alfred Perlstein <alfred@freebsd.org> wrote:
> I have incorporated Eric Wong's feedback into the git-svn propset support patch.
>
> There is a nit that I want to point out.  The code does not support adding props
> unless there are also content changes to the files as well.  You can see this in
> the testcase.

This is an important nugget of information which would be worthwhile
to mention in the commit message of the patch rather than here as mere
commentary.

> That is still sufficient for many people's workflows (FreeBSD at
> least).  So I am wondering if this is OK.
>
> I would gladly take any pointers to making it work with unchanged
> files either for a later diff or to wrap this up.

^ permalink raw reply	[relevance 91%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-07  8:00 99%   ` Torsten Bögershausen
@ 2014-12-07  9:23 99%     ` Eric Sunshine
  0 siblings, 0 replies; 17+ results
From: Eric Sunshine @ 2014-12-07  9:23 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Alfred Perlstein, Git List, Eric Wong, Jonathan Nieder,
	Junio C Hamano, Michael G. Schwern, David Fraser

On Sun, Dec 7, 2014 at 3:00 AM, Torsten Bögershausen <tboegi@web.de> 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.

^ permalink raw reply	[relevance 99%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-07  5:45 97% ` Torsten Bögershausen
@ 2014-12-07  8:00 99%   ` Torsten Bögershausen
  2014-12-07  9:23 99%     ` Eric Sunshine
  0 siblings, 1 reply; 17+ results
From: Torsten Bögershausen @ 2014-12-07  8:00 UTC (permalink / raw)
  To: Torsten Bögershausen, Alfred Perlstein, git
  Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

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)

^ permalink raw reply	[relevance 99%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-06 22:29 56% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
  2014-12-07  5:42 63% ` Eric Wong
@ 2014-12-07  5:45 97% ` Torsten Bögershausen
  2014-12-07  8:00 99%   ` Torsten Bögershausen
  1 sibling, 1 reply; 17+ results
From: Torsten Bögershausen @ 2014-12-07  5:45 UTC (permalink / raw)
  To: Alfred Perlstein, git
  Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser


> diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
> new file mode 100755
> index 0000000..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

^ permalink raw reply	[relevance 97%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-06 22:29 56% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
@ 2014-12-07  5:42 63% ` Eric Wong
  2014-12-07  9:52 99%   ` Alfred Perlstein
  2014-12-07  5:45 97% ` Torsten Bögershausen
  1 sibling, 1 reply; 17+ results
From: Eric Wong @ 2014-12-07  5:42 UTC (permalink / raw)
  To: Alfred Perlstein
  Cc: git, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

Alfred Perlstein <alfred@freebsd.org> 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 <davidf () sjsoft ! com>

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.GA13931@dcvr.yhbt.net

> Reviewed-by: Eric Wong <normalperson@yhbt.net>
> Signed-off-by: Alfred Perlstein <alfred@freebsd.org>
> Signed-off-by: David Fraser <davidf@sjsoft.com>

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 {
 			for ($n, $v) {
 				s/^\s+//; s/\s+$//;
 			}
-			# FIXME: clearly I don't know perl and couldn't work
-			# out how to evaluate this better
 			my $existing = $existing_props->{$n};
 			if (!defined($existing) || $existing ne $v) {
 			    $self->change_file_prop($fbat, $n, $v);
diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
index b36a8a2..6973e8d 100755
--- a/t/t9148-git-svn-propset.sh
+++ b/t/t9148-git-svn-propset.sh
@@ -9,16 +9,14 @@ test_description='git svn propset tests'
 
 foo_subdir2="subdir/subdir2/foo_subdir2"
 
-mkdir import
-(cd import
-	mkdir subdir
-	mkdir subdir/subdir2
-	touch foo		# for 'add props top level'
-	touch subdir/foo_subdir # for 'add props relative'
-	touch "$foo_subdir2"	# for 'add props subdir'
+(mkdir import && cd import &&
+	mkdir subdir &&
+	mkdir subdir/subdir2 &&
+	touch foo && : "for 'add props top level'" &&
+	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
+) && rm -rf import
 
 test_expect_success 'initialize git svn' 'git svn init "$svnrepo"'
 test_expect_success 'fetch revisions from svn' 'git svn fetch'
@@ -30,9 +28,10 @@ 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
+	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_cmd propget svn:keywords foo)" = "FreeBSD=%H") &&
 	rm -rf svn_project
 	'
 
@@ -41,11 +40,12 @@ test_expect_success 'add multiple props' '
 	git svn propset fbsd:nokeywords yes foo &&
 	echo hello >> foo &&
 	git commit -m "testing propset" foo &&
-	git svn dcommit
+	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) &&
+	(cd svn_project &&
+		test "$(svn_cmd propget svn:keywords foo)" = "FreeBSD=%H" &&
+		test "$(svn_cmd propget fbsd:nokeywords foo)" = "yes" &&
+		test "$(svn_cmd proplist -q foo | wc -l)" -eq 2) &&
 	rm -rf svn_project
 	'
 
@@ -53,19 +53,24 @@ test_expect_success 'add props subdir' '
 	git svn propset svn:keywords "FreeBSD=%H" "$foo_subdir2" &&
 	echo hello >> "$foo_subdir2" &&
 	git commit -m "testing propset" "$foo_subdir2" &&
-	git svn dcommit
+	git svn dcommit &&
 	svn_cmd co "$svnrepo" svn_project &&
-	(cd svn_project && test "`svn propget svn:keywords "$foo_subdir2"`" = "FreeBSD=%H") &&
+	(cd svn_project &&
+	 test "$(svn_cmd propget svn:keywords "$foo_subdir2")" = "FreeBSD=%H"
+	) &&
 	rm -rf svn_project
 	'
 
 test_expect_success 'add props relative' '
-	(cd subdir/subdir2 && git svn propset svn:keywords "FreeBSD=%H" ../foo_subdir ) &&
+	(cd subdir/subdir2 &&
+	 git svn propset svn:keywords "FreeBSD=%H" ../foo_subdir ) &&
 	echo hello >> subdir/foo_subdir &&
 	git commit -m "testing propset" subdir/foo_subdir &&
-	git svn dcommit
+	git svn dcommit &&
 	svn_cmd co "$svnrepo" svn_project &&
-	(cd svn_project && test "`svn propget svn:keywords subdir/foo_subdir`" = "FreeBSD=%H") &&
+	(cd svn_project &&
+	 test "$(svn_cmd propget svn:keywords subdir/foo_subdir)" = "FreeBSD=%H"
+	) &&
 	rm -rf svn_project
 	'
 test_done

^ permalink raw reply related	[relevance 63%]

* [PATCH] git-svn: Support for git-svn propset
@ 2014-12-06 22:29 56% Alfred Perlstein
  2014-12-07  5:42 63% ` Eric Wong
  2014-12-07  5:45 97% ` Torsten Bögershausen
  0 siblings, 2 replies; 17+ results
From: Alfred Perlstein @ 2014-12-06 22:29 UTC (permalink / raw)
  To: git
  Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

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 <davidf () sjsoft ! com>
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:
  http://marc.info/?l=git&m=141742735608544&w=2

Reviewed-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Alfred Perlstein <alfred@freebsd.org>
Signed-off-by: David Fraser <davidf@sjsoft.com>
---
 git-svn.perl               | 49 +++++++++++++++++++++++++++++++-
 perl/Git/SVN/Editor.pm     | 43 ++++++++++++++++++++++++++++
 t/t9148-git-svn-propset.sh | 71 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100755 t/t9148-git-svn-propset.sh

diff --git a/git-svn.perl b/git-svn.perl
index b6e2186..5cdbf39 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,49 @@ 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);
+	my $cur_props = Git::SVN::Editor::check_attr( "svn-properties", $path );
+	my $new_props = "";
+	if ($cur_props eq "unset" || $cur_props eq "" || $cur_props eq "set") {
+		$new_props = "$propname=$propval";
+	} else {
+		# TODO: handle combining properties better
+		my @props = split(/;/, $cur_props);
+		my $replaced_prop;
+		foreach my $prop (@props) {
+			# Parse 'name=value' syntax and set the property.
+			if ($prop =~ /([^=]+)=(.*)/) {
+				my ($n,$v) = ($1,$2);
+				if ($n eq $propname)
+				{
+					$v = $propval;
+					$replaced_prop = 1;
+				}
+				if ($new_props eq "") { $new_props="$n=$v"; }
+				else { $new_props="$new_props;$n=$v"; }
+			}
+		}
+		if (!$replaced_prop) {
+			$new_props = "$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";
+}
+
 # cmd_proplist (PATH)
 # -------------------
 # Print the list of SVN properties for PATH.
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 34e8af9..dd15318 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -288,6 +288,45 @@ 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;
+	$val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/;
+	return $val;
+}
+
+sub apply_manualprops {
+	my ($self, $file, $fbat) = @_;
+	my $pending_properties = check_attr( "svn-properties", $file );
+	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
+	# my $existing_props = ::get_svnprops($file);
+	my $existing_props = {};
+	# 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 =~ /([^=]+)=(.*)/) {
+			my ($n,$v) = ($1,$2);
+			for ($n, $v) {
+				s/^\s+//; s/\s+$//;
+			}
+			# FIXME: clearly I don't know perl and couldn't work
+			# out how to evaluate this better
+			my $existing = $existing_props->{$n};
+			if (!defined($existing) || $existing ne $v) {
+			    $self->change_file_prop($fbat, $n, $v);
+			}
+		}
+	}
+}
+
 sub A {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -296,6 +335,7 @@ sub A {
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -311,6 +351,7 @@ sub C {
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 				$upa, $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -333,6 +374,7 @@ sub R {
 				$upa, $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 
@@ -348,6 +390,7 @@ sub M {
 	my $fbat = $self->open_file($self->repo_path($m->{file_b}),
 				$pbat,$self->{r},$self->{pool});
 	print "\t$m->{chg}\t$m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
diff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh
new file mode 100755
index 0000000..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"
+
+mkdir import
+(cd import
+	mkdir subdir
+	mkdir subdir/subdir2
+	touch foo		# for 'add props top level'
+	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'
+
+# 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.
+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
+	'
+
+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
+	'
+
+test_expect_success 'add props subdir' '
+	git svn propset svn:keywords "FreeBSD=%H" "$foo_subdir2" &&
+	echo hello >> "$foo_subdir2" &&
+	git commit -m "testing propset" "$foo_subdir2" &&
+	git svn dcommit
+	svn_cmd co "$svnrepo" svn_project &&
+	(cd svn_project && test "`svn propget svn:keywords "$foo_subdir2"`" = "FreeBSD=%H") &&
+	rm -rf svn_project
+	'
+
+test_expect_success 'add props relative' '
+	(cd subdir/subdir2 && git svn propset svn:keywords "FreeBSD=%H" ../foo_subdir ) &&
+	echo hello >> subdir/foo_subdir &&
+	git commit -m "testing propset" subdir/foo_subdir &&
+	git svn dcommit
+	svn_cmd co "$svnrepo" svn_project &&
+	(cd svn_project && test "`svn propget svn:keywords subdir/foo_subdir`" = "FreeBSD=%H") &&
+	rm -rf svn_project
+	'
+test_done
-- 
2.1.2

^ permalink raw reply related	[relevance 56%]

* [PATCH] git-svn: propset support v2
@ 2014-12-06 22:29 86% Alfred Perlstein
  2014-12-07  9:25 91% ` Eric Sunshine
  0 siblings, 1 reply; 17+ results
From: Alfred Perlstein @ 2014-12-06 22:29 UTC (permalink / raw)
  To: git
  Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

I have incorporated Eric Wong's feedback into the git-svn propset support patch.

Issues resolved:
1) Test-case written.
2) Remove dead code.
3) Use shorter vars for formatting.
4) Fix bool comparisons.
5) Check for filesystem errors on write to .gitattribute file.
6) Use command_output_pipe() instead of open my $fh, '-|', ...
7) Refactor check for existing props.

There is a nit that I want to point out.  The code does not support adding props
unless there are also content changes to the files as well.  You can see this in
the testcase.

That is still sufficient for many people's workflows (FreeBSD at
least).  So I am wondering if this is OK. 

I would gladly take any pointers to making it work with unchanged
files either for a later diff or to wrap this up.

Thank you.

Alfred Perlstein (1):
  git-svn: Support for git-svn propset

 git-svn.perl               | 49 +++++++++++++++++++++++++++++++-
 perl/Git/SVN/Editor.pm     | 43 ++++++++++++++++++++++++++++
 t/t9148-git-svn-propset.sh | 71 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100755 t/t9148-git-svn-propset.sh

-- 
2.1.2

^ permalink raw reply	[relevance 86%]

* Re: [PATCH] git-svn: Support for git-svn propset
  2014-12-01  6:24 61% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
@ 2014-12-01  9:49 84% ` Eric Wong
  0 siblings, 0 replies; 17+ results
From: Eric Wong @ 2014-12-01  9:49 UTC (permalink / raw)
  To: Alfred Perlstein
  Cc: git, Jonathan Nieder, Junio C Hamano, Michael G. Schwern,
	David Fraser

Alfred Perlstein <alfred@freebsd.org> 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 <davidf () sjsoft ! com>
> 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.GA20673@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

<snip> no need for the whole DCO in the commit message.
just the S-o-b:

> Signed-off-by: Alfred Perlstein <alfred@freebsd.org>

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 =~ /([^=]+)=(.*)/) {
> +				my ($n,$v) = ($1,$2);
> +				if ($n eq $propname)
> +				{
> +					$v = $propval;
> +					$replaced_prop = 1;
> +				}
> +				if ($new_properties eq "") { $new_properties="$n=$v"; }
> +				else { $new_properties="$new_properties;$n=$v"; }
> +			}
> +		}
> +		if ($replaced_prop eq 0) {

Generally, == is favored for numeric comparisons
(but this is boolean)

> +			$new_properties = "$new_properties;$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_properties\n";

Would be good to have an explicit close and error checking on it in case
of FS errors

> +}
> +
>  # cmd_proplist (PATH)
>  # -------------------
>  # Print the list of SVN properties for PATH.
> diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
> index 34e8af9..5158c03 100644
> --- a/perl/Git/SVN/Editor.pm
> +++ b/perl/Git/SVN/Editor.pm
> @@ -288,6 +288,49 @@ sub apply_autoprops {
>  	}
>  }
>  
> +sub check_attr
> +{
> +    my ($attr,$path) = @_;
> +    if ( open my $fh, '-|', "git", "check-attr", $attr, "--", $path )

Use Git.pm (command_output_pipe) for running git commands portably.
(And formatting for this sub alone is really off).

> +    {
> +	my $val = <$fh>;
> +	close $fh;
> +	$val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/;
> +	return $val;
> +    }
> +    else
> +    {
> +	return undef;
> +    }
> +}
> +
> +sub apply_manualprops {
> +	my ($self, $file, $fbat) = @_;
> +	my $pending_properties = check_attr( "svn-properties", $file );
> +	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
> +	# my $existing_props = ::get_svnprops($file);
> +	my $existing_props = {};
> +	# 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 =~ /([^=]+)=(.*)/) {
> +			my ($n,$v) = ($1,$2);
> +			for ($n, $v) {
> +				s/^\s+//; s/\s+$//;
> +			}

I'd probably rewrite the following hunk:

> +			# FIXME: clearly I don't know perl and couldn't work out how to evaluate this better
> +			if (defined $existing_props->{$n} && $existing_props->{$n} eq $v) {
> +				my $needed = 0;
> +			} else {
> +				$self->change_file_prop($fbat, $n, $v);
> +			}

as:

			my $existing = $existing_props->{$n};
			if (!defined($existing) || $existing ne $v) {
				$self->change_file_prop($fbat, $n, $v);
			}

No need for setting $needed = 0 from what I can tell.

Rest of the patch looks fine.

Thank you again for bringing this up and I look forward to a reroll
of this with my comments addressed (maybe wait a few days for others
to comment, holidays and all).

^ permalink raw reply	[relevance 84%]

* [PATCH] git-svn: Support for git-svn propset
@ 2014-12-01  6:24 61% Alfred Perlstein
  2014-12-01  9:49 84% ` Eric Wong
  0 siblings, 1 reply; 17+ results
From: Alfred Perlstein @ 2014-12-01  6:24 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Jonathan Nieder, Junio C Hamano, Michael G. Schwern

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 <davidf () sjsoft ! com>
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.

        Developer's Certificate of Origin 1.1

        By making a contribution to this project, I certify that:

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or

        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.

	(d) I understand and agree that this project and the contribution
	    are public and that a record of the contribution (including all
	    personal information I submit with it, including my sign-off) is
	    maintained indefinitely and may be redistributed consistent with
	    this project or the open source license(s) involved.

Signed-off-by: Alfred Perlstein <alfred@freebsd.org>
---
 git-svn.perl           | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 perl/Git/SVN/Editor.pm | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 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 );
+	my $current_properties = Git::SVN::Editor::check_attr( "svn-properties", $path );
+	my $new_properties = "";
+	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 =~ /([^=]+)=(.*)/) {
+				my ($n,$v) = ($1,$2);
+				if ($n eq $propname)
+				{
+					$v = $propval;
+					$replaced_prop = 1;
+				}
+				if ($new_properties eq "") { $new_properties="$n=$v"; }
+				else { $new_properties="$new_properties;$n=$v"; }
+			}
+		}
+		if ($replaced_prop eq 0) {
+			$new_properties = "$new_properties;$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_properties\n";
+}
+
 # cmd_proplist (PATH)
 # -------------------
 # Print the list of SVN properties for PATH.
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 34e8af9..5158c03 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -288,6 +288,49 @@ sub apply_autoprops {
 	}
 }
 
+sub check_attr
+{
+    my ($attr,$path) = @_;
+    if ( open my $fh, '-|', "git", "check-attr", $attr, "--", $path )
+    {
+	my $val = <$fh>;
+	close $fh;
+	$val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/;
+	return $val;
+    }
+    else
+    {
+	return undef;
+    }
+}
+
+sub apply_manualprops {
+	my ($self, $file, $fbat) = @_;
+	my $pending_properties = check_attr( "svn-properties", $file );
+	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
+	# my $existing_props = ::get_svnprops($file);
+	my $existing_props = {};
+	# 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 =~ /([^=]+)=(.*)/) {
+			my ($n,$v) = ($1,$2);
+			for ($n, $v) {
+				s/^\s+//; s/\s+$//;
+			}
+			# FIXME: clearly I don't know perl and couldn't work out how to evaluate this better
+			if (defined $existing_props->{$n} && $existing_props->{$n} eq $v) {
+				my $needed = 0;
+			} else {
+				$self->change_file_prop($fbat, $n, $v);
+			}
+		}
+	}
+}
+
 sub A {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -296,6 +339,7 @@ sub A {
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -311,6 +355,7 @@ sub C {
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 				$upa, $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -333,6 +378,7 @@ sub R {
 				$upa, $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 
@@ -348,6 +394,7 @@ sub M {
 	my $fbat = $self->open_file($self->repo_path($m->{file_b}),
 				$pbat,$self->{r},$self->{pool});
 	print "\t$m->{chg}\t$m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($m->{file_b}, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
-- 
2.1.2

^ permalink raw reply related	[relevance 61%]

* git svn propset support
       [not found]     <1699967795.351252597251356.JavaMail.root@klofta.sjsoft.com>
@ 2009-09-10 15:41 59% ` David Fraser
  0 siblings, 0 replies; 17+ results
From: David Fraser @ 2009-09-10 15:41 UTC (permalink / raw)
  To: git; +Cc: David Moore

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

Hi

I've been trying to hack at getting manually property setting support for git-svn.

I've got a start of a patch that basically stores an attribute 'svn-properties' for each file that needs them changed, and then sets the properties when committing.

Issues remaining:
 * The way it edits the .gitattributes file is suboptimal - it just appends to the end the latest version
 * It could use the existing code to get the current svn properties to see if properties need to be changed; but this doesn't work on add
 * It would be better to cache all the svn properties locally - this could be done automatically in .gitattributes but I'm not sure everyone would want this, etc
 * No support for deleting properties

Usage is:
 git svn propset PROPNAME PROPVALUE PATH

Patch attached (against 1.6.0.2) - any comments welcome

David

-- 
David Fraser
St James Software

[-- Attachment #2: git-svn-propset-0.patch --]
[-- Type: text/x-patch, Size: 5427 bytes --]

--- git-1.6.0.2/git-svn.perl	2008-12-15 15:52:14.000000000 +0200
+++ git-svn.perl	2009-09-10 17:35:04.000000000 +0200
@@ -66,7 +66,7 @@
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format, $_commit_url);
+	$_git_format, $_commit_url, $_set_svn_props);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -128,6 +128,7 @@
 			  '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,
 			%cmt_opts, %fc_opts } ],
@@ -141,6 +142,9 @@
         '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 } ],
@@ -700,6 +704,64 @@
 	print $props->{$prop} . "\n";
 }
 
+sub check_attr
+{
+    my ($attr,$path) = @_;
+    if ( open my $fh, '-|', "git", "check-attr", $attr, "--", $path )
+    {
+	my $val = <$fh>;
+	close $fh;
+	$val =~ s/^[^:]*:\s*[^:]*:\s*(.*)\s*$/$1/;
+	return $val;
+    }
+    else
+    {
+	return undef;
+    }
+}
+
+# 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);
+	my $current_properties = check_attr( "svn-properties", $path );
+	my $new_properties = "";
+	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 =~ /([^=]+)=(.*)/) {
+				my ($n,$v) = ($1,$2);
+				if ($n eq $propname)
+				{
+					$v = $propval;
+					$replaced_prop = 1;
+				}
+				if ($new_properties eq "") { $new_properties="$n=$v"; }
+				else { $new_properties="$new_properties;$n=$v"; }
+			}
+		}
+		if ($replaced_prop eq 0) {
+			$new_properties = "$new_properties;$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_properties\n";
+}
+
 # cmd_proplist (PATH)
 # -------------------
 # Print the list of SVN properties for PATH.
@@ -3598,6 +3660,34 @@
 	}
 }
 
+sub apply_manualprops {
+	my ($self, $file, $fbat) = @_;
+	my $path = $cmd_dir_prefix . $file;
+	my $pending_properties = ::check_attr( "svn-properties", $path );
+	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
+	# my $existing_props = ::get_svnprops($file);
+	my $existing_props = {};
+	# 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 =~ /([^=]+)=(.*)/) {
+			my ($n,$v) = ($1,$2);
+			for ($n, $v) {
+				s/^\s+//; s/\s+$//;
+			}
+			# FIXME: clearly I don't know perl and couldn't work out how to evaluate this better
+			if (defined $existing_props->{$n} && $existing_props->{$n} eq $v) {
+				my $needed = 0;
+			} else {
+				$self->change_file_prop($fbat, $n, $v);
+			}
+		}
+	}
+}
+
 sub A {
 	my ($self, $m) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -3606,6 +3696,7 @@
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
+	$self->apply_manualprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -3617,6 +3708,7 @@
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 				$self->url_path($m->{file_a}), $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
@@ -3636,6 +3728,7 @@
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
 				$self->url_path($m->{file_a}), $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 
@@ -3651,6 +3744,7 @@
 	my $fbat = $self->open_file($self->repo_path($m->{file_b}),
 				$pbat,$self->{r},$self->{pool});
 	print "\t$m->{chg}\t$m->{file_b}\n" unless $::_q;
+	$self->apply_manualprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }

^ permalink raw reply	[relevance 59%]

Results 1-17 of 17 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
     [not found]     <1699967795.351252597251356.JavaMail.root@klofta.sjsoft.com>
2009-09-10 15:41 59% ` git svn propset support David Fraser
2014-12-01  6:24 61% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
2014-12-01  9:49 84% ` Eric Wong
2014-12-06 22:29 86% [PATCH] git-svn: propset support v2 Alfred Perlstein
2014-12-07  9:25 91% ` Eric Sunshine
2014-12-06 22:29 56% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
2014-12-07  5:42 63% ` Eric Wong
2014-12-07  9:52 99%   ` Alfred Perlstein
2014-12-07  5:45 97% ` Torsten Bögershausen
2014-12-07  8:00 99%   ` Torsten Bögershausen
2014-12-07  9:23 99%     ` Eric Sunshine
2014-12-07 10:46 84% [PATCH] git-svn: propset support v3 Alfred Perlstein
2014-12-07 10:47 56% [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
2014-12-08 21:36 92% ` Eric Wong
2014-12-08 23:43 99%   ` Alfred Perlstein
2017-03-22 23:29 98% git svn propset and multi-line svn:externals property Craig McQueen
2017-03-23  3:01 98% ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).