[PATCH] Initial manually svn property setting support for git-svn

[PATCH] Initial manually svn property setting support for git-svn

[David Fraser]

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

This 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

Added minimal documentation for git-svn propset

Signed-off-by: David Fraser <davidf@sjsoft.com>
---
 Documentation/git-svn.txt |    5 ++
 git-svn.perl              |   95 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 99 insertions(+), 1 deletions(-)

[-- Attachment #2: 0001-Initial-manually-svn-property-setting-support-for-gi.patch --]
[-- Type: text/x-patch, Size: 7329 bytes --]

>From 521ed4fc7c877269fb029b9494ef57300c722a10 Mon Sep 17 00:00:00 2001
From: David Fraser <davidf@sjsoft.com>
Date: Wed, 16 Sep 2009 13:28:00 +0200
Subject: [PATCH] Initial manually svn property setting support for git-svn.

This 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

Added minimal documentation for git-svn propset

Signed-off-by: David Fraser <davidf@sjsoft.com>
---
 Documentation/git-svn.txt |    5 ++
 git-svn.perl              |   95 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 99 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 1812890..b14bcf0 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -345,6 +345,11 @@ Any other arguments are passed directly to 'git log'
 	Gets the Subversion property given as the first argument, for a
 	file.  A specific revision can be specified with -r/--revision.
 
+'propset'::
+	Sets the Subversion property given as the first argument, to the value
+        given as the second argument, for files specifed as the remaining
+	arguments. The property will be sent with the next commit.
+
 'show-externals'::
 	Shows the Subversion externals.  Use -r/--revision to specify a
 	specific revision.
diff --git a/git-svn.perl b/git-svn.perl
index e0ec258..aaf92fb 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -80,7 +80,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase, $_fetch_parent,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format, $_commit_url, $_tag);
+	$_git_format, $_commit_url, $_tag, $_set_svn_props);
 $Git::SVN::_follow_parent = 1;
 $_q ||= 0;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
@@ -147,6 +147,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,
 			%cmt_opts, %fc_opts } ],
@@ -171,6 +172,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 } ],
@@ -892,6 +896,64 @@ sub cmd_propget {
 	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.
@@ -4185,6 +4247,33 @@ sub apply_autoprops {
 	}
 }
 
+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+$//;
+			}
+			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});
@@ -4193,6 +4282,7 @@ sub A {
 					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});
 }
@@ -4204,6 +4294,7 @@ sub C {
 	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});
 }
@@ -4223,6 +4314,7 @@ sub R {
 	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->apply_autoprops($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
@@ -4239,6 +4331,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($file, $fbat);
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
 }
-- 
1.6.0.4

Re: [PATCH] Initial manually svn property setting support for git-svn

[Thiago Farina]

On Wed, Sep 16, 2009 at 4:02 AM, David Fraser <davidf@sjsoft.com> wrote:
> This 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
>
> Added minimal documentation for git-svn propset
>
> Signed-off-by: David Fraser <davidf@sjsoft.com>
> ---
>  Documentation/git-svn.txt |    5 ++
>  git-svn.perl              |   95 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 99 insertions(+), 1 deletions(-)
>
David, please read this, line 28:
http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/SubmittingPatches;h=76fc84d8780762e083cd4ca584b9d783b8c0cd81;hb=HEAD

Patches need to be send as plain text, not attached.

Re: [PATCH] Initial manually svn property setting support for git-svn

[Eric Wong]

David Fraser <davidf@sjsoft.com> wrote:
> This basically stores an attribute 'svn-properties' for each file that
> needs them changed, and then sets the properties when committing.

Hi David,

Please wrap your commit messages and emails at 72 columns or less.
All git svn code should be wrapped at 80 or less, too.

> Issues remaining:
>  * The way it edits the .gitattributes file is suboptimal - it just
>  appends to the end the latest version

Consider using $GIT_DIR/info/attributes or having an option to use that
instead.  Keeping a .gitattributes file in the git working tree but
_out_ of SVN is important and required, but also difficult to get right.

There are users working on projects that frown upon using unsupported
clients like git svn, and accidentally checking .gitattributes into
the project would likely annoy non-git users.  It's best to keep
*.git* stuff outside of SVN projects unless they allow/want it.

But also you should not fail to consider the case that somebody else did
intentionally commit .gitattributes into svn and you have little choice
but to commit your modifications to .gitattributes.  Things like
maintaining a mapping between svn:ignore and .gitignore has never
happened because of the corner cases that could pop up.

>  * 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

It should be possible to infer/rebuild this by parsing unhandled.log
files git svn generates by default.  There are definitely people who
don't want .gitattributes being written to automatically.

>  * No support for deleting properties

What advantage(s) does having this feature in git svn this give over
using:

	svn prop(edit|set|del) ARGS $(git svn info --url)

In my experience, explicitly set properties are rarely-used so I
need to be convinced it's worth supporting in the future.

> Usage is:
>  git svn propset PROPNAME PROPVALUE PATH
> 
> Added minimal documentation for git-svn propset

We'll also need a test case to ensure it continues working as other
changes get made and refactoring gets done.

> +sub check_attr
> +{
> +    my ($attr,$path) = @_;

Please use formatting consistent with the rest of the file.  Always use
tabs for indent here.

> +    if ( open my $fh, '-|', "git", "check-attr", $attr, "--", $path )

Consider command_output_pipe for better portability/consistency in
Git.pm instead of open(my $fh, "-|", @args) which is Perl 5.8+-only.


Thanks for the effort and keep us informed of improvements you make.

-- 
Eric Wong