From: Alfred Perlstein <alfred@freebsd.org>
To: Eric Wong <normalperson@yhbt.net>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
"Michael G. Schwern" <schwern@pobox.com>,
David Fraser <davidf@sjsoft.com>
Subject: Re: [PATCH] git-svn: Support for git-svn propset
Date: Sun, 07 Dec 2014 01:52:30 -0800 [thread overview]
Message-ID: <5484235E.5070003@freebsd.org> (raw)
In-Reply-To: <20141207054211.GA25793@dcvr.yhbt.net>
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
>
next prev parent reply other threads:[~2014-12-07 9:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-06 22:29 [PATCH] git-svn: Support for git-svn propset Alfred Perlstein
2014-12-07 5:42 ` Eric Wong
2014-12-07 9:52 ` Alfred Perlstein [this message]
2014-12-07 5:45 ` Torsten Bögershausen
2014-12-07 8:00 ` Torsten Bögershausen
2014-12-07 9:23 ` Eric Sunshine
-- strict thread matches above, loose matches on Subject: below --
2014-12-07 10:47 Alfred Perlstein
2014-12-08 21:36 ` Eric Wong
2014-12-08 23:43 ` Alfred Perlstein
2014-12-01 6:24 Alfred Perlstein
2014-12-01 9:49 ` Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5484235E.5070003@freebsd.org \
--to=alfred@freebsd.org \
--cc=davidf@sjsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=normalperson@yhbt.net \
--cc=schwern@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).