Re: [PATCH v2 2/2] git-svn: allow empty email-address in authors-prog and authors-file
Andreas Heidukwrote: > Am 19.03.2018 um 00:04 schrieb Eric Wong: > > Andreas Heiduk wrote: > >> 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. > > That's still available and it's even still the default. OK. > 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. > 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 OK, that's a good reason to allow "<>" and should be in the commit message. > 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 :<
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: > Andreas Heidukwrote: >> 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. 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: > git -c user.email= commit --allow-empty -m "foo" > git show --format=raw HEAD | egrep "author|committer" author jondoe <> 1521495217 +0100 committer jondoe <> 1521495217 +0100 Doing 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. > > "<>" 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 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+egd9tgudvp7npj3uyxy1pl...@mail.gmail.com/
Re: [PATCH v2 2/2] git-svn: allow empty email-address in authors-prog and authors-file
Andreas Heidukwrote: > 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.
[PATCH v2 2/2] git-svn: allow empty email-address in authors-prog and authors-file
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. Signed-off-by: Andreas Heiduk--- 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(-) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index b858374649..d59379ee23 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -635,7 +635,8 @@ config key: svn.findcopiesharder -A:: --authors-file=:: - 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 @@ -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 ", - which will be treated as if included in the authors file. + expected to return a single line of the form "Name " 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 diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index bc4eed3d75..945ca4db2b 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -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; diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh index 41264818cc..6af6daf461 100755 --- a/t/t9130-git-svn-authors-file.sh +++ b/t/t9130-git-svn-authors-file.sh @@ -108,6 +108,20 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' ' ) ' +cat >> svn-authors < +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 FFF FFF <> " + ) + ' + test_debug 'GIT_DIR=gitconfig.clone/.git git log' test_done diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh index 7d7e9d46bc..0cec56128f 100755 --- a/t/t9138-git-svn-authors-prog.sh +++ b/t/t9138-git-svn-authors-prog.sh @@ -9,7 +9,9 @@ test_description='git svn authors prog tests'