{"thread":{"id":"31166","subject":"[PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","startedAt":"2012-08-01T21:23:17Z","lastAt":"2012-08-26T00:36:58Z","messageCount":13,"participants":["Robert Luberda","Eric Wong","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196308","messageId":"1343856197-6393-1-git-send-email-robert@debian.org","threadId":"31166","inReplyTo":null,"subject":"[PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2012-08-01T21:23:17Z","receivedAt":"2012-08-01T21:23:17Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"While importing changes from SVN by `git svn fetch' strip any\nwhite spaces from beginnings and endings of SVN commit messages\nand skip adding a new line character before `git-svn-id:'\nline in case the commit message ends with another pseudo-header\n(like From:, Signed-off-by: or Change-Id:, etc.).\n\nThis patch allows one to use gerrit code review system on git-svn-managed\nrepositories. gerrit expects its `Change-Id:' header to appear in the\nlast paragraph of commit message and `git-svn-id:' following a new\nline character was breaking this expectation.\n---\n perl/Git/SVN.pm | 5 +-\n t/t9122-git-svn-author.sh | 4 +-\n t/t9163-git-svn-import-messages.sh | 174 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 180 insertions(+), 3 deletions(-)\n create mode 100755 t/t9163-git-svn-import-messages.sh\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex b8b3474..bf22408 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1015,7 +1015,8 @@ sub do_git_commit {\n \tprint $msg_fh $log_entry->{log} or croak $!;\n \trestore_commit_header_env($old_env);\n \tunless ($self->no_metadata) {\n-\t\tprint $msg_fh \"\\ngit-svn-id: $log_entry->{metadata}\\n\"\n+\t\tprint $msg_fh \"\\n\" unless $log_entry->{log} =~ m/\\n\\n([\\w-]+:\\s.*\\n)+$/;\n+\t\tprint $msg_fh \"git-svn-id: $log_entry->{metadata}\\n\"\n \t\t or croak $!;\n \t}\n \t$msg_fh->flush == 0 or croak $!;\n@@ -1803,6 +1804,8 @@ sub make_log_entry {\n \tclose $un or croak $!;\n \n \t$log_entry{date} = parse_svn_date($log_entry{date});\n+\t$log_entry{log} =~ s/^\\s*//;\n+\t$log_entry{log} =~ s/\\s*$//;\n \t$log_entry{log} .= \"\\n\";\n \tmy $author = $log_entry{author} = check_author($log_entry{author});\n \tmy ($name, $email) = defined $::users{$author} ? @{$::users{$author}}\ndiff --git a/t/t9122-git-svn-author.sh b/t/t9122-git-svn-author.sh\nindex 30013b7..c1d55eb 100755\n--- a/t/t9122-git-svn-author.sh\n+++ b/t/t9122-git-svn-author.sh\n@@ -68,8 +68,8 @@ test_expect_success 'interact with it via git svn' '\n \n \t# Make sure there are no commit messages with excess blank lines\n \ttest $(grep \"^ \" actual.2 | wc -l) = 3 &&\n-\ttest $(grep \"^ \" actual.3 | wc -l) = 5 &&\n-\ttest $(grep \"^ \" actual.4 | wc -l) = 5 &&\n+\ttest $(grep \"^ \" actual.3 | wc -l) = 4 &&\n+\ttest $(grep \"^ \" actual.4 | wc -l) = 4 &&\n \n \t# Make sure there are no svn commit messages with excess blank lines\n \t(\ndiff --git a/t/t9163-git-svn-import-messages.sh b/t/t9163-git-svn-import-messages.sh\nnew file mode 100755\nindex 0000000..46b7c5b\n--- /dev/null\n+++ b/t/t9163-git-svn-import-messages.sh\n@@ -0,0 +1,174 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2012 Robert Luberda\n+#\n+\n+test_description='git svn check log messages imported from svn'\n+. ./lib-git-svn.sh\n+\n+get_file_contents()\n+{\n+\tfor line in \"$@\"; do\n+\t\techo \"$line\"\n+\tdone\n+}\n+\n+svn_commit()\n+{\n+\tN=`expr $N + 1`\n+\tget_file_contents \"$@\" > svn-message.$N;\n+\t(cd work.svn && echo \"$N\" >> file &&\n+\tsvn_cmd commit -F ../svn-message.$N file)\n+}\n+\n+git_svn_dcommit()\n+{\n+\tN=`expr $N + 1`\n+\tget_file_contents \"$@\" > git-svn-message.$N;\n+\t(cd work.git && echo \"$N\" >> file &&\n+\tgit commit -a -F ../git-svn-message.$N &&\n+\tgit svn dcommit )\n+}\n+\n+fetch_and_check()\n+{\n+\tget_file_contents \"$@\" >expected.$N\t&&\n+\techo \"GIT-SVN-ID-LINE\" >> expected.$N\t&&\n+\t(cd work.git && git svn rebase)\t\t&&\n+\t(cd work.git && git show -s HEAD) | sed -ne '/^ /,${\n+\t\ts/^ //\n+\t\ts/^git-svn-id: .*$/GIT-SVN-ID-LINE/\n+\t\tp\n+\t\t}' > actual.$N\t\t\t&&\n+\ttest_cmp expected.$N actual.$N\n+}\n+\n+\n+test_expect_success 'setup svn & git repository' '\n+\tsvn_cmd checkout \"$svnrepo\" work.svn &&\n+\t(\n+\t\tcd work.svn &&\n+\t\techo >file\n+\t\tsvn_cmd add file &&\n+\t\tsvn_cmd commit -m \"initial commit\"\n+ ) &&\n+ git svn clone \"$svnrepo\" work.git\n+'\n+\n+test_expect_success 'check empty line is added before git-svn-id' '\n+\tsvn_commit \"test message 1\" &&\n+\tfetch_and_check \"test message 1\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'no empty line before git-svn-id if ends with pseudo-header' '\n+\tsvn_commit \"test message 2\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I123456\" &&\n+\tfetch_and_check \"test message 2\" \\\n+\t\t\t\t\"\" \\\n+\t\t\t\t\"Change-Id: I123456\"\n+'\n+\n+test_expect_success 'no empty line before git-svn-id if ends with 2 pseudo-headers' '\n+\tsvn_commit \"test message 3\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I123456\" \\\n+\t\t\t\"Signed-off-by: Au Thor \" &&\n+\tfetch_and_check \"test message 3\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I123456\" \\\n+\t\t\t\"Signed-off-by: Au Thor \"\n+'\n+\n+test_expect_success 'empty line added when pseudo-header not in last section' '\n+\tsvn_commit \"test 4\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I123456\" \\\n+\t\t\t\"line without colon\" &&\n+\tfetch_and_check \"test 4\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I123456\" \\\n+\t\t\t\"line without colon\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'empty line added when pseudo-header missing space' '\n+\tsvn_commit \"test 5\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id:I123456\" &&\n+\tfetch_and_check \"test 5\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id:I123456\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'empty line added when pseudo-header missing colon' '\n+\tsvn_commit \"test 6\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id I123456\" &&\n+\tfetch_and_check \"test 6\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id I123456\" \\\n+\t\t\t\"\"\n+'\n+test_expect_success 'empty line added when message consist of pseudo-header only' '\n+\tsvn_commit \"Change-Id: I7a1b2c3\" &&\n+\tfetch_and_check \"Change-Id: I7a1b2c3\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'whitespaces removed from start of message' '\n+\tsvn_commit \" \" \\\n+\t\t \" test 8\" &&\n+\tfetch_and_check \"test 8\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'whitespaces removed from end of message' '\n+\tsvn_commit \"test commit 9 \" \\\n+\t\t \" \" \\\n+\t\t \"\t\" &&\n+\tfetch_and_check \"test commit 9\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'empty message imported as git-svn-id only' '\n+\tsvn_commit \" \" &&\n+\tfetch_and_check\n+'\n+\n+test_expect_success 'pseudo-header preserved during git svn dcommit/rebase' '\n+\tgit_svn_dcommit \"test 11\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I23445\" &&\n+\tfetch_and_check \"test 11\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Change-Id: I23445\"\n+'\n+\n+test_expect_success 'empty line added if no pseudo-header when using git svn dcommit' '\n+\tgit_svn_dcommit \"test 12\" &&\n+\tfetch_and_check \"test 12\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'suprious git-svn-id line removed by git svn dcommit' '\n+\tgit_svn_dcommit \"test 13\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"git-svn-id: file:///tmp/test@100 cb7b2de7-d0f6-461c-9b5c-d86679671c8\" &&\n+\tfetch_and_check \"test 13\" \\\n+\t\t\t\"\"\n+'\n+\n+test_expect_success 'suprious git-svn-id line removed by git svn dcommit when in middle of message' '\n+\tgit_svn_dcommit \"test 14\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"git-svn-id: file:///tmp/test@110 cb7b2de7-d0f6-461c-9b5c-d86679671c8\" \\\n+\t\t\t\"Header: test\" &&\n+\tfetch_and_check \"test 14\" \\\n+\t\t\t\"\" \\\n+\t\t\t\"Header: test\"\n+\n+'\n+test_done\n-- \n1.7.10.4\n"},{"id":"196309","messageId":"20120801214318.GA10384@dcvr.yhbt.net","threadId":"31166","inReplyTo":"1343856197-6393-1-git-send-email-robert@debian.org","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-01T21:43:18Z","receivedAt":"2012-08-01T21:43:18Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Robert Luberda wrote:\n> While importing changes from SVN by `git svn fetch' strip any\n> white spaces from beginnings and endings of SVN commit messages\n> and skip adding a new line character before `git-svn-id:'\n> line in case the commit message ends with another pseudo-header\n> (like From:, Signed-off-by: or Change-Id:, etc.).\n> \n> This patch allows one to use gerrit code review system on git-svn-managed\n> repositories. gerrit expects its `Change-Id:' header to appear in the\n> last paragraph of commit message and `git-svn-id:' following a new\n> line character was breaking this expectation.\n\nI've long wanted to change this, but it breaks compatibility if folks\nare importing from the same repo, sharing changes and one upgrades\ngit-svn.\n\nHow about making this optional and configurable at init/clone time?\n"},{"id":"196315","messageId":"5019AD3A.8030609@debian.org","threadId":"31166","inReplyTo":"20120801214318.GA10384@dcvr.yhbt.net","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2012-08-01T22:27:06Z","receivedAt":"2012-08-01T22:27:06Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"Eric Wong wrote:\n\nHi,\n\n> \n> I've long wanted to change this, but it breaks compatibility if folks\n> are importing from the same repo, sharing changes and one upgrades\n> git-svn.\n\nYes, I'm aware of this. That's why in our team at work everybody is\nforced to use the modified version of git-svn:)\n[ A bit of background: we have recently started using git svn at after\nour main repository had been migrated to svn. Previously git over\nclearcase approach was used; we established some workflow and a part of\nit was to use gerrit for code reviews, however it turned out to be\nhardly possible with git svn. On the other hand the lack of error\nhandling was pretty annoying when changes get rejected by svn pre-commit\nhook. That's why I wrote the two patches I sent today. ]\n\n> How about making this optional and configurable at init/clone time?\n\nI don't think it will be hard to make it configurable. I can try to make\nsuch a change, do you have any preferences about the option and\nconfiguration key names?\n\nThanks a lot,\nrobert\n"},{"id":"196319","messageId":"20120801230118.GA15479@dcvr.yhbt.net","threadId":"31166","inReplyTo":"5019AD3A.8030609@debian.org","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-01T23:01:18Z","receivedAt":"2012-08-01T23:01:18Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Robert Luberda wrote:\n> Eric Wong wrote:\n> > How about making this optional and configurable at init/clone time?\n> \n> I don't think it will be hard to make it configurable. I can try to make\n> such a change, do you have any preferences about the option and\n> configuration key names?\n\nNo preference off the top of my head. As long as it makes sense to\nenough people here and is consistent in style with existing options in\ngit.\n\nThanks.\n"},{"id":"197304","messageId":"50315ED1.6080803@debian.org","threadId":"31166","inReplyTo":"20120801230118.GA15479@dcvr.yhbt.net","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2012-08-19T21:46:57Z","receivedAt":"2012-08-19T21:46:57Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"Eric Wong wrote:\n\nHi,\n>> I don't think it will be hard to make it configurable. I can try to make\n>> such a change, do you have any preferences about the option and\n>> configuration key names?\n> \n> No preference off the top of my head. As long as it makes sense to\n> enough people here and is consistent in style with existing options in\n> git.\n\nI have been quite a busy recently, so it took me longer that I thought.\n\nIt was quite hard for me to think some sensible option name, and finally\nhave chosen --trim-svn-log (svn.trimsvnlog as config key name). Please\nlet me know if such name is ok for you. If not, I'll try to find a\ndifferent one (but as I wrote I'm not really good at giving names to\noptions/functions/variables, etc. :()\n\nI considered making the option a default one for new git svn clones, so\nthat existing repositories would use the older approach, but I gave up\nthe idea, and implemented the simpler solution, in which the option must\nbe given explicitly if one needs the new behavior. If making it a\ndefault for new clones would make sense for you, I can try to implement\nthis as well.\n\nFor consistency, the `--add-author-from' option was modified not to add\nan extra new line before 'From: ' line when the newly introduced option\nis in effect.\n\nI'm sending a new patch in next e-mail, could you please look at it and\nshare any comments you might have? One thing I was not sure about is the\nrequirement, introduced in the change, of having a whitespace character\nafter a colon in pseudo-header lines\n(e.g. `From:somebody ' won't be considered as a\npseudo-header) - is this consistent with a way git handles\nheaders/pseudo-headers?\n\nBest regards,\nrobert\n"},{"id":"197305","messageId":"1345413170-2519-1-git-send-email-robert@debian.org","threadId":"31166","inReplyTo":"50315ED1.6080803@debian.org","subject":"[PATCH/RFC] git svn: optionally trim imported log messages","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2012-08-19T21:52:50Z","receivedAt":"2012-08-19T21:52:50Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"Introduce a `--trim-svn-log' option and svn.trimsvnlog boolean\nconfiguration key for git svn commands.\n\nWhen enabled while retrieving commits from svn, git svn will strip any\nwhite spaces from beginnings and endings of SVN commit messages and will\nskip adding an extra new line character before `git-svn-id:' line in case\nthe commit message already ends with a pseudo-headers section (a section\nstarting with an empty line followed by one or more pseudo-headers like\n`From: ', `Signed-off-by: ', or `Change-Id: ').\n\nAdditionally, while creating new commits in svn when the new option is\nenabled and `--add-author-from' is in effect, git svn will not add\na new line character before the `From: ' pseudo-header if the commit\nmessage already ends with a pseudo-headers section.\n\nThe new option allows one to use gerrit code review system on\ngit-svn-managed repositories. gerrit expects its `Change-Id:' header\nto appear in the last paragraph of commit message and the standard\nbehaviour of preceding `git-svn-id:' line with a new line character\nwas breaking this expectation.\n---\n Documentation/git-svn.txt | 16 ++\n git-svn.perl | 8 +-\n perl/Git/SVN.pm | 19 +-\n t/t9165-git-svn-import-messages.sh | 387 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 427 insertions(+), 3 deletions(-)\n create mode 100755 t/t9165-git-svn-import-messages.sh\n\ndiff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\nindex cfe8d2b..79c21ee 100644\n--- a/Documentation/git-svn.txt\n+++ b/Documentation/git-svn.txt\n@@ -603,6 +603,22 @@ creating the branch or tag.\n \tgit commit's author string. If you use this, then `--use-log-author`\n \twill retrieve a valid author string for all commits.\n \n+--trim-svn-log::\n+\tWhen retrieving svn commits into git (as part of 'fetch', 'rebase',\n+\tor 'dcommit') trim the whitespaces from beginnings and endings\n+\tof the svn log messages and avoid preceding `git-svn-id:` line with\n+\ta new line character in case the log message already ends with a\n+\tpseudo-headers section (i.e. section starting with an empty line\n+\tfollowed by one or more lines like `Signed-off-by: `, `From: `,\n+\tor `Change-Id: `).\n++\n+When committing to svn from git (as part of 'commit-diff', 'set-tree'\n+or 'dcommit') and '--add-author-from' is in effect, a new line character\n+is not added before the `From: ` line if the log message ends with\n+a pseudo-headers section.\n++\n+[verse]\n+config key: svn.trimsvnlog\n \n ADVANCED OPTIONS\n ----------------\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 828b8f0..3740835 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -133,6 +133,7 @@ my %fc_opts = ( 'follow-parent|follow!' => \\$Git::SVN::_follow_parent,\n \t\t \\$Git::SVN::_repack_flags,\n \t\t'use-log-author' => \\$Git::SVN::_use_log_author,\n \t\t'add-author-from' => \\$Git::SVN::_add_author_from,\n+\t\t'trim-svn-log' => \\$Git::SVN::_trim_svn_log,\n \t\t'localtime' => \\$Git::SVN::_localtime,\n \t\t%remote_opts );\n \n@@ -500,6 +501,8 @@ sub cmd_clone {\n \tcmd_init($url, $path);\n \tcommand_oneline('config', 'svn.authorsfile', $authors_absolute)\n \t if $_authors;\n+\tcommand_oneline('config', 'svn.trimsvnlog', 'true')\n+\t if $Git::SVN::_trim_svn_log;\n \tGit::SVN::fetch_all($Git::SVN::default_repo_id);\n }\n \n@@ -1782,7 +1785,10 @@ sub get_commit_entry {\n \t\t$msgbuf =~ s/\\s+$//s;\n \t\tif ($Git::SVN::_add_author_from && defined($author)\n \t\t && !$saw_from) {\n-\t\t\t$msgbuf .= \"\\n\\nFrom: $author\";\n+\t\t\t$msgbuf .= \"\\n\";\n+\t\t\t$msgbuf .= \"\\n\"\n+\t\t\t\tunless Git::SVN::avoid_extra_new_line($msgbuf);\n+\t\t\t$msgbuf .= \"From: $author\";\n \t\t}\n \t\tprint $log_fh $msgbuf or croak $!;\n \t\tcommand_close_pipe($msg_fh, $ctx);\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 8478d0c..9604667 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -5,7 +5,7 @@ use Fcntl qw/:DEFAULT :seek/;\n use constant rev_map_fmt => 'NH40';\n use vars qw/$_no_metadata\n $_repack $_repack_flags $_use_svm_props $_head\n- $_use_svnsync_props $no_reuse_existing\n+ $_use_svnsync_props $no_reuse_existing $_trim_svn_log\n \t $_use_log_author $_add_author_from $_localtime/;\n use Carp qw/croak/;\n use File::Path qw/mkpath/;\n@@ -978,6 +978,17 @@ sub gc {\n \tcommand_noisy('gc', '--auto');\n };\n \n+# An utility function which returns true if both trimming svn log messages\n+# is enabled, and passed log entry ends with a pseudo-headers section (i.e.\n+# section starting with empty line followed by one or more pseudo-headers\n+# like 'From: ' or 'Change-Id: ' etc.)\n+sub avoid_extra_new_line {\n+\treturn 0 unless $_trim_svn_log;\n+\n+\tmy $log_entry = shift;\n+\treturn ($log_entry =~ m/\\n\\n([\\w-]+:\\s.*\\n)+$/);\n+}\n+\n sub do_git_commit {\n \tmy ($self, $log_entry) = @_;\n \tmy $lr = $self->last_rev;\n@@ -1015,7 +1026,9 @@ sub do_git_commit {\n \tprint $msg_fh $log_entry->{log} or croak $!;\n \trestore_commit_header_env($old_env);\n \tunless ($self->no_metadata) {\n-\t\tprint $msg_fh \"\\ngit-svn-id: $log_entry->{metadata}\\n\"\n+\t\tprint $msg_fh \"\\n\"\n+\t\t\tunless avoid_extra_new_line($log_entry->{log});\n+\t\tprint $msg_fh \"git-svn-id: $log_entry->{metadata}\\n\"\n \t\t or croak $!;\n \t}\n \t$msg_fh->flush == 0 or croak $!;\n@@ -1821,6 +1834,8 @@ sub make_log_entry {\n \tclose $un or croak $!;\n \n \t$log_entry{date} = parse_svn_date($log_entry{date});\n+\t$log_entry{log} =~ s/^\\s*// if $_trim_svn_log;\n+\t$log_entry{log} =~ s/\\s*$// if $_trim_svn_log;\n \t$log_entry{log} .= \"\\n\";\n \tmy $author = $log_entry{author} = check_author($log_entry{author});\n \tmy ($name, $email) = defined $::users{$author} ? @{$::users{$author}}\ndiff --git a/t/t9165-git-svn-import-messages.sh b/t/t9165-git-svn-import-messages.sh\nnew file mode 100755\nindex 0000000..1ad4485\n--- /dev/null\n+++ b/t/t9165-git-svn-import-messages.sh\n@@ -0,0 +1,387 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2012 Robert Luberda\n+#\n+\n+test_description='checks behaviour of --trim-svn-log option of git svn'\n+. ./lib-git-svn.sh\n+\n+# The test cases in this file check if the --trim-svn-log option\n+# of git svn works as expected.\n+#\n+# Basically the test cases use two git repositories:\n+# * work-TRIM.git, created by git svn clone with the option enabled,\n+# * work-NOTR.git, created with the option disabled.\n+#\n+# Each test case (except for the first two) does the following:\n+# 1. commit a change to svn with either svn commit, or git svn dcommit\n+# (what is important is the commit log message, not the changed file)\n+# 2. git svn rebase the work-NOTR.git repository and check its most recent\n+# log message against some expected output\n+# 3. git svn rebase the work-TRIM.git repository and similarly check the\n+# latest log message\n+#\n+# The following two prerequisites are defined mostly for debugging purposes\n+# to make it possible to skip test cases or parts of the test cases that\n+# operate on one of the two git repositories. For example to ignore checking\n+# of log messages imported when --trim-svn-log is enabled, one needs to comment\n+# out the PRQ_TRIM pre-requisite (this makes it possible to run the test with\n+# a version of git svn that does not support the option yet). Similarly\n+# commenting out the PRQ_NOTR pre-requisite will check only the effects of the\n+# svn log trimming option.\n+# Please note that a whole test case must be marked as requiring one of\n+# those pre-requisites if and only if it uses `git svn dcommit' for committing\n+# changes to svn.\n+test_set_prereq PRQ_TRIM\n+test_set_prereq PRQ_NOTR\n+\n+N=0\n+NL=\"\"\t# for better readability only, e.g.:\n+\t# \"$NL\" \" \" \"$NL\" is cleaner than \"\" \" \" \"\"\n+\n+next_N()\n+{\n+\tN=$((N + 1)) &&\n+\techo \"N is $N\"\n+}\n+\n+# An utility function used for writing log messages to files\n+get_file_contents()\n+{\n+\tfor line in \"$@\"; do\n+\t\techo \"$line\"\n+\tdone\n+}\n+\n+# Commit to svn using 'svn commit'.\n+# Usage:\n+# svn_commit commit_log_line ...\n+# For example:\n+# svn_commit \"message line 1\" \"message line 2\"\n+# will create a svn commit which log message is\n+# \"message line 1\\nmessage line 2\\n\\n\"\n+# Please note the two ending new line characters - the first one\n+# is added by us and the second one is added by svn (which seems\n+# to always adds a new line character on its own).\n+svn_commit()\n+{\n+\tget_file_contents \"$@\" > t${N}-svn-message &&\n+\t(\n+\t\tcd work.svn && svn_cmd up &&\n+\t\techo \"$N\" >> file &&\n+\t\tsvn_cmd commit -F ../t${N}-svn-message file\n+\t)\n+}\n+\n+# Commit to svn using 'git svn dcommit' in either work-TRIM.git or\n+# work-NOTR.git repository.\n+# Usage:\n+# git_svn_dcm [ git_svn_dcommit_options ] TRIM | NOTR commit_log_line ...\n+# Examples:\n+# git_svn_dcm TRIM \"message line 1\" \"message line 2\"\n+# git_svn_dcm NOTR \"message line 1\" \"message line 2\"\n+# git_svn_dcm --add-author-from TRIM \"message line 1\" \"message line 2\"\n+# Notes:\n+# * On the contrary to the above svn_commit() function, created\n+# svn log message ends with only one new line character, e.g. the command\n+# from the first example will create a commit in svn with a log message:\n+# \"message line 1\\nmessage line 2\\n\"\n+# * Test using this function should declare an appropriate pre-requisite.\n+git_svn_dcm()\n+{\n+\topts=\"\" &&\n+\twhile test \"$1\" != \"TRIM\" && test \"$1\" != \"NOTR\"\n+\tdo\n+\t\topts=\"$opts $1\" && shift;\n+\tdone &&\n+\ttype=\"$1\" && shift &&\n+\tget_file_contents \"$@\" > t${N}-git-svn-message.$type &&\n+\t(\n+\t\tcd work-${type}.git && git svn rebase &&\n+\t\techo \"$N\" >> file &&\n+\t\tgit commit -a -F ../t${N}-git-svn-message.$type &&\n+\t\tgit svn dcommit $opts\n+\t)\n+}\n+\n+\n+# Rebases either work-TRIM.git or work-NOTR.git repository using `git svn\n+# rebase' and checks the latest git log message against the given expected\n+# message log lines.\n+# Usage:\n+# fetch_check TRIM | NOTR expected_log_line ...\n+# Examples:\n+# fetch_check TRIM \"expected message line 1\" \"expected message line 2\"\n+# fetch_check NOTR \"expected message line 1\"\n+# Notes:\n+# * A `git-svn-id: ' line is assumed to end each log message. No such line\n+# should be passed as argument of this function.\n+# * The function does nothing but returning successfully if a necessary\n+# pre-requisite is not set.\n+fetch_check()\n+{\n+\ttype=\"$1\" && shift &&\n+\t{ test_have_prereq \"PRQ_$type\" || return 0; } &&\n+\texpected_file=\"t${N}-expected.${type}\" \t &&\n+\t{\n+\t\tget_file_contents \"$@\" > \"$expected_file\" &&\n+\t\techo \"GIT-SVN-ID-LINE\" >> \"$expected_file\"\n+\t} &&\n+\tactual_file=\"t${N}-actual.${type}\" &&\n+\t{\n+\t\t(cd work-${type}.git && git svn rebase) &&\n+\t\t(cd work-${type}.git && git show -s HEAD) | sed -ne '/^ /,${\n+\t\t\ts/^ //\n+\t\t\ts/^git-svn-id: .*$/GIT-SVN-ID-LINE/\n+\t\t\tp\n+\t\t\t}' > \"$actual_file\"\n+\t} &&\n+\ttest_cmp \"$expected_file\" \"$actual_file\"\n+}\n+\n+\n+test_expect_success 'setup svn & git repositories' '\n+\tsvn_cmd checkout \"$svnrepo\" work.svn &&\n+\t(\n+\t\tcd work.svn &&\n+\t\techo >file &&\n+\t\tsvn_cmd add file &&\n+\t\tsvn_cmd commit -m \"Initial commit\"\n+\t) &&\n+\tif test_have_prereq PRQ_NOTR; then\n+\t\tgit svn clone \"$svnrepo\" work-NOTR.git\n+\tfi &&\n+\tif test_have_prereq PRQ_TRIM; then\n+\t\tgit svn clone --trim-svn-log \\\n+\t\t\t\t\"$svnrepo\" work-TRIM.git\n+\tfi\n+'\n+\n+test_expect_success 'configuration key check' '\n+\tkey=svn.trimsvnlog &&\n+\tif test_have_prereq PRQ_NOTR; then\n+\t(\n+\t\tcd work-NOTR.git &&\n+\t\ttest_must_fail git config --local --get \"$key\"\n+\t)\n+\tfi &&\n+\tif test_have_prereq PRQ_TRIM; then\n+\t(\n+\t\tcd work-TRIM.git &&\n+\t\techo \"true\" > expected.setting &&\n+\t\tgit config --local --get \"$key\" > actual.setting &&\n+\t\ttest_cmp expected.setting actual.setting\n+\t)\n+\tfi\n+'\n+\n+test_expect_success 'no pseudo-header in message' '\n+\tnext_N &&\n+\tsvn_commit \"test message $N\" &&\n+\tfetch_check NOTR \"test message $N\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test message $N\" \"$NL\"\n+'\n+\n+test_expect_success 'one pseudo-header at the end of message' '\n+\tnext_N &&\n+\tsvn_commit \"test message $N\" \"$NL\" \"Change-Id: I123456\" &&\n+\tfetch_check NOTR \"test message $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test message $N\" \"$NL\" \"Change-Id: I123456\"\n+'\n+\n+test_expect_success 'two pseudo-headers at the end of message' '\n+\tnext_N &&\n+\tsvn_commit \"test message $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"Signed-off-by: Au Thor \" &&\n+\tfetch_check NOTR \"test message $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"Signed-off-by: Au Thor \" \\\n+\t\t\t\"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test message $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"Signed-off-by: Au Thor \"\n+'\n+\n+test_expect_success 'pseudo-header with digits in header name' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"Key12X3: value\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Key12X3: value\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Key12X3: value\"\n+'\n+\n+test_expect_success 'pseudo-header with tab as a separator' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"Key:\tvalue\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Key:\tvalue\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Key:\tvalue\"\n+'\n+\n+test_expect_success 'pseudo-header not in the last section' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"line without colon\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"line without colon\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Change-Id: I123456\" \\\n+\t\t\t\"line without colon\" \"$NL\"\n+'\n+\n+test_expect_success 'pseudo-header not separated by empty line' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"not a pseudo header\" \\\n+\t\t\t\"Pseudo-Header: value\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"not a pseudo header\" \\\n+\t\t\t\"Pseudo-Header: value\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"not a pseudo header\" \\\n+\t\t\t\"Pseudo-Header: value\" \"$NL\"\n+'\n+\n+test_expect_success 'not considered as pseudo-header: no space after colon' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"Change-Id:I123456\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Change-Id:I123456\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Change-Id:I123456\" \"$NL\"\n+'\n+\n+test_expect_success 'not considered as pseudo-header: missing colon' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"Change-Id I123456\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Change-Id I123456\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Change-Id I123456\" \"$NL\"\n+'\n+\n+test_expect_success 'not considered as pseudo-header: space in header name' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \"Change Id: I1234567\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Change Id: I1234567\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Change Id: I1234567\" \"$NL\"\n+'\n+\n+test_expect_success 'not considered as pseudo-header: space at front' '\n+\tnext_N &&\n+\tsvn_commit \"test $N\" \"$NL\" \" Change Id: Iaaaab\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \" Change Id: Iaaaab\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \" Change Id: Iaaaab\" \"$NL\"\n+'\n+\n+test_expect_success 'new line added if pseudo-header only message' '\n+\tnext_N &&\n+\tsvn_commit \"Change-Id: I7a1b2c3\" &&\n+\tfetch_check NOTR \"Change-Id: I7a1b2c3\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"Change-Id: I7a1b2c3\" \"$NL\"\n+'\n+\n+test_expect_success 'extra whitespaces dropped from beginning of message' '\n+\tnext_N &&\n+\tsvn_commit \" \" \" test $N\" &&\n+\tfetch_check NOTR \" test $N\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\"\n+'\n+\n+test_expect_success 'extra whitespaces dropped from the end of message' '\n+\tnext_N &&\n+\tws1=\" \" && ws2=\"\t \" \\\n+\tsvn_commit \"test $N \" \"$ws1\" \"$ws2\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"$NL\" \"$NL\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\"\n+'\n+\n+test_expect_success 'whitespace-only message imported as git-svn-id only' '\n+\tnext_N &&\n+\tsvn_commit \" \" &&\n+\tfetch_check NOTR &&\n+\tfetch_check TRIM\n+'\n+\n+test_expect_success PRQ_NOTR 'dcommit: no-trim, no-pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm NOTR \"test $N\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\"\n+'\n+\n+test_expect_success PRQ_TRIM 'dcommit: trim, no-pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm TRIM \"test $N\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\"\n+'\n+\n+test_expect_success PRQ_NOTR 'dcommit: no-trim, pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm NOTR \"test $N\" \"$NL\" \"Change-Id: I2344a\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Change-Id: I2344a\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Change-Id: I2344a\"\n+'\n+\n+test_expect_success PRQ_TRIM 'dcommit: trim, pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm TRIM \"test $N\" \"$NL\" \"Change-Id: I2344b\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Change-Id: I2344b\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Change-Id: I2344b\"\n+'\n+\n+\n+test_expect_success PRQ_NOTR 'dcommit --add-a-f: no-trim, no-pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm --add-author-from NOTR \"test $N\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \\\n+\t\t\t\"From: A U Thor \" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \\\n+\t\t\t\"From: A U Thor \"\n+'\n+\n+test_expect_success PRQ_TRIM 'dcommit --add-a-f: trim, no-pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm --add-author-from TRIM \"test $N\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \\\n+\t\t\t\"From: A U Thor \" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \\\n+\t\t\t\"From: A U Thor \"\n+'\n+\n+test_expect_success PRQ_NOTR 'dcommit --add-a-f: no-trim, pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm --add-author-from NOTR \\\n+\t\t\t\"test $N\" \"$NL\" \\\n+\t\t\t\"Change-Id: Ixxx1\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \\\n+\t\t\t\"Change-Id: Ixxx1\" \"$NL\" \\\n+\t\t\t\"From: A U Thor \" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \\\n+\t\t\t\"Change-Id: Ixxx1\" \"$NL\" \\\n+\t\t\t\"From: A U Thor \"\n+'\n+\n+test_expect_success PRQ_TRIM 'dcommit --add-a-f: trim, pseudo-header' '\n+\tnext_N &&\n+\tgit_svn_dcm --add-author-from TRIM \\\n+\t\t\t\"test $N\" \"$NL\" \\\n+\t\t\t\"Change-Id: Ixxx1\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \\\n+\t\t\t\"Change-Id: Ixxx1\" \\\n+\t\t\t\"From: A U Thor \" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \\\n+\t\t\t\"Change-Id: Ixxx1\" \\\n+\t\t\t\"From: A U Thor \"\n+'\n+\n+test_expect_success PRQ_NOTR 'dcommit: remove suprious git-svn-id' '\n+\tnext_N &&\n+\tid=cb7b2de7-d0f6-461c-9b5c-d86679671c8 &&\n+\tgit_svn_dcm NOTR \"test $N\" \"$NL\" \\\n+\t\t\t\"git-svn-id: file:///tmp/test@100 $id\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\"\n+'\n+\n+test_expect_success PRQ_NOTR 'dcommit: remove git-svn-id in middle of message' '\n+\tnext_N &&\n+\tid=cb7b2de7-d0f6-461c-9b5c-d86679671c8 &&\n+\tgit_svn_dcm NOTR \"test $N\" \"$NL\" \\\n+\t\t\t\"git-svn-id: file:///tmp/test@110 $id\" \\\n+\t\t\t\"Header: test\" &&\n+\tfetch_check NOTR \"test $N\" \"$NL\" \"Header: test\" \"$NL\" &&\n+\tfetch_check TRIM \"test $N\" \"$NL\" \"Header: test\"\n+\n+'\n+test_done\n-- \n1.7.10.4\n"},{"id":"197308","messageId":"7vsjbio2ky.fsf@alter.siamese.dyndns.org","threadId":"31166","inReplyTo":"1345413170-2519-1-git-send-email-robert@debian.org","subject":"Re: [PATCH/RFC] git svn: optionally trim imported log messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-19T23:59:41Z","receivedAt":"2012-08-19T23:59:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Luberda writes:\n\n> Introduce a `--trim-svn-log' option and svn.trimsvnlog boolean\n> configuration key for git svn commands.\n>\n> When enabled while retrieving commits from svn, git svn will strip any\n> white spaces from beginnings and endings of SVN commit messages and will\n> skip adding an extra new line character before `git-svn-id:' line in case\n> the commit message already ends with a pseudo-headers section (a section\n> starting with an empty line followed by one or more pseudo-headers like\n> `From: ', `Signed-off-by: ', or `Change-Id: ').\n>\n> Additionally, while creating new commits in svn when the new option is\n> enabled and `--add-author-from' is in effect, git svn will not add\n> a new line character before the `From: ' pseudo-header if the commit\n> message already ends with a pseudo-headers section.\n>\n> The new option allows one to use gerrit code review system on\n> git-svn-managed repositories. gerrit expects its `Change-Id:' header\n> to appear in the last paragraph of commit message and the standard\n> behaviour of preceding `git-svn-id:' line with a new line character\n> was breaking this expectation.\n> ---\n\nSign-off?\n\n> Documentation/git-svn.txt | 16 ++\n> git-svn.perl | 8 +-\n> perl/Git/SVN.pm | 19 +-\n> t/t9165-git-svn-import-messages.sh | 387 ++++++++++++++++++++++++++++++++++++\n> 4 files changed, 427 insertions(+), 3 deletions(-)\n> create mode 100755 t/t9165-git-svn-import-messages.sh\n>\n> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\n> index cfe8d2b..79c21ee 100644\n> --- a/Documentation/git-svn.txt\n> +++ b/Documentation/git-svn.txt\n> @@ -603,6 +603,22 @@ creating the branch or tag.\n> \tgit commit's author string. If you use this, then `--use-log-author`\n> \twill retrieve a valid author string for all commits.\n> \n> +--trim-svn-log::\n> +\tWhen retrieving svn commits into git (as part of 'fetch', 'rebase',\n> +\tor 'dcommit') trim the whitespaces from beginnings and endings\n> +\tof the svn log messages and avoid preceding `git-svn-id:` line with\n> +\ta new line character in case the log message already ends with a\n> +\tpseudo-headers section (i.e. section starting with an empty line\n> +\tfollowed by one or more lines like `Signed-off-by: `, `From: `,\n> +\tor `Change-Id: `).\n> ++\n> +When committing to svn from git (as part of 'commit-diff', 'set-tree'\n> +or 'dcommit') and '--add-author-from' is in effect, a new line character\n> +is not added before the `From: ` line if the log message ends with\n> +a pseudo-headers section.\n\nI think it would be saner to call them \"trailers\" to avoid\nconfusion.\n\nI've seen S-o-b, Cc and Change-Id there, but does anybody actually\nput \"From: \" there?\n\nThere needs an explanation to the reader why this is an optional\nfeature.\n\n> --- /dev/null\n> +++ b/t/t9165-git-svn-import-messages.sh\n> @@ -0,0 +1,387 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2012 Robert Luberda\n> +#\n> +\n> +test_description='checks behaviour of --trim-svn-log option of git svn'\n> +. ./lib-git-svn.sh\n> +\n> +# The test cases in this file check if the --trim-svn-log option\n> +# of git svn works as expected.\n> +#\n> +# Basically the test cases use two git repositories:\n> +# * work-TRIM.git, created by git svn clone with the option enabled,\n> +# * work-NOTR.git, created with the option disabled.\n> +#\n> +# Each test case (except for the first two) does the following:\n> +# 1. commit a change to svn with either svn commit, or git svn dcommit\n> +# (what is important is the commit log message, not the changed file)\n> +# 2. git svn rebase the work-NOTR.git repository and check its most recent\n> +# log message against some expected output\n> +# 3. git svn rebase the work-TRIM.git repository and similarly check the\n> +# latest log message\n> +#\n> +# The following two prerequisites are defined mostly for debugging purposes\n> +# to make it possible to skip test cases or parts of the test cases that\n> +# operate on one of the two git repositories. For example to ignore checking\n\nThe prerequisite mechanism is to allow some tests in an environment\nwhere they cannot be run (e.g. no SVN available on the platform);\nany code that uses set_prereq unconditionally looks extremely\nstrange. It is OK while the patch is in RFC stage under active\ndebugging, but they would want to go away before the polishing is\ndone.\n\n> +# of log messages imported when --trim-svn-log is enabled, one needs to comment\n> +# out the PRQ_TRIM pre-requisite (this makes it possible to run the test with\n> +# a version of git svn that does not support the option yet). Similarly\n> +# commenting out the PRQ_NOTR pre-requisite will check only the effects of the\n> +# svn log trimming option.\n> +# Please note that a whole test case must be marked as requiring one of\n> +# those pre-requisites if and only if it uses `git svn dcommit' for committing\n> +# changes to svn.\n> +test_set_prereq PRQ_TRIM\n> +test_set_prereq PRQ_NOTR\n\n> +N=0\n> +NL=\"\"\t# for better readability only, e.g.:\n> +\t# \"$NL\" \" \" \"$NL\" is cleaner than \"\" \" \" \"\"\n\nWhat does En-El stand for? We often see (but not in Git where we\nprefer LF for that purpose)\n\n\tNL='\n ' ;# newline\n\nand using $NL to mean \"empty\" may be misleading to the readers.\n\n> +next_N()\n> +{\n> +\tN=$((N + 1)) &&\n> +\techo \"N is $N\"\n> +}\n\nMost shells tolerate writing a bare N inside arith substitution, but\nfor better portability, please spell this as\n\n\tnext_N () {\n\t\tN=$(($N + 1)) &&\n ...\n\t}\n\n(the above also has two style fixes).\n\n> +\n> +# An utility function used for writing log messages to files\n> +get_file_contents()\n> +{\n> +\tfor line in \"$@\"; do\n> +\t\techo \"$line\"\n> +\tdone\n> +}\n\n(Style)\n\tget_file_contents () {\n for line\n do\n ...\n done\n\t}\n"},{"id":"197558","messageId":"20120821214538.GA4879@dcvr.yhbt.net","threadId":"31166","inReplyTo":"50315ED1.6080803@debian.org","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-21T21:45:38Z","receivedAt":"2012-08-21T21:45:38Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Robert Luberda wrote:\n> I have been quite a busy recently, so it took me longer that I thought.\n\nNo worries, thanks for following up.\n\n> It was quite hard for me to think some sensible option name, and finally\n> have chosen --trim-svn-log (svn.trimsvnlog as config key name). Please\n> let me know if such name is ok for you. If not, I'll try to find a\n> different one (but as I wrote I'm not really good at giving names to\n> options/functions/variables, etc. :()\n\nI think having \"svn\" in \"svn.trimsvnlog\" twice is redundant and not ideal.\nPerhaps just --trim-log and svn.trimlog?\n\n> I considered making the option a default one for new git svn clones, so\n> that existing repositories would use the older approach, but I gave up\n> the idea, and implemented the simpler solution, in which the option must\n> be given explicitly if one needs the new behavior. If making it a\n> default for new clones would make sense for you, I can try to implement\n> this as well.\n\nDefault behavior should not change, especially not without warning.\n\n> For consistency, the `--add-author-from' option was modified not to add\n> an extra new line before 'From: ' line when the newly introduced option\n> is in effect.\n\nOK.\n\n> I'm sending a new patch in next e-mail, could you please look at it and\n> share any comments you might have? One thing I was not sure about is the\n> requirement, introduced in the change, of having a whitespace character\n> after a colon in pseudo-header lines\n> (e.g. `From:somebody ' won't be considered as a\n> pseudo-header) - is this consistent with a way git handles\n> headers/pseudo-headers?\n\nBoth git-send-email and git-am check for space after these headers,\nso git-svn should, too\n"},{"id":"197566","messageId":"7vsjbfc1q7.fsf@alter.siamese.dyndns.org","threadId":"31166","inReplyTo":"20120821214538.GA4879@dcvr.yhbt.net","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T22:35:44Z","receivedAt":"2012-08-21T22:35:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Robert Luberda wrote:\n>> I have been quite a busy recently, so it took me longer that I thought.\n>\n> No worries, thanks for following up.\n>\n>> It was quite hard for me to think some sensible option name, and finally\n>> have chosen --trim-svn-log (svn.trimsvnlog as config key name). Please\n>> let me know if such name is ok for you. If not, I'll try to find a\n>> different one (but as I wrote I'm not really good at giving names to\n>> options/functions/variables, etc. :()\n>\n> I think having \"svn\" in \"svn.trimsvnlog\" twice is redundant and not ideal.\n> Perhaps just --trim-log and svn.trimlog?\n\nDo we ever want to trim \"our\" log when relaying the Git commits back\nto subversion? Having \"svn\" in \"trimsvnlog\" makes it clear that the\nlogs from subversion side is getting trimmed.\n\n> Default behavior should not change, especially not without warning.\n\nMakes sense.\n"},{"id":"197802","messageId":"5038027B.4090208@debian.org","threadId":"31166","inReplyTo":"7vsjbio2ky.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] git svn: optionally trim imported log messages","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2012-08-24T22:38:51Z","receivedAt":"2012-08-24T22:38:51Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"Junio C Hamano writes:\n\nHi,\n\nJunio, for some reason I don't get mails from you, I've just discovered\nyour e-mails on gmane news list. Anyway many thanks for your comments,\nI'll fix them and send updated patch next week.\n\n\n>> +When committing to svn from git (as part of 'commit-diff', 'set-tree'\n>> +or 'dcommit') and '--add-author-from' is in effect, a new line character\n>> +is not added before the `From: ` line if the log message ends with\n>> +a pseudo-headers section.\n> \n> I think it would be saner to call them \"trailers\" to avoid\n> confusion.\n\nThanks, I haven't got any idea how to call them, especially because\nexisting git documentation refers to them just by using the word `line',\ne.g.:\n\n git-am.txt: Add a `Signed-off-by:` line to the commit message,\n git-cherry-pick.txt: Add Signed-off-by line at the end of the\n\n(The \"trailer\" keyword appears once in SubmittingPatches and - in a bit\ndifferent meaning in technical/pack-format.txt)\n\n> \n> I've seen S-o-b, Cc and Change-Id there, but does anybody actually\n> put \"From: \" there?\n\nYes, `git svn dcommit --add-author-from' adds such a trailer to the svn\nlog message, and then resets or rebases the git index against svn, so\nthat the message with the trailer appears in git.\n\n> \n> There needs an explanation to the reader why this is an optional\n> feature.\n\nOK, I'll add some explanation. Basically it is optional, per Eric\nrequest, for backward compatibility to make it possible to work on a\ncentralized clone of svn repository by people using both old and new\nversions of git svn.\n\n> \n> The prerequisite mechanism is to allow some tests in an environment\n> where they cannot be run (e.g. no SVN available on the platform);\n> any code that uses set_prereq unconditionally looks extremely\n> strange. It is OK while the patch is in RFC stage under active\n> debugging, but they would want to go away before the polishing is\n> done.\n\nOK, I used it merely for checking that the tests work on older version\nof git svn, and I didn't break the backward compatibility by accident.\nWill be dropped from next version of the patch.\n\n> \n> What does En-El stand for? We often see (but not in Git where we\n> prefer LF for that purpose)\n> \n> \tNL='\n> ' ;# newline\n> \n> and using $NL to mean \"empty\" may be misleading to the readers.\n\nNL means newline. The new line characters implicitly added after each\ncommit message line, that's why the value is empty. But, yes, this can\nbe misleading. I'd prefer to keep it short, so would EL (i.e.\n`empty-line') be an acceptable name?\n\n>> +\tN=$((N + 1)) &&\n> \n\nSorry, it was a typo, I meant to use $(($N + 1)). Thanks for catching this.\n\n> \n> \tnext_N () {\n> \t\tN=$(($N + 1)) &&\n> ...\n> \t}\n> \n> (the above also has two style fixes).\n\nJust to be sure: shall the `...' line start a new level of indentation\nor is it a typo? (I guess that the two style fixes are just after the\nfunction name.)\n\n\nThanks a lot,\nrobert\n"},{"id":"197801","messageId":"50380AEA.6020000@debian.org","threadId":"31166","inReplyTo":"7vsjbfc1q7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Robert Luberda","fromEmail":"robert@debian.org","sentAt":"2012-08-24T23:14:50Z","receivedAt":"2012-08-24T23:14:50Z","isPatch":true,"sender":{"key":"robert@debian.org","avatar":"https://avatars.githubusercontent.com/u/18017362?v=4"},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n>>\n>> I think having \"svn\" in \"svn.trimsvnlog\" twice is redundant and not ideal.\n>> Perhaps just --trim-log and svn.trimlog?\n> \n> Do we ever want to trim \"our\" log when relaying the Git commits back\n> to subversion? Having \"svn\" in \"trimsvnlog\" makes it clear that the\n> logs from subversion side is getting trimmed.\n\n`git commit' already trims the messages (except for removing the leading\nwhitespaces from the first non-whitespace-only line) and git svn doesn't\nchange that.\n\nThe new option affects the way the messages are imported from svn to\ngit, with one exception when the --add-author-from option of dcommit is\nused (in which case it may skip adding an extra new line character\nbefore the `From: ' line). For that reason --trim-svn-log might be a\nbetter name.\n\nRegards,\nrobert\n"},{"id":"197803","messageId":"7vk3wnyi5u.fsf@alter.siamese.dyndns.org","threadId":"31166","inReplyTo":"5038027B.4090208@debian.org","subject":"Re: [PATCH/RFC] git svn: optionally trim imported log messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-24T23:38:53Z","receivedAt":"2012-08-24T23:38:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Luberda writes:\n\n>> I think it would be saner to call them \"trailers\" to avoid\n>> confusion.\n>\n> Thanks, I haven't got any idea how to call them, especially because\n> existing git documentation refers to them just by using the word `line',\n> e.g.:\n>\n> git-am.txt: Add a `Signed-off-by:` line to the commit message,\n> git-cherry-pick.txt: Add Signed-off-by line at the end of the\n\nThen \"line\" is fine; they never come before the body, and are\ncertainly not headers.\n\n>> There needs an explanation to the reader why this is an optional\n>> feature.\n>\n> OK, I'll add some explanation. Basically it is optional, per Eric\n> request, for backward compatibility to make it possible to work on a\n> centralized clone of svn repository by people using both old and new\n> versions of git svn.\n\nThat matches my recollection. I didn't ask you to explain it to me,\nby the way, as I've skimmed the discussion during the review.\n\nI wanted the resulting history and the documentation to explain that\nto git-svn users.\n\n> NL means newline. The new line characters implicitly added after each\n> commit message line, that's why the value is empty. But, yes, this can\n> be misleading. I'd prefer to keep it short, so would EL (i.e.\n> `empty-line') be an acceptable name?\n\nI'd rather call it \"$EMPTY\"; $NL is already obscure, nobody uses $EL.\n\n>> \tnext_N () {\n>> \t\tN=$(($N + 1)) &&\n>> ...\n>> \t}\n>> \n>> (the above also has two style fixes).\n>\n> Just to be sure: shall the `...' line start a new level of indentation\n> or is it a typo?\n\nIt was meant to align with \"N=\", but perhaps HT and quoting\ninteracted badly or something.\n"},{"id":"197853","messageId":"20120826003658.GB32346@dcvr.yhbt.net","threadId":"31166","inReplyTo":"50380AEA.6020000@debian.org","subject":"Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-26T00:36:58Z","receivedAt":"2012-08-26T00:36:58Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Robert Luberda wrote:\n> Junio C Hamano wrote:\n> > Eric Wong writes:\n> >>\n> >> I think having \"svn\" in \"svn.trimsvnlog\" twice is redundant and not ideal.\n> >> Perhaps just --trim-log and svn.trimlog?\n> > \n> > Do we ever want to trim \"our\" log when relaying the Git commits back\n> > to subversion? Having \"svn\" in \"trimsvnlog\" makes it clear that the\n> > logs from subversion side is getting trimmed.\n> \n> `git commit' already trims the messages (except for removing the leading\n> whitespaces from the first non-whitespace-only line) and git svn doesn't\n> change that.\n> \n> The new option affects the way the messages are imported from svn to\n> git, with one exception when the --add-author-from option of dcommit is\n> used (in which case it may skip adding an extra new line character\n> before the `From: ' line). For that reason --trim-svn-log might be a\n> better name.\n\nOK. I now agree --trim-svn-log is a better name :>\n"}]}