git-svn: --author-prog improvements
A message that is not in the archive (sent off-list or not yet received): [email protected]
v2 cover git-svn: --author-prog improvements
The first patch has been queued by Eric Wong but by Junio Hamano, so I'm not sure what's the expected procedure. I#M posting it again just in case.
The second patch has grown up with some documentation and some tests.
Andreas Heiduk (2):
git-svn: search --authors-prog in PATH too
git-svn: allow empty email-address in authors-prog and authors-file
Documentation/git-svn.txt | 13 ++++++++++--- git-svn.perl | 3 ++- perl/Git/SVN.pm | 13 ++++++------- t/t9130-git-svn-authors-file.sh | 14 ++++++++++++++ t/t9138-git-svn-authors-prog.sh | 25 ++++++++++++++++++++++++- 5 files changed, 56 insertions(+), 12 deletions(-)
Signature · 1 line
2.16.2
PATCH v2 1/2 git-svn: search --authors-prog in PATH too
In 36db1eddf9 ("git-svn: add --authors-prog option", 2009-05-14) the path to authors-prog was made absolute because git-svn changes the current directory in some situations. This makes sense if the program is part of the repository but prevents searching via $PATH.
The old behaviour is still retained, but if the file does not exists, then authors-prog is searched for in $PATH as any other command.
--- Documentation/git-svn.txt | 5 +++++ git-svn.perl | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-)
Documentation/git-svn.txt +5 -0
@@ -657,6 +657,11 @@ config key: svn.authorsfile expected to return a single line of the form "Name <email>", which will be treated as if included in the authors file. + +Due to historical reasons a relative 'filename' is first searched +relative to the current directory for 'init' and 'clone' and relative +to the root of the working tree for 'fetch'. If 'filename' is +not found, it is searched like any other command in '$PATH'. ++ [verse] config key: svn.authorsProg
git-svn.perl +2 -1
@@ -374,7 +374,8 @@ version() if $_version; usage(1) unless defined $cmd; load_authors() if $_authors; if (defined $_authors_prog) { - $_authors_prog = "'" . File::Spec->rel2abs($_authors_prog) . "'"; + my $abs_file = File::Spec->rel2abs($_authors_prog); + $_authors_prog = "'" . $abs_file . "'" if -x $abs_file; } unless ($cmd =~ /^(?:clone|init|multi-init|commit-diff)$/) {
Signature · 1 line
2.16.2
PATCH v2 2/2 git-svn: allow empty email-address in authors-prog and authors-file
In reply to Andreas HeidukThe email address in --authors-file and --authors-prog can be empty but git-svn translated it into a syntethic email address in the form $USERNAME@$REPO_UUID. Now git-svn behaves like git-commit: If the email is explicitly set to the empty string, the commit does not contain an email address.
--- Documentation/git-svn.txt | 8 +++++--- perl/Git/SVN.pm | 13 ++++++------- t/t9130-git-svn-authors-file.sh | 14 ++++++++++++++ t/t9138-git-svn-authors-prog.sh | 25 ++++++++++++++++++++++++- 4 files changed, 49 insertions(+), 11 deletions(-)
Documentation/git-svn.txt +5 -3
@@ -635,7 +635,8 @@ config key: svn.findcopiesharder -A<filename>:: --authors-file=<filename>:: - Syntax is compatible with the file used by 'git cvsimport': + Syntax is compatible with the file used by 'git cvsimport' but + an empty email address can be supplied with '<>': + ------------------------------------------------------------------------ loginname = Joe User <[email protected]>
@@ -654,8 +655,9 @@ config key: svn.authorsfile If this option is specified, for each SVN committer name that does not exist in the authors file, the given file is executed with the committer name as the first argument. The program is - expected to return a single line of the form "Name <email>", - which will be treated as if included in the authors file. + expected to return a single line of the form "Name <email>" or + "Name <>", which will be treated as if included in the authors + file. + Due to historical reasons a relative 'filename' is first searched relative to the current directory for 'init' and 'clone' and relative
perl/Git/SVN.pm +6 -7
@@ -1482,7 +1482,6 @@ sub call_authors_prog { } if ($author =~ /^\s*(.+?)\s*<(.*)>\s*$/) { my ($name, $email) = ($1, $2); - $email = undef if length $2 == 0; return [$name, $email]; } else { die "Author: $orig_author: $::_authors_prog returned "
@@ -2020,8 +2019,8 @@ sub make_log_entry { remove_username($full_url); $log_entry{metadata} = "$full_url\@$r $uuid"; $log_entry{svm_revision} = $r; - $email ||= "$author\@$uuid"; - $commit_email ||= "$author\@$uuid"; + $email = "$author\@$uuid" unless defined $email; + $commit_email = "$author\@$uuid" unless defined $commit_email; } elsif ($self->use_svnsync_props) { my $full_url = canonicalize_url( add_path_to_url( $self->svnsync->{url}, $self->path )
@@ -2029,15 +2028,15 @@ sub make_log_entry { remove_username($full_url); my $uuid = $self->svnsync->{uuid}; $log_entry{metadata} = "$full_url\@$rev $uuid"; - $email ||= "$author\@$uuid"; - $commit_email ||= "$author\@$uuid"; + $email = "$author\@$uuid" unless defined $email; + $commit_email = "$author\@$uuid" unless defined $commit_email; } else { my $url = $self->metadata_url; remove_username($url); my $uuid = $self->rewrite_uuid || $self->ra->get_uuid; $log_entry{metadata} = "$url\@$rev " . $uuid; - $email ||= "$author\@" . $uuid; - $commit_email ||= "$author\@" . $uuid; + $email = "$author\@$uuid" unless defined $email; + $commit_email = "$author\@$uuid" unless defined $commit_email; } $log_entry{name} = $name; $log_entry{email} = $email;
t/t9130-git-svn-authors-file.sh +14 -0
@@ -108,6 +108,20 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' ' ) ' +cat >> svn-authors <<EOF +ff = FFFFFFF FFFFFFF <> +EOF + +test_expect_success 'authors-file imported user without email' ' + svn_cmd mkdir -m aa/branches/ff --username ff "$svnrepo/aa/branches/ff" && + ( + cd aa-work && + git svn fetch --authors-file=../svn-authors && + git rev-list -1 --pretty=raw refs/remotes/origin/ff | \ + grep "^author FFFFFFF FFFFFFF <> " + ) + ' + test_debug 'GIT_DIR=gitconfig.clone/.git git log' test_done
t/t9138-git-svn-authors-prog.sh +24 -1
@@ -9,7 +9,9 @@ test_description='git svn authors prog tests' write_script svn-authors-prog "$PERL_PATH" <<-\EOF $_ = shift; - if (s/-sub$//) { + if (s/-hermit//) { + print "$_ <>\n"; + } elsif (s/-sub$//) { print "$_ <$_\@sub.example.com>\n"; } else { print "$_ <$_\@example.com>\n";
@@ -68,6 +70,27 @@ test_expect_success 'authors-file overrode authors-prog' ' git --git-dir=x/.git config --unset svn.authorsfile git --git-dir=x/.git config --unset svn.authorsprog +test_expect_success 'authors-prog imported user without email' ' + svn mkdir -m gg --username gg-hermit "$svnrepo"/gg && + ( + cd x && + git svn fetch --authors-prog=../svn-authors-prog && + git rev-list -1 --pretty=raw refs/remotes/git-svn | \ + grep "^author gg <> " + ) +' + +test_expect_success 'imported without authors-prog and authors-file' ' + svn mkdir -m hh --username hh "$svnrepo"/hh && + ( + uuid=$(svn info --show-item=repos-uuid "$svnrepo") && + cd x && + git svn fetch && + git rev-list -1 --pretty=raw refs/remotes/git-svn | \ + grep "^author hh <hh@$uuid> " + ) +' + test_expect_success 'authors-prog handled special characters in username' ' svn mkdir -m bad --username "xyz; touch evil" "$svnrepo"/bad && (
Signature · 1 line
2.16.2
Re: [PATCH v2 2/2] git-svn: allow empty email-address in authors-prog and authors-file
Quoted 5 lines · Andreas Heiduk
The email address in --authors-file and --authors-prog can be empty but git-svn translated it into a syntethic email address in the form $USERNAME@$REPO_UUID. Now git-svn behaves like git-commit: If the email is explicitly set to the empty string, the commit does not contain an email address.
What is missing is WHY "<>" is preferable to "<$USERNAME@$REPO_UUID>".
$USERNAME is good anyways since projects/organizations tie their SVN usernames to email usernames via LDAP, making it easy to infer their email address from $USERNAME. The latter can also be used to disambiguate authors if they happen to have the same real name.
"<>" is completely meaningless.
Re: [PATCH v2 2/2] git-svn: allow empty email-address in authors-prog and authors-file
Am 19.03.2018 um 00:04 schrieb Eric Wong:
Quoted 7 lines
Quoted 5 lines · Andreas Heiduk
The email address in --authors-file and --authors-prog can be empty but git-svn translated it into a syntethic email address in the form $USERNAME@$REPO_UUID. Now git-svn behaves like git-commit: If the email is explicitly set to the empty string, the commit does not contain an email address.
Quoted 7 lines
What is missing is WHY "<>" is preferable to "<$USERNAME@$REPO_UUID>".
$USERNAME is good anyways since projects/organizations tie their SVN usernames to email usernames via LDAP, making it easy to infer their email address from $USERNAME. The latter can also be used to disambiguate authors if they happen to have the same real name.
That's still available and it's even still the default.
But: If the user of git-svn takes the burden of writing an authors script or maintaining an authors file then he should have full control over the result as long as git can handle the output reasonably. Currently that's the case for git but not for git-svn.
Git can handle empty emails quite nicely:
Quoted 2 lines
git -c user.email= commit --allow-empty -m "foo" git show --format=raw HEAD | egrep "author|committer"
author jondoe <> 1521495217 +0100
committer jondoe <> 1521495217 +0100Doing the same with current git-svn requires a filter-branch followed by rm -r .git/svn/ followed by git svn fetch to recreate the rev_map files. That would be feasible for a one-time conversion but not in a situation where SVN is live and the master repository.
Quoted 3 lines
"<>" is completely meaningless.
Not quite. The "<>" is not the only information - there is still the mandatory "name" part. So the commit id
jondoe <>
just means: "There is intentionally no email address." For an internal, ephemeral repository that can be OK. It has the advantage, that no automatic system (Jira, Jenkins, ...) will try to send emails to
jondoe <jondoe@6aafaa21e0fb4338a68ab372a049893d>
Additionally the log output isn't cluttered with irrelevant stuff. :-)
And last but not least we don't have to hunt down names long gone by and already deleted in LDAP. In that case the UUID doesn't help either.
Further steps: Eric Sunshine mentioned [1] that you might have concerns about the change of behavior per se. For me the patch is not so much a new feature but a bugfix bringing git-svn in sync with git itself. Adding an option parameter to enable the new behavior seems strange to me. But there might be other ways to achieve the same effect:
- changing the output format of the file and prog: empty emails could be marked by a syntax which is invalid so far.
- OR (if some change of behaviour is acceptable) the script could evaluate a new environment variable like GIT_SVN_UUID to compose the <$user@$uuid> part itself.
- OR just mention it in the relaese notes ;-)
- OR [please insert ideas here]
[1] https://public-inbox.org/git/CAPig+cQ1Si-AvazF_1Kf4yX9+eGD9tGUdvP7npJ3uYxy1pLy2w@mail.gmail.com/
Re: [PATCH v2 2/2] git-svn: allow empty email-address in authors-prog and authors-file
Quoted 10 lines · Andreas Heiduk
Am 19.03.2018 um 00:04 schrieb Eric Wong:
Quoted 9 lines
Quoted 5 lines · Andreas Heiduk
The email address in --authors-file and --authors-prog can be empty but git-svn translated it into a syntethic email address in the form $USERNAME@$REPO_UUID. Now git-svn behaves like git-commit: If the email is explicitly set to the empty string, the commit does not contain an email address.
What is missing is WHY "<>" is preferable to "<$USERNAME@$REPO_UUID>".
Quoted 7 lines
Quoted 5 lines
$USERNAME is good anyways since projects/organizations tie their SVN usernames to email usernames via LDAP, making it easy to infer their email address from $USERNAME. The latter can also be used to disambiguate authors if they happen to have the same real name.
That's still available and it's even still the default.
OK.
Quoted 4 lines
But: If the user of git-svn takes the burden of writing an authors script or maintaining an authors file then he should have full control over the result as long as git can handle the output reasonably. Currently that's the case for git but not for git-svn.
Fair enough.
Quoted 7 lines
jondoe <>
just means: "There is intentionally no email address." For an internal, ephemeral repository that can be OK. It has the advantage, that no automatic system (Jira, Jenkins, ...) will try to send emails to
jondoe <jondoe@6aafaa21e0fb4338a68ab372a049893d>
OK, that's a good reason to allow "<>" and should be in the commit message.
Quoted 5 lines
Further steps: Eric Sunshine mentioned [1] that you might have concerns about the change of behavior per se. For me the patch is not so much a new feature but a bugfix bringing git-svn in sync with git itself. Adding an option parameter to enable the new behavior seems strange to me. But there might be other ways to achieve the same effect:
New options are not desirable, either, as they increase testing/maintenance overhead. So I'm inclined to take your patch with only an updated commit message...
No rush, though; will wait another bit for others to comment and I expect to be preoccupied this week with other projects and weather problems on the forecast :<
PATCH v3 1/1 git-svn: allow empty email-address using authors-prog and authors-file
The email address in --authors-file and --authors-prog can be empty but git-svn translated it into a fictional email address in the form
jondoe <jondoe@6aafaa21e0fb4338a68ab372a049893d>
containing the SVN repository UUID. Now git-svn behaves like git-commit:
If the email is *explicitly* set to the empty string using '<>', the commit does not contain an email address, only the name:
jondoe <>
Allowing to remove the email address *intentionally* prevents automatic systems from sending emails to those fictional addresses and avoids cluttering the log output with unnecessary stuff.
--- Documentation/git-svn.txt | 8 +++++--- perl/Git/SVN.pm | 13 ++++++------- t/t9130-git-svn-authors-file.sh | 14 ++++++++++++++ t/t9138-git-svn-authors-prog.sh | 25 ++++++++++++++++++++++++- 4 files changed, 49 insertions(+), 11 deletions(-)
Documentation/git-svn.txt +5 -3
@@ -635,7 +635,8 @@ config key: svn.findcopiesharder -A<filename>:: --authors-file=<filename>:: - Syntax is compatible with the file used by 'git cvsimport': + Syntax is compatible with the file used by 'git cvsimport' but + an empty email address can be supplied with '<>': + ------------------------------------------------------------------------ loginname = Joe User <[email protected]>
@@ -654,8 +655,9 @@ config key: svn.authorsfile If this option is specified, for each SVN committer name that does not exist in the authors file, the given file is executed with the committer name as the first argument. The program is - expected to return a single line of the form "Name <email>", - which will be treated as if included in the authors file. + expected to return a single line of the form "Name <email>" or + "Name <>", which will be treated as if included in the authors + file. + Due to historical reasons a relative 'filename' is first searched relative to the current directory for 'init' and 'clone' and relative
perl/Git/SVN.pm +6 -7
@@ -1482,7 +1482,6 @@ sub call_authors_prog { } if ($author =~ /^\s*(.+?)\s*<(.*)>\s*$/) { my ($name, $email) = ($1, $2); - $email = undef if length $2 == 0; return [$name, $email]; } else { die "Author: $orig_author: $::_authors_prog returned "
@@ -2020,8 +2019,8 @@ sub make_log_entry { remove_username($full_url); $log_entry{metadata} = "$full_url\@$r $uuid"; $log_entry{svm_revision} = $r; - $email ||= "$author\@$uuid"; - $commit_email ||= "$author\@$uuid"; + $email = "$author\@$uuid" unless defined $email; + $commit_email = "$author\@$uuid" unless defined $commit_email; } elsif ($self->use_svnsync_props) { my $full_url = canonicalize_url( add_path_to_url( $self->svnsync->{url}, $self->path )
@@ -2029,15 +2028,15 @@ sub make_log_entry { remove_username($full_url); my $uuid = $self->svnsync->{uuid}; $log_entry{metadata} = "$full_url\@$rev $uuid"; - $email ||= "$author\@$uuid"; - $commit_email ||= "$author\@$uuid"; + $email = "$author\@$uuid" unless defined $email; + $commit_email = "$author\@$uuid" unless defined $commit_email; } else { my $url = $self->metadata_url; remove_username($url); my $uuid = $self->rewrite_uuid || $self->ra->get_uuid; $log_entry{metadata} = "$url\@$rev " . $uuid; - $email ||= "$author\@" . $uuid; - $commit_email ||= "$author\@" . $uuid; + $email = "$author\@$uuid" unless defined $email; + $commit_email = "$author\@$uuid" unless defined $commit_email; } $log_entry{name} = $name; $log_entry{email} = $email;
t/t9130-git-svn-authors-file.sh +14 -0
@@ -108,6 +108,20 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' ' ) ' +cat >> svn-authors <<EOF +ff = FFFFFFF FFFFFFF <> +EOF + +test_expect_success 'authors-file imported user without email' ' + svn_cmd mkdir -m aa/branches/ff --username ff "$svnrepo/aa/branches/ff" && + ( + cd aa-work && + git svn fetch --authors-file=../svn-authors && + git rev-list -1 --pretty=raw refs/remotes/origin/ff | \ + grep "^author FFFFFFF FFFFFFF <> " + ) + ' + test_debug 'GIT_DIR=gitconfig.clone/.git git log' test_done
t/t9138-git-svn-authors-prog.sh +24 -1
@@ -9,7 +9,9 @@ test_description='git svn authors prog tests' write_script svn-authors-prog "$PERL_PATH" <<-\EOF $_ = shift; - if (s/-sub$//) { + if (s/-hermit//) { + print "$_ <>\n"; + } elsif (s/-sub$//) { print "$_ <$_\@sub.example.com>\n"; } else { print "$_ <$_\@example.com>\n";
@@ -68,6 +70,27 @@ test_expect_success 'authors-file overrode authors-prog' ' git --git-dir=x/.git config --unset svn.authorsfile git --git-dir=x/.git config --unset svn.authorsprog +test_expect_success 'authors-prog imported user without email' ' + svn mkdir -m gg --username gg-hermit "$svnrepo"/gg && + ( + cd x && + git svn fetch --authors-prog=../svn-authors-prog && + git rev-list -1 --pretty=raw refs/remotes/git-svn | \ + grep "^author gg <> " + ) +' + +test_expect_success 'imported without authors-prog and authors-file' ' + svn mkdir -m hh --username hh "$svnrepo"/hh && + ( + uuid=$(svn info --show-item=repos-uuid "$svnrepo") && + cd x && + git svn fetch && + git rev-list -1 --pretty=raw refs/remotes/git-svn | \ + grep "^author hh <hh@$uuid> " + ) +' + test_expect_success 'authors-prog handled special characters in username' ' svn mkdir -m bad --username "xyz; touch evil" "$svnrepo"/bad && (
Signature · 1 line
2.16.2
Re: [PATCH v3] git-svn: allow empty email-address using authors-prog and authors-file
Thanks for the update. The patch itself looks good, but I noticed one --show-item isn't supported on SVN 1.8.10 for me.
I've tested the following on both SVN 1.8.10 and 1.9.5:
t/t9138-git-svn-authors-prog.sh +2 -1
@@ -83,7 +83,8 @@ test_expect_success 'authors-prog imported user without email' ' test_expect_success 'imported without authors-prog and authors-file' ' svn mkdir -m hh --username hh "$svnrepo"/hh && ( - uuid=$(svn info --show-item=repos-uuid "$svnrepo") && + uuid=$(svn info "$svnrepo" | + sed -n "s/^Repository UUID: //p") && cd x && git svn fetch && git rev-list -1 --pretty=raw refs/remotes/git-svn | \
Can you confirm it's OK for you? Thanks.
Re: [PATCH v3] git-svn: allow empty email-address using authors-prog and authors-file
Am 05.04.2018 um 09:51 schrieb Eric Wong:
Quoted 2 lines
Thanks for the update. The patch itself looks good, but I noticed one --show-item isn't supported on SVN 1.8.10 for me.
--show-item is indeed a 1.9.0 thing:
https://subversion.apache.org/docs/release-notes/1.9.html#svn-info-item
Quoted 15 lines
I've tested the following on both SVN 1.8.10 and 1.9.5:
t/t9138-git-svn-authors-prog.sh +2
-1@@ -83,7 +83,8 @@ test_expect_success 'authors-prog imported user without email' ' test_expect_success 'imported without authors-prog and authors-file' ' svn mkdir -m hh --username hh "$svnrepo"/hh && ( - uuid=$(svn info --show-item=repos-uuid "$svnrepo") && + uuid=$(svn info "$svnrepo" | + sed -n "s/^Repository UUID: //p") && cd x && git svn fetch && git rev-list -1 --pretty=raw refs/remotes/git-svn | \
Quoted 1 line
Can you confirm it's OK for you? Thanks.
Looks good, works for me.
Do you squash this patch with with my commit or do you need a reroll?
Re: [PATCH v3] git-svn: allow empty email-address using authors-prog and authors-file
Quoted 6 lines · Andreas Heiduk
Am 05.04.2018 um 09:51 schrieb Eric Wong:
Quoted 1 line
Can you confirm it's OK for you? Thanks.
Looks good, works for me.
Do you squash this patch with with my commit or do you need a reroll?
Nope, no need to reroll. Pushed to my repo for Junio. Thanks all.
The following changes since commit 468165c1d8a442994a825f3684528361727cd8c0:
Git 2.17 (2018-04-02 10:13:35 -0700)
are available in the Git repository at:
git://bogomips.org/git-svn.git svn/authors-prog-2
for you to fetch changes up to cb427e9eb0243fe7a1a22ea3bd0a46b7410c0bf3:
git-svn: allow empty email-address using authors-prog and authors-file (2018-04-05 19:22:06 +0000)
----------------------------------------------------------------
Andreas Heiduk (2):
git-svn: search --authors-prog in PATH too
git-svn: allow empty email-address using authors-prog and authors-file
Documentation/git-svn.txt | 13 ++++++++++--- git-svn.perl | 3 ++- perl/Git/SVN.pm | 13 ++++++------- t/t9130-git-svn-authors-file.sh | 14 ++++++++++++++ t/t9138-git-svn-authors-prog.sh | 26 +++++++++++++++++++++++++- 5 files changed, 57 insertions(+), 12 deletions(-)
Re: [PATCH v3] git-svn: allow empty email-address using authors-prog and authors-file
Quoted 14 lines · Eric Wong
Quoted 6 lines · Andreas Heiduk
Am 05.04.2018 um 09:51 schrieb Eric Wong:
Quoted 1 line
Can you confirm it's OK for you? Thanks.
Looks good, works for me.
Do you squash this patch with with my commit or do you need a reroll?
Nope, no need to reroll. Pushed to my repo for Junio. Thanks all.
The following changes since commit
468165c1d8a442994a825f3684528361727cd8c0:Git 2.17 (2018-04-02 10:13:35 -0700)
Quoted 7 lines
are available in the Git repository at:
git://bogomips.org/git-svn.gitsvn/authors-prog-2for you to fetch changes up to
cb427e9eb0243fe7a1a22ea3bd0a46b7410c0bf3:git-svn: allow empty email-address using authors-prog and authors-file (2018-04-05 19:22:06 +0000)
Sorry; this message fell under my radar and I had to privately get reminded of it. Pulled.
Thanks, both.
No comments on this one?
Quoted 1 line · Andreas Heiduk
No comments on this one?
I can't speak for Eric W., but my impression was that v2 did not address his concern that patch 2/2's unconditional change of behavior was unacceptable[1]. He didn't say so explicitly, but perhaps the implication was that he might be more open to the behavior change being an opt-in.
[1]: https://public-inbox.org/git/20180305202017.GA26533@whir/