{"thread":{"id":"38082","subject":"[PATCH] git-svn: Support for git-svn propset","startedAt":"2014-12-01T06:24:20Z","lastAt":"2014-12-01T09:49:11Z","messageCount":2,"participants":["Alfred Perlstein","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"252769","messageId":"20141201062420.GF99906@elvis.mu.org","threadId":"38082","inReplyTo":null,"subject":"[PATCH] git-svn: Support for git-svn propset","fromName":"Alfred Perlstein","fromEmail":"alfred@freebsd.org","sentAt":"2014-12-01T06:24:20Z","receivedAt":"2014-12-01T06:24:20Z","isPatch":true,"sender":{"key":"alfred@freebsd.org","avatar":"https://avatars.githubusercontent.com/u/629072?v=4"},"body":"This change allows git-svn to support setting subversion properties.\n\nVery useful for manually setting properties when committing to a\nsubversion repo that *requires* properties to be set without requiring\nmoving your changeset to separate subversion checkout in order to\nset props.\n\nThis change is initially from David Fraser <davidf () sjsoft ! com>\nAppearing here: http://marc.info/?l=git&m=125259772625008&w=2\n\nThey are now forward ported to most recent git along with fixes to\ndeal with files in subdirectories.\n\n        Developer's Certificate of Origin 1.1\n\n        By making a contribution to this project, I certify that:\n\n        (a) The contribution was created in whole or in part by me and I\n            have the right to submit it under the open source license\n            indicated in the file; or\n\n        (b) The contribution is based upon previous work that, to the best\n            of my knowledge, is covered under an appropriate open source\n            license and I have the right under that license to submit that\n            work with modifications, whether created in whole or in part\n            by me, under the same open source license (unless I am\n            permitted to submit under a different license), as indicated\n            in the file; or\n\n        (c) The contribution was provided directly to me by some other\n            person who certified (a), (b) or (c) and I have not modified\n            it.\n\n\t(d) I understand and agree that this project and the contribution\n\t    are public and that a record of the contribution (including all\n\t    personal information I submit with it, including my sign-off) is\n\t    maintained indefinitely and may be redistributed consistent with\n\t    this project or the open source license(s) involved.\n\nSigned-off-by: Alfred Perlstein <alfred@freebsd.org>\n---\n git-svn.perl           | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-\n perl/Git/SVN/Editor.pm | 47 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 96 insertions(+), 1 deletion(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex b6e2186..91423a8 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -115,7 +115,7 @@ my ($_stdin, $_help, $_edit,\n \t$_before, $_after,\n \t$_merge, $_strategy, $_preserve_merges, $_dry_run, $_parents, $_local,\n \t$_prefix, $_no_checkout, $_url, $_verbose,\n-\t$_commit_url, $_tag, $_merge_info, $_interactive);\n+\t$_commit_url, $_tag, $_merge_info, $_interactive, $_set_svn_props);\n \n # This is a refactoring artifact so Git::SVN can get at this git-svn switch.\n sub opt_prefix { return $_prefix || '' }\n@@ -193,6 +193,7 @@ my %cmd = (\n \t\t\t  'dry-run|n' => \\$_dry_run,\n \t\t\t  'fetch-all|all' => \\$_fetch_all,\n \t\t\t  'commit-url=s' => \\$_commit_url,\n+\t\t\t  'set-svn-props=s' => \\$_set_svn_props,\n \t\t\t  'revision|r=i' => \\$_revision,\n \t\t\t  'no-rebase' => \\$_no_rebase,\n \t\t\t  'mergeinfo=s' => \\$_merge_info,\n@@ -228,6 +229,9 @@ my %cmd = (\n         'propget' => [ \\&cmd_propget,\n \t\t       'Print the value of a property on a file or directory',\n \t\t       { 'revision|r=i' => \\$_revision } ],\n+        'propset' => [ \\&cmd_propset,\n+\t\t       'Set the value of a property on a file or directory - will be set on commit',\n+\t\t       {} ],\n         'proplist' => [ \\&cmd_proplist,\n \t\t       'List all properties of a file or directory',\n \t\t       { 'revision|r=i' => \\$_revision } ],\n@@ -1376,6 +1380,50 @@ sub cmd_propget {\n \tprint $props->{$prop} . \"\\n\";\n }\n \n+# cmd_propset (PROPNAME, PROPVAL, PATH)\n+# ------------------------\n+# Adjust the SVN property PROPNAME to PROPVAL for PATH.\n+sub cmd_propset {\n+\tmy ($propname, $propval, $path) = @_;\n+\t$path = '.' if not defined $path;\n+\t$path = $cmd_dir_prefix . $path;\n+\tusage(1) if not defined $propname;\n+\tusage(1) if not defined $propval;\n+\tmy $file = basename($path);\n+\tmy $dn = dirname($path);\n+\t# diff has check_attr locally, so just call direct\n+\t#my $current_properties = check_attr( \"svn-properties\", $path );\n+\tmy $current_properties = Git::SVN::Editor::check_attr( \"svn-properties\", $path );\n+\tmy $new_properties = \"\";\n+\tif ($current_properties eq \"unset\" || $current_properties eq \"\" || $current_properties eq \"set\") {\n+\t\t$new_properties = \"$propname=$propval\";\n+\t} else {\n+\t\t# TODO: handle combining properties better\n+\t\tmy @props = split(/;/, $current_properties);\n+\t\tmy $replaced_prop = 0;\n+\t\tforeach my $prop (@props) {\n+\t\t\t# Parse 'name=value' syntax and set the property.\n+\t\t\tif ($prop =~ /([^=]+)=(.*)/) {\n+\t\t\t\tmy ($n,$v) = ($1,$2);\n+\t\t\t\tif ($n eq $propname)\n+\t\t\t\t{\n+\t\t\t\t\t$v = $propval;\n+\t\t\t\t\t$replaced_prop = 1;\n+\t\t\t\t}\n+\t\t\t\tif ($new_properties eq \"\") { $new_properties=\"$n=$v\"; }\n+\t\t\t\telse { $new_properties=\"$new_properties;$n=$v\"; }\n+\t\t\t}\n+\t\t}\n+\t\tif ($replaced_prop eq 0) {\n+\t\t\t$new_properties = \"$new_properties;$propname=$propval\";\n+\t\t}\n+\t}\n+\tmy $attrfile = \"$dn/.gitattributes\";\n+\topen my $attrfh, '>>', $attrfile or die \"Can't open $attrfile: $!\\n\";\n+\t# TODO: don't simply append here if $file already has svn-properties\n+\tprint $attrfh \"$file svn-properties=$new_properties\\n\";\n+}\n+\n # cmd_proplist (PATH)\n # -------------------\n # Print the list of SVN properties for PATH.\ndiff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm\nindex 34e8af9..5158c03 100644\n--- a/perl/Git/SVN/Editor.pm\n+++ b/perl/Git/SVN/Editor.pm\n@@ -288,6 +288,49 @@ sub apply_autoprops {\n \t}\n }\n \n+sub check_attr\n+{\n+    my ($attr,$path) = @_;\n+    if ( open my $fh, '-|', \"git\", \"check-attr\", $attr, \"--\", $path )\n+    {\n+\tmy $val = <$fh>;\n+\tclose $fh;\n+\t$val =~ s/^[^:]*:\\s*[^:]*:\\s*(.*)\\s*$/$1/;\n+\treturn $val;\n+    }\n+    else\n+    {\n+\treturn undef;\n+    }\n+}\n+\n+sub apply_manualprops {\n+\tmy ($self, $file, $fbat) = @_;\n+\tmy $pending_properties = check_attr( \"svn-properties\", $file );\n+\tif ($pending_properties eq \"\") { return; }\n+\t# Parse the list of properties to set.\n+\tmy @props = split(/;/, $pending_properties);\n+\t# TODO: get existing properties to compare to - this fails for add so currently not done\n+\t# my $existing_props = ::get_svnprops($file);\n+\tmy $existing_props = {};\n+\t# TODO: caching svn properties or storing them in .gitattributes would make that faster\n+\tforeach my $prop (@props) {\n+\t\t# Parse 'name=value' syntax and set the property.\n+\t\tif ($prop =~ /([^=]+)=(.*)/) {\n+\t\t\tmy ($n,$v) = ($1,$2);\n+\t\t\tfor ($n, $v) {\n+\t\t\t\ts/^\\s+//; s/\\s+$//;\n+\t\t\t}\n+\t\t\t# FIXME: clearly I don't know perl and couldn't work out how to evaluate this better\n+\t\t\tif (defined $existing_props->{$n} && $existing_props->{$n} eq $v) {\n+\t\t\t\tmy $needed = 0;\n+\t\t\t} else {\n+\t\t\t\t$self->change_file_prop($fbat, $n, $v);\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n sub A {\n \tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n@@ -296,6 +339,7 @@ sub A {\n \t\t\t\t\tundef, -1);\n \tprint \"\\tA\\t$m->{file_b}\\n\" unless $::_q;\n \t$self->apply_autoprops($file, $fbat);\n+\t$self->apply_manualprops($m->{file_b}, $fbat);\n \t$self->chg_file($fbat, $m);\n \t$self->close_file($fbat,undef,$self->{pool});\n }\n@@ -311,6 +355,7 @@ sub C {\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t$upa, $self->{r});\n \tprint \"\\tC\\t$m->{file_a} => $m->{file_b}\\n\" unless $::_q;\n+\t$self->apply_manualprops($m->{file_b}, $fbat);\n \t$self->chg_file($fbat, $m);\n \t$self->close_file($fbat,undef,$self->{pool});\n }\n@@ -333,6 +378,7 @@ sub R {\n \t\t\t\t$upa, $self->{r});\n \tprint \"\\tR\\t$m->{file_a} => $m->{file_b}\\n\" unless $::_q;\n \t$self->apply_autoprops($file, $fbat);\n+\t$self->apply_manualprops($m->{file_b}, $fbat);\n \t$self->chg_file($fbat, $m);\n \t$self->close_file($fbat,undef,$self->{pool});\n \n@@ -348,6 +394,7 @@ sub M {\n \tmy $fbat = $self->open_file($self->repo_path($m->{file_b}),\n \t\t\t\t$pbat,$self->{r},$self->{pool});\n \tprint \"\\t$m->{chg}\\t$m->{file_b}\\n\" unless $::_q;\n+\t$self->apply_manualprops($m->{file_b}, $fbat);\n \t$self->chg_file($fbat, $m);\n \t$self->close_file($fbat,undef,$self->{pool});\n }\n-- \n2.1.2\n"},{"id":"252773","messageId":"20141201094911.GA13931@dcvr.yhbt.net","threadId":"38082","inReplyTo":"20141201062420.GF99906@elvis.mu.org","subject":"Re: [PATCH] git-svn: Support for git-svn propset","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-12-01T09:49:11Z","receivedAt":"2014-12-01T09:49:11Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Alfred Perlstein <alfred@freebsd.org> wrote:\n> This change allows git-svn to support setting subversion properties.\n> \n> Very useful for manually setting properties when committing to a\n> subversion repo that *requires* properties to be set without requiring\n> moving your changeset to separate subversion checkout in order to\n> set props.\n\nThanks Alfred, that's a good reason for supporting this feature\n(something I wasn't convinced of with the original).\n\n> This change is initially from David Fraser <davidf () sjsoft ! com>\n> Appearing here: http://marc.info/?l=git&m=125259772625008&w=2\n\nI've added David to the Cc: (but I never heard back from him regarding\ncomments from his original patch).\n\nAlfred: I had some comments on David's original patch here that\nwere never addressed:\n  http://mid.gmane.org/20090923085812.GA20673@dcvr.yhbt.net\n\nI suspect my concerns about .gitattributes in the source tree from\n2009 are less relevant now in 2014 as git-svn seems more socially\nacceptable in SVN-using projects.\n\nSome of my other concerns about mimicking existing Perl style still\napply, and we have a Perl section in Documenting/CodingGuidelines\nnowadays.\n\n> They are now forward ported to most recent git along with fixes to\n> deal with files in subdirectories.\n> \n>         Developer's Certificate of Origin 1.1\n\n<snip> no need for the whole DCO in the commit message.\njust the S-o-b:\n\n> Signed-off-by: Alfred Perlstein <alfred@freebsd.org>\n\nSince this was originally written by David, his sign-off from the\noriginal email should be here (Cc: for bigger changes)\n\n> ---\n>  git-svn.perl           | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-\n>  perl/Git/SVN/Editor.pm | 47 +++++++++++++++++++++++++++++++++++++++++++++++\n\nWe need a test case written for this new feature.\n\nMost folks (including myself) who were ever involved with git-svn\nrarely use it anymore, and this will likely be rarely-used.\n\n>  2 files changed, 96 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index b6e2186..91423a8 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -115,7 +115,7 @@ my ($_stdin, $_help, $_edit,\n>  \t$_before, $_after,\n>  \t$_merge, $_strategy, $_preserve_merges, $_dry_run, $_parents, $_local,\n>  \t$_prefix, $_no_checkout, $_url, $_verbose,\n> -\t$_commit_url, $_tag, $_merge_info, $_interactive);\n> +\t$_commit_url, $_tag, $_merge_info, $_interactive, $_set_svn_props);\n>  \n>  # This is a refactoring artifact so Git::SVN can get at this git-svn switch.\n>  sub opt_prefix { return $_prefix || '' }\n> @@ -193,6 +193,7 @@ my %cmd = (\n>  \t\t\t  'dry-run|n' => \\$_dry_run,\n>  \t\t\t  'fetch-all|all' => \\$_fetch_all,\n>  \t\t\t  'commit-url=s' => \\$_commit_url,\n> +\t\t\t  'set-svn-props=s' => \\$_set_svn_props,\n>  \t\t\t  'revision|r=i' => \\$_revision,\n>  \t\t\t  'no-rebase' => \\$_no_rebase,\n>  \t\t\t  'mergeinfo=s' => \\$_merge_info,\n> @@ -228,6 +229,9 @@ my %cmd = (\n>          'propget' => [ \\&cmd_propget,\n>  \t\t       'Print the value of a property on a file or directory',\n>  \t\t       { 'revision|r=i' => \\$_revision } ],\n> +        'propset' => [ \\&cmd_propset,\n> +\t\t       'Set the value of a property on a file or directory - will be set on commit',\n> +\t\t       {} ],\n>          'proplist' => [ \\&cmd_proplist,\n>  \t\t       'List all properties of a file or directory',\n>  \t\t       { 'revision|r=i' => \\$_revision } ],\n> @@ -1376,6 +1380,50 @@ sub cmd_propget {\n>  \tprint $props->{$prop} . \"\\n\";\n>  }\n>  \n> +# cmd_propset (PROPNAME, PROPVAL, PATH)\n> +# ------------------------\n> +# Adjust the SVN property PROPNAME to PROPVAL for PATH.\n> +sub cmd_propset {\n> +\tmy ($propname, $propval, $path) = @_;\n> +\t$path = '.' if not defined $path;\n> +\t$path = $cmd_dir_prefix . $path;\n> +\tusage(1) if not defined $propname;\n> +\tusage(1) if not defined $propval;\n> +\tmy $file = basename($path);\n> +\tmy $dn = dirname($path);\n> +\t# diff has check_attr locally, so just call direct\n> +\t#my $current_properties = check_attr( \"svn-properties\", $path );\n\nI prefer leaving out dead code entirely instead of leaving it commented out.\n\n> +\tmy $current_properties = Git::SVN::Editor::check_attr( \"svn-properties\", $path );\n> +\tmy $new_properties = \"\";\n\nSince some lines are too long, local variables can be shortened\nto cur_props, new_props without being any less descriptive.\n\n> +\tif ($current_properties eq \"unset\" || $current_properties eq \"\" || $current_properties eq \"set\") {\n> +\t\t$new_properties = \"$propname=$propval\";\n> +\t} else {\n> +\t\t# TODO: handle combining properties better\n> +\t\tmy @props = split(/;/, $current_properties);\n> +\t\tmy $replaced_prop = 0;\n> +\t\tforeach my $prop (@props) {\n> +\t\t\t# Parse 'name=value' syntax and set the property.\n> +\t\t\tif ($prop =~ /([^=]+)=(.*)/) {\n> +\t\t\t\tmy ($n,$v) = ($1,$2);\n> +\t\t\t\tif ($n eq $propname)\n> +\t\t\t\t{\n> +\t\t\t\t\t$v = $propval;\n> +\t\t\t\t\t$replaced_prop = 1;\n> +\t\t\t\t}\n> +\t\t\t\tif ($new_properties eq \"\") { $new_properties=\"$n=$v\"; }\n> +\t\t\t\telse { $new_properties=\"$new_properties;$n=$v\"; }\n> +\t\t\t}\n> +\t\t}\n> +\t\tif ($replaced_prop eq 0) {\n\nGenerally, == is favored for numeric comparisons\n(but this is boolean)\n\n> +\t\t\t$new_properties = \"$new_properties;$propname=$propval\";\n> +\t\t}\n> +\t}\n> +\tmy $attrfile = \"$dn/.gitattributes\";\n> +\topen my $attrfh, '>>', $attrfile or die \"Can't open $attrfile: $!\\n\";\n> +\t# TODO: don't simply append here if $file already has svn-properties\n> +\tprint $attrfh \"$file svn-properties=$new_properties\\n\";\n\nWould be good to have an explicit close and error checking on it in case\nof FS errors\n\n> +}\n> +\n>  # cmd_proplist (PATH)\n>  # -------------------\n>  # Print the list of SVN properties for PATH.\n> diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm\n> index 34e8af9..5158c03 100644\n> --- a/perl/Git/SVN/Editor.pm\n> +++ b/perl/Git/SVN/Editor.pm\n> @@ -288,6 +288,49 @@ sub apply_autoprops {\n>  \t}\n>  }\n>  \n> +sub check_attr\n> +{\n> +    my ($attr,$path) = @_;\n> +    if ( open my $fh, '-|', \"git\", \"check-attr\", $attr, \"--\", $path )\n\nUse Git.pm (command_output_pipe) for running git commands portably.\n(And formatting for this sub alone is really off).\n\n> +    {\n> +\tmy $val = <$fh>;\n> +\tclose $fh;\n> +\t$val =~ s/^[^:]*:\\s*[^:]*:\\s*(.*)\\s*$/$1/;\n> +\treturn $val;\n> +    }\n> +    else\n> +    {\n> +\treturn undef;\n> +    }\n> +}\n> +\n> +sub apply_manualprops {\n> +\tmy ($self, $file, $fbat) = @_;\n> +\tmy $pending_properties = check_attr( \"svn-properties\", $file );\n> +\tif ($pending_properties eq \"\") { return; }\n> +\t# Parse the list of properties to set.\n> +\tmy @props = split(/;/, $pending_properties);\n> +\t# TODO: get existing properties to compare to - this fails for add so currently not done\n> +\t# my $existing_props = ::get_svnprops($file);\n> +\tmy $existing_props = {};\n> +\t# TODO: caching svn properties or storing them in .gitattributes would make that faster\n> +\tforeach my $prop (@props) {\n> +\t\t# Parse 'name=value' syntax and set the property.\n> +\t\tif ($prop =~ /([^=]+)=(.*)/) {\n> +\t\t\tmy ($n,$v) = ($1,$2);\n> +\t\t\tfor ($n, $v) {\n> +\t\t\t\ts/^\\s+//; s/\\s+$//;\n> +\t\t\t}\n\nI'd probably rewrite the following hunk:\n\n> +\t\t\t# FIXME: clearly I don't know perl and couldn't work out how to evaluate this better\n> +\t\t\tif (defined $existing_props->{$n} && $existing_props->{$n} eq $v) {\n> +\t\t\t\tmy $needed = 0;\n> +\t\t\t} else {\n> +\t\t\t\t$self->change_file_prop($fbat, $n, $v);\n> +\t\t\t}\n\nas:\n\n\t\t\tmy $existing = $existing_props->{$n};\n\t\t\tif (!defined($existing) || $existing ne $v) {\n\t\t\t\t$self->change_file_prop($fbat, $n, $v);\n\t\t\t}\n\nNo need for setting $needed = 0 from what I can tell.\n\nRest of the patch looks fine.\n\nThank you again for bringing this up and I look forward to a reroll\nof this with my comments addressed (maybe wait a few days for others\nto comment, holidays and all).\n"}]}