{"thread":{"id":"50991","subject":"[PATCH 0/5] Fix and extend encoding handling in fast export/import","startedAt":"2019-04-25T15:51:29Z","lastAt":"2019-04-26T21:32:51Z","messageCount":13,"participants":["Elijah Newren","Eric Sunshine","Junio C Hamano","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"374508","messageId":"20190425155118.7918-1-newren@gmail.com","threadId":"50991","inReplyTo":null,"subject":"[PATCH 0/5] Fix and extend encoding handling in fast export/import","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:51:13Z","receivedAt":"2019-04-25T15:51:29Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"While stress testing `git filter-repo`, I noticed an issue with\nencoding; further digging led to the fixes and features in this series.\nSee the individual commit messages for details.\n\nElijah Newren (5):\n  t9350: fix encoding test to actually test reencoding\n  fast-import: support 'encoding' commit header\n  fast-export: avoid stripping encoding header if we cannot reencode\n  fast-export: differentiate between explicitly utf-8 and implicitly\n    utf-8\n  fast-export: do automatic reencoding of commit messages only if\n    requested\n\n Documentation/git-fast-import.txt |  7 ++++\n builtin/fast-export.c             | 44 ++++++++++++++++++++----\n fast-import.c                     | 12 +++++--\n t/t9300-fast-import.sh            | 20 +++++++++++\n t/t9350-fast-export.sh            | 57 ++++++++++++++++++++++++++-----\n 5 files changed, 123 insertions(+), 17 deletions(-)\n\n-- \n2.21.0.779.g2f4b9c5032\n"},{"id":"374509","messageId":"20190425155118.7918-2-newren@gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-1-newren@gmail.com","subject":"[PATCH 1/5] t9350: fix encoding test to actually test reencoding","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:51:14Z","receivedAt":"2019-04-25T15:51:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This test used an author with non-ascii characters in the name, but\nno special commit message.  It then grep'ed for those non-ascii\ncharacters, but those are guaranteed to exist regardless of the\nreencoding process since the reencoding only affects the commit message,\nnot the author or committer names.  As such, the test would work even if\nthe re-encoding process simply stripped the commit message entirely.\nModify the test to actually check that the reencoding in utf-8 worked.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n t/t9350-fast-export.sh | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 5690fe2810..6c07f910eb 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -94,22 +94,22 @@ test_expect_success 'fast-export --show-original-ids | git fast-import' '\n \ttest $MUSS = $(git rev-parse --verify refs/tags/muss)\n '\n \n-test_expect_success 'iso-8859-1' '\n+test_expect_success 'iso-8859-7' '\n \n-\tgit config i18n.commitencoding ISO8859-1 &&\n-\t# use author and committer name in ISO-8859-1 to match it.\n-\t. \"$TEST_DIRECTORY\"/t3901/8859-1.txt &&\n+\ttest_when_finished \"git reset --hard HEAD~1\" &&\n+\ttest_when_finished \"git config --unset i18n.commitencoding\" &&\n+\tgit config i18n.commitencoding iso-8859-7 &&\n \ttest_tick &&\n \techo rosten >file &&\n-\tgit commit -s -m den file &&\n-\tgit fast-export wer^..wer >iso8859-1.fi &&\n-\tsed \"s/wer/i18n/\" iso8859-1.fi |\n+\tgit commit -s -m \"$(printf \"Pi: \\360\")\" file &&\n+\tgit fast-export wer^..wer >iso-8859-7.fi &&\n+\tsed \"s/wer/i18n/\" iso-8859-7.fi |\n \t\t(cd new &&\n \t\t git fast-import &&\n \t\t git cat-file commit i18n >actual &&\n-\t\t grep \"Áéí óú\" actual)\n-\n+\t\t grep $(printf \"\\317\\200\") actual)\n '\n+\n test_expect_success 'import/export-marks' '\n \n \tgit checkout -b marks master &&\n@@ -224,7 +224,6 @@ GIT_COMMITTER_NAME='C O Mitter'; export GIT_COMMITTER_NAME\n \n test_expect_success 'setup copies' '\n \n-\tgit config --unset i18n.commitencoding &&\n \tgit checkout -b copy rein &&\n \tgit mv file file3 &&\n \tgit commit -m move1 &&\n-- \n2.21.0.779.g2f4b9c5032\n\n"},{"id":"374510","messageId":"20190425155118.7918-3-newren@gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-1-newren@gmail.com","subject":"[PATCH 2/5] fast-import: support 'encoding' commit header","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:51:15Z","receivedAt":"2019-04-25T15:51:32Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Since git supports commit messages with an encoding other than utf-8,\nallow fast-import to import such commits.  This may be useful for folks\nwho do not want to reencode commit messages from an external system, and\nmay also be useful to achieve reversible history rewrites (e.g. sha1sum\n<-> sha256sum transitions or subtree work) with git repositories that\nhave used specialized encodings in their commit history.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n Documentation/git-fast-import.txt |  7 +++++++\n fast-import.c                     | 12 ++++++++++--\n t/t9300-fast-import.sh            | 20 ++++++++++++++++++++\n 3 files changed, 37 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex d65cdb3d08..7baf9e47b5 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -388,6 +388,7 @@ change to the project.\n \toriginal-oid?\n \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n+\t('encoding' SP <encoding>)?\n \tdata\n \t('from' SP <commit-ish> LF)?\n \t('merge' SP <commit-ish> LF)?\n@@ -455,6 +456,12 @@ that was selected by the --date-format=<fmt> command-line option.\n See ``Date Formats'' above for the set of supported formats, and\n their syntax.\n \n+`encoding`\n+^^^^^^^^^^\n+The optional `encoding` command indicates the encoding of the commit\n+message.  Most commits are UTF-8 and the encoding is omitted, but this\n+allows importing commit messages into git without first reencoding them.\n+\n `from`\n ^^^^^^\n The `from` command is used to specify the commit to initialize\ndiff --git a/fast-import.c b/fast-import.c\nindex f38d04fa58..25026c068a 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2585,6 +2585,7 @@ static void parse_new_commit(const char *arg)\n \tstruct branch *b;\n \tchar *author = NULL;\n \tchar *committer = NULL;\n+\tconst char *encoding = NULL;\n \tstruct hash_list *merge_list = NULL;\n \tunsigned int merge_count;\n \tunsigned char prev_fanout, new_fanout;\n@@ -2607,6 +2608,9 @@ static void parse_new_commit(const char *arg)\n \t}\n \tif (!committer)\n \t\tdie(\"Expected committer but didn't get one\");\n+\tif (skip_prefix(command_buf.buf, \"encoding \", &encoding)) {\n+\t\tread_next_command();\n+\t}\n \tparse_data(&msg, 0, NULL);\n \tread_next_command();\n \tparse_from(b);\n@@ -2670,9 +2674,13 @@ static void parse_new_commit(const char *arg)\n \t}\n \tstrbuf_addf(&new_data,\n \t\t\"author %s\\n\"\n-\t\t\"committer %s\\n\"\n-\t\t\"\\n\",\n+\t\t\"committer %s\\n\",\n \t\tauthor ? author : committer, committer);\n+\tif (encoding)\n+\t\tstrbuf_addf(&new_data,\n+\t\t\t\"encoding %s\\n\",\n+\t\t\tencoding);\n+\tstrbuf_addf(&new_data, \"\\n\");\n \tstrbuf_addbuf(&new_data, &msg);\n \tfree(author);\n \tfree(committer);\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 3668263c40..141b7fa35e 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -3299,4 +3299,24 @@ test_expect_success !MINGW 'W: get-mark & empty orphan commit with erroneous thi\n \tsed -e s/LFs/LLL/ W-input | tr L \"\\n\" | test_must_fail git fast-import\n '\n \n+###\n+### series X (other new features)\n+###\n+\n+test_expect_success 'X: handling encoding' '\n+\ttest_tick &&\n+\tcat >input <<-INPUT_END &&\n+\tcommit refs/heads/encoding\n+\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\tencoding iso-8859-7\n+\tdata <<COMMIT\n+\tINPUT_END\n+\n+\tprintf \"Pi: \\360\\nCOMMIT\\n\" >>input &&\n+\n+\tgit fast-import <input &&\n+\tgit cat-file -p encoding | grep $(printf \"\\360\") &&\n+\tgit log -1 --format=%B encoding | grep $(printf \"\\317\\200\")\n+'\n+\n test_done\n-- \n2.21.0.779.g2f4b9c5032\n\n"},{"id":"374511","messageId":"20190425155118.7918-4-newren@gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-1-newren@gmail.com","subject":"[PATCH 3/5] fast-export: avoid stripping encoding header if we cannot reencode","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:51:16Z","receivedAt":"2019-04-25T15:51:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"When fast-export encounters a commit with an 'encoding' header, it tries\nto reencode in utf-8 and then drops the encoding header.  However, if it\nfails to reencode in utf-8 because e.g. one of the characters in the\ncommit message was invalid in the old encoding, then we need to retain\nthe original encoding or otherwise we lose information needed to\nunderstand all the other (valid) characters in the original commit\nmessage.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n builtin/fast-export.c  |  7 +++++--\n t/t9350-fast-export.sh | 15 +++++++++++++++\n 2 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 9e283482ef..7734a9f5a5 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -642,9 +642,12 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tprintf(\"commit %s\\nmark :%\"PRIu32\"\\n\", refname, last_idnum);\n \tif (show_original_ids)\n \t\tprintf(\"original-oid %s\\n\", oid_to_hex(&commit->object.oid));\n-\tprintf(\"%.*s\\n%.*s\\ndata %u\\n%s\",\n+\tprintf(\"%.*s\\n%.*s\\n\",\n \t       (int)(author_end - author), author,\n-\t       (int)(committer_end - committer), committer,\n+\t       (int)(committer_end - committer), committer);\n+\tif (!reencoded && encoding)\n+\t\tprintf(\"encoding %s\\n\", encoding);\n+\tprintf(\"data %u\\n%s\",\n \t       (unsigned)(reencoded\n \t\t\t  ? strlen(reencoded) : message\n \t\t\t  ? strlen(message) : 0),\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 6c07f910eb..975c8c4014 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -110,6 +110,21 @@ test_expect_success 'iso-8859-7' '\n \t\t grep $(printf \"\\317\\200\") actual)\n '\n \n+test_expect_success 'encoding preserved if reencoding fails' '\n+\n+\ttest_when_finished \"git reset --hard HEAD~1\" &&\n+\ttest_when_finished \"git config --unset i18n.commitencoding\" &&\n+\tgit config i18n.commitencoding iso-8859-7 &&\n+\techo rosten >file &&\n+\tgit commit -s -m \"$(printf \"Pi: \\360; Invalid: \\377\")\" file &&\n+\tgit fast-export wer^..wer >iso-8859-7.fi &&\n+\tsed \"s/wer/i18n-invalid/\" iso-8859-7.fi |\n+\t\t(cd new &&\n+\t\t git fast-import &&\n+\t\t git cat-file commit i18n-invalid >actual &&\n+\t\t grep ^encoding actual)\n+'\n+\n test_expect_success 'import/export-marks' '\n \n \tgit checkout -b marks master &&\n-- \n2.21.0.779.g2f4b9c5032\n\n"},{"id":"374512","messageId":"20190425155118.7918-6-newren@gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-1-newren@gmail.com","subject":"[PATCH 5/5] fast-export: do automatic reencoding of commit messages only if requested","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:51:18Z","receivedAt":"2019-04-25T15:51:35Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Automatic re-encoding of commit messages (and dropping of the encoding\nheader) hurts attempts to do reversible history rewrites (e.g. sha1sum\n<-> sha256sum transitions, some subtree rewrites), and seems\ninconsistent with the general principle followed elsewhere in\nfast-export of requiring explicit user requests to modify the output\n(e.g. --signed-tags=strip, --tag-of-filtered-object=rewrite).  Add a\n--reencode flag that the user can use to specify, and like other\nfast-export flags, default it to 'abort'.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n builtin/fast-export.c  | 35 ++++++++++++++++++++++++++++++++---\n t/t9350-fast-export.sh | 31 ++++++++++++++++++++++++++++---\n 2 files changed, 60 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 66331fa401..43cc52331c 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -33,6 +33,7 @@ static const char *fast_export_usage[] = {\n static int progress;\n static enum { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;\n static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;\n+static enum { REENCODE_ABORT, REENCODE_PLEASE, REENCODE_NEVER } reencode_mode = REENCODE_ABORT;\n static int fake_missing_tagger;\n static int use_done_feature;\n static int no_data;\n@@ -77,6 +78,20 @@ static int parse_opt_tag_of_filtered_mode(const struct option *opt,\n \treturn 0;\n }\n \n+static int parse_opt_reencode_mode(const struct option *opt,\n+\t\t\t\t   const char *arg, int unset)\n+{\n+\tif (unset || !strcmp(arg, \"abort\"))\n+\t\treencode_mode = REENCODE_ABORT;\n+\telse if (!strcmp(arg, \"yes\"))\n+\t\treencode_mode = REENCODE_PLEASE;\n+\telse if (!strcmp(arg, \"no\"))\n+\t\treencode_mode = REENCODE_NEVER;\n+\telse\n+\t\treturn error(\"Unknown reencoding mode: %s\", arg);\n+\treturn 0;\n+}\n+\n static struct decoration idnums;\n static uint32_t last_idnum;\n \n@@ -633,10 +648,21 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t}\n \n \tmark_next_object(&commit->object);\n-\tif (anonymize)\n+\tif (anonymize) {\n \t\treencoded = anonymize_commit_message(message);\n-\telse if (!is_encoding_utf8(encoding))\n-\t\treencoded = reencode_string(message, \"UTF-8\", encoding);\n+\t} else if (encoding) {\n+\t\tswitch(reencode_mode) {\n+\t\tcase REENCODE_PLEASE:\n+\t\t\treencoded = reencode_string(message, \"UTF-8\", encoding);\n+\t\t\tbreak;\n+\t\tcase REENCODE_NEVER:\n+\t\t\tbreak;\n+\t\tcase REENCODE_ABORT:\n+\t\t\tdie(\"Encountered commit-specific encoding %s in commit \"\n+\t\t\t    \"%s; use --reencode=<mode> to handle it\",\n+\t\t\t    encoding, oid_to_hex(&commit->object.oid));\n+\t\t}\n+\t}\n \tif (!commit->parents)\n \t\tprintf(\"reset %s\\n\", refname);\n \tprintf(\"commit %s\\nmark :%\"PRIu32\"\\n\", refname, last_idnum);\n@@ -1091,6 +1117,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK(0, \"tag-of-filtered-object\", &tag_of_filtered_mode, N_(\"mode\"),\n \t\t\t     N_(\"select handling of tags that tag filtered objects\"),\n \t\t\t     parse_opt_tag_of_filtered_mode),\n+\t\tOPT_CALLBACK(0, \"reencode\", &reencode_mode, N_(\"mode\"),\n+\t\t\t     N_(\"select handling of commit messages in an alternate encoding\"),\n+\t\t\t     parse_opt_reencode_mode),\n \t\tOPT_STRING(0, \"export-marks\", &export_filename, N_(\"file\"),\n \t\t\t     N_(\"Dump marks to this file\")),\n \t\tOPT_STRING(0, \"import-marks\", &import_filename, N_(\"file\"),\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 975c8c4014..4774926bb6 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -94,7 +94,7 @@ test_expect_success 'fast-export --show-original-ids | git fast-import' '\n \ttest $MUSS = $(git rev-parse --verify refs/tags/muss)\n '\n \n-test_expect_success 'iso-8859-7' '\n+test_expect_success 'reencoding iso-8859-7' '\n \n \ttest_when_finished \"git reset --hard HEAD~1\" &&\n \ttest_when_finished \"git config --unset i18n.commitencoding\" &&\n@@ -102,7 +102,7 @@ test_expect_success 'iso-8859-7' '\n \ttest_tick &&\n \techo rosten >file &&\n \tgit commit -s -m \"$(printf \"Pi: \\360\")\" file &&\n-\tgit fast-export wer^..wer >iso-8859-7.fi &&\n+\tgit fast-export --reencode=yes wer^..wer >iso-8859-7.fi &&\n \tsed \"s/wer/i18n/\" iso-8859-7.fi |\n \t\t(cd new &&\n \t\t git fast-import &&\n@@ -110,6 +110,31 @@ test_expect_success 'iso-8859-7' '\n \t\t grep $(printf \"\\317\\200\") actual)\n '\n \n+test_expect_success 'aborting on iso-8859-7' '\n+\n+\ttest_when_finished \"git reset --hard HEAD~1\" &&\n+\ttest_when_finished \"git config --unset i18n.commitencoding\" &&\n+\tgit config i18n.commitencoding iso-8859-7 &&\n+\techo rosten >file &&\n+\tgit commit -s -m \"$(printf \"Pi: \\360\")\" file &&\n+\ttest_must_fail git fast-export --reencode=abort wer^..wer >iso-8859-7.fi\n+'\n+\n+test_expect_success 'preserving iso-8859-7' '\n+\n+\ttest_when_finished \"git reset --hard HEAD~1\" &&\n+\ttest_when_finished \"git config --unset i18n.commitencoding\" &&\n+\tgit config i18n.commitencoding iso-8859-7 &&\n+\techo rosten >file &&\n+\tgit commit -s -m \"$(printf \"Pi: \\360\")\" file &&\n+\tgit fast-export --reencode=no wer^..wer >iso-8859-7.fi &&\n+\tsed \"s/wer/i18n-no-recoding/\" iso-8859-7.fi |\n+\t\t(cd new &&\n+\t\t git fast-import &&\n+\t\t git cat-file commit i18n-no-recoding >actual &&\n+\t\t grep $(printf \"\\360\") actual)\n+'\n+\n test_expect_success 'encoding preserved if reencoding fails' '\n \n \ttest_when_finished \"git reset --hard HEAD~1\" &&\n@@ -117,7 +142,7 @@ test_expect_success 'encoding preserved if reencoding fails' '\n \tgit config i18n.commitencoding iso-8859-7 &&\n \techo rosten >file &&\n \tgit commit -s -m \"$(printf \"Pi: \\360; Invalid: \\377\")\" file &&\n-\tgit fast-export wer^..wer >iso-8859-7.fi &&\n+\tgit fast-export --reencode=yes wer^..wer >iso-8859-7.fi &&\n \tsed \"s/wer/i18n-invalid/\" iso-8859-7.fi |\n \t\t(cd new &&\n \t\t git fast-import &&\n-- \n2.21.0.779.g2f4b9c5032\n\n"},{"id":"374513","messageId":"20190425155118.7918-5-newren@gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-1-newren@gmail.com","subject":"[PATCH 4/5] fast-export: differentiate between explicitly utf-8 and implicitly utf-8","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:51:17Z","receivedAt":"2019-04-25T15:51:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"The find_encoding() function returned the encoding used by a commit\nmessage, returning a default of git_commit_encoding (usually utf-8).\nAlthough the current code does not differentiate between a commit which\nexplicitly requested utf-8 and one where we just assume utf-8 because no\nencoding is set, it will become important when we try to preserve the\nencoding header.  Since is_encoding_utf8() returns true when passed\nNULL, we can just return NULL from find_encoding() instead of returning\ngit_commit_encoding.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n builtin/fast-export.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 7734a9f5a5..66331fa401 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -453,7 +453,7 @@ static const char *find_encoding(const char *begin, const char *end)\n \tbol = memmem(begin, end ? end - begin : strlen(begin),\n \t\t     needle, strlen(needle));\n \tif (!bol)\n-\t\treturn git_commit_encoding;\n+\t\treturn NULL;\n \tbol += strlen(needle);\n \teol = strchrnul(bol, '\\n');\n \t*eol = '\\0';\n-- \n2.21.0.779.g2f4b9c5032\n\n"},{"id":"374514","messageId":"CABPp-BEsSpFYVto2E998n-UUwD6UcVRVP61VPVg1nEgRUL1UzQ@mail.gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-1-newren@gmail.com","subject":"Re: [PATCH 0/5] Fix and extend encoding handling in fast export/import","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:55:11Z","receivedAt":"2019-04-25T15:55:25Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Apr 25, 2019 at 9:51 AM Elijah Newren <newren@gmail.com> wrote:\n>\n> While stress testing `git filter-repo`, I noticed an issue with\n> encoding; further digging led to the fixes and features in this series.\n> See the individual commit messages for details.\n\nWhoops, forgot to cc Brian; I'm curious if my understanding is correct\nabout the sha256sum transition plans that the intent in the short term\nis using fast-export & fast-import to transition to-and-from a\nsha256sum repo on the fly; if so, I believe that transition work\nshould use the new --reencode=yes option in patch five.\n"},{"id":"374515","messageId":"CABPp-BHLgTM50m4gVoLXWDZbHPojpOyigDnP7ux9zMHKzH2myQ@mail.gmail.com","threadId":"50991","inReplyTo":"CABPp-BEsSpFYVto2E998n-UUwD6UcVRVP61VPVg1nEgRUL1UzQ@mail.gmail.com","subject":"Re: [PATCH 0/5] Fix and extend encoding handling in fast export/import","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-25T15:57:47Z","receivedAt":"2019-04-25T15:58:02Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Apr 25, 2019 at 9:55 AM Elijah Newren <newren@gmail.com> wrote:\n>\n> On Thu, Apr 25, 2019 at 9:51 AM Elijah Newren <newren@gmail.com> wrote:\n> >\n> > While stress testing `git filter-repo`, I noticed an issue with\n> > encoding; further digging led to the fixes and features in this series.\n> > See the individual commit messages for details.\n>\n> Whoops, forgot to cc Brian; I'm curious if my understanding is correct\n> about the sha256sum transition plans that the intent in the short term\n> is using fast-export & fast-import to transition to-and-from a\n> sha256sum repo on the fly; if so, I believe that transition work\n> should use the new --reencode=yes option in patch five.\n\nI seem to be struggling with distractions this morning; I mean the\n`--reencode=no` option from patch 5/5.\n"},{"id":"374520","messageId":"CAPig+cR2kOmKMmSu4hP6gJp_S6t9QQCnMtKjTp6QEw+JwQxG=w@mail.gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-2-newren@gmail.com","subject":"Re: [PATCH 1/5] t9350: fix encoding test to actually test reencoding","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-04-25T17:52:23Z","receivedAt":"2019-04-25T17:52:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 25, 2019 at 11:51 AM Elijah Newren <newren@gmail.com> wrote:\n> This test used an author with non-ascii characters in the name, but\n> no special commit message.  It then grep'ed for those non-ascii\n> characters, but those are guaranteed to exist regardless of the\n> reencoding process since the reencoding only affects the commit message,\n> not the author or committer names.  As such, the test would work even if\n> the re-encoding process simply stripped the commit message entirely.\n> Modify the test to actually check that the reencoding in utf-8 worked.\n>\n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\n> @@ -94,22 +94,22 @@ test_expect_success 'fast-export --show-original-ids | git fast-import' '\n> +test_expect_success 'iso-8859-7' '\n> +       test_when_finished \"git config --unset i18n.commitencoding\" &&\n> +       git config i18n.commitencoding iso-8859-7 &&\n\nAren't these two lines are pretty much equivalent to this single line?\n\n    test_config i18n.commitencoding iso-8859-7 &&\n"},{"id":"374524","messageId":"CAPig+cRaOAX+Yap+SiPjEoPRcH9z5qTHV6KPGGwN2LfPfxyA8Q@mail.gmail.com","threadId":"50991","inReplyTo":"20190425155118.7918-3-newren@gmail.com","subject":"Re: [PATCH 2/5] fast-import: support 'encoding' commit header","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-04-25T19:36:57Z","receivedAt":"2019-04-25T19:37:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 25, 2019 at 11:51 AM Elijah Newren <newren@gmail.com> wrote:\n> Since git supports commit messages with an encoding other than utf-8,\n> allow fast-import to import such commits.  This may be useful for folks\n> who do not want to reencode commit messages from an external system, and\n> may also be useful to achieve reversible history rewrites (e.g. sha1sum\n> <-> sha256sum transitions or subtree work) with git repositories that\n> have used specialized encodings in their commit history.\n>\n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n> diff --git a/fast-import.c b/fast-import.c\n> @@ -2607,6 +2608,9 @@ static void parse_new_commit(const char *arg)\n>         if (!committer)\n>                 die(\"Expected committer but didn't get one\");\n> +       if (skip_prefix(command_buf.buf, \"encoding \", &encoding)) {\n> +               read_next_command();\n> +       }\n\nStyle nit: unnecessary braces\n\n> @@ -2670,9 +2674,13 @@ static void parse_new_commit(const char *arg)\n>         strbuf_addf(&new_data,\n>                 \"author %s\\n\"\n> -               \"committer %s\\n\"\n> -               \"\\n\",\n> +               \"committer %s\\n\",\n>                 author ? author : committer, committer);\n> +       if (encoding)\n> +               strbuf_addf(&new_data,\n> +                       \"encoding %s\\n\",\n> +                       encoding);\n> +       strbuf_addf(&new_data, \"\\n\");\n\nAlternately:\n\n    strbuf_addch(&new_data, '\\n');\n\n> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\n> @@ -3299,4 +3299,24 @@ test_expect_success !MINGW 'W: get-mark & empty orphan commit with erroneous thi\n> +test_expect_success 'X: handling encoding' '\n> +       test_tick &&\n> +       [...]\n> +       git cat-file -p encoding | grep $(printf \"\\360\") &&\n> +       git log -1 --format=%B encoding | grep $(printf \"\\317\\200\")\n\nThis script is already full of instances of Git commands upstream of\npipes, so this usage is consistent (despite recent work to eliminate\nsuch situations). Okay.\n"},{"id":"374538","messageId":"xmqq7ebhwkt6.fsf@gitster-ct.c.googlers.com","threadId":"50991","inReplyTo":"CAPig+cR2kOmKMmSu4hP6gJp_S6t9QQCnMtKjTp6QEw+JwQxG=w@mail.gmail.com","subject":"Re: [PATCH 1/5] t9350: fix encoding test to actually test reencoding","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-26T02:40:37Z","receivedAt":"2019-04-26T02:40:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Thu, Apr 25, 2019 at 11:51 AM Elijah Newren <newren@gmail.com> wrote:\n>> This test used an author with non-ascii characters in the name, but\n>> no special commit message.  It then grep'ed for those non-ascii\n>> characters, but those are guaranteed to exist regardless of the\n>> reencoding process since the reencoding only affects the commit message,\n>> not the author or committer names.  As such, the test would work even if\n>> the re-encoding process simply stripped the commit message entirely.\n>> Modify the test to actually check that the reencoding in utf-8 worked.\n>>\n>> Signed-off-by: Elijah Newren <newren@gmail.com>\n>> ---\n>> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\n>> @@ -94,22 +94,22 @@ test_expect_success 'fast-export --show-original-ids | git fast-import' '\n>> +test_expect_success 'iso-8859-7' '\n>> +       test_when_finished \"git config --unset i18n.commitencoding\" &&\n>> +       git config i18n.commitencoding iso-8859-7 &&\n>\n> Aren't these two lines are pretty much equivalent to this single line?\n>\n>     test_config i18n.commitencoding iso-8859-7 &&\n\nYes.\n"},{"id":"374553","messageId":"CABPp-BHwyKJuRbYt65HXwLf-AxHRXOr1LMP0WbeSFc05sJVBVQ@mail.gmail.com","threadId":"50991","inReplyTo":"CAPig+cRaOAX+Yap+SiPjEoPRcH9z5qTHV6KPGGwN2LfPfxyA8Q@mail.gmail.com","subject":"Re: [PATCH 2/5] fast-import: support 'encoding' commit header","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-26T11:39:58Z","receivedAt":"2019-04-26T11:40:30Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Eric,\n\nOn Thu, Apr 25, 2019 at 1:37 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Thu, Apr 25, 2019 at 11:51 AM Elijah Newren <newren@gmail.com> wrote:\n> > Since git supports commit messages with an encoding other than utf-8,\n> > allow fast-import to import such commits.  This may be useful for folks\n> > who do not want to reencode commit messages from an external system, and\n> > may also be useful to achieve reversible history rewrites (e.g. sha1sum\n> > <-> sha256sum transitions or subtree work) with git repositories that\n> > have used specialized encodings in their commit history.\n> >\n> > Signed-off-by: Elijah Newren <newren@gmail.com>\n> > ---\n> > diff --git a/fast-import.c b/fast-import.c\n> > @@ -2607,6 +2608,9 @@ static void parse_new_commit(const char *arg)\n> >         if (!committer)\n> >                 die(\"Expected committer but didn't get one\");\n> > +       if (skip_prefix(command_buf.buf, \"encoding \", &encoding)) {\n> > +               read_next_command();\n> > +       }\n>\n> Style nit: unnecessary braces\n>\n> > @@ -2670,9 +2674,13 @@ static void parse_new_commit(const char *arg)\n> >         strbuf_addf(&new_data,\n> >                 \"author %s\\n\"\n> > -               \"committer %s\\n\"\n> > -               \"\\n\",\n> > +               \"committer %s\\n\",\n> >                 author ? author : committer, committer);\n> > +       if (encoding)\n> > +               strbuf_addf(&new_data,\n> > +                       \"encoding %s\\n\",\n> > +                       encoding);\n> > +       strbuf_addf(&new_data, \"\\n\");\n>\n> Alternately:\n>\n>     strbuf_addch(&new_data, '\\n');\n\nThanks for taking a look.  I'll fix both of these items you\nhighlighted and the test_config item you pointed out in 1/5 in the\nnext re-roll.\n\n> > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\n> > @@ -3299,4 +3299,24 @@ test_expect_success !MINGW 'W: get-mark & empty orphan commit with erroneous thi\n> > +test_expect_success 'X: handling encoding' '\n> > +       test_tick &&\n> > +       [...]\n> > +       git cat-file -p encoding | grep $(printf \"\\360\") &&\n> > +       git log -1 --format=%B encoding | grep $(printf \"\\317\\200\")\n>\n> This script is already full of instances of Git commands upstream of\n> pipes, so this usage is consistent (despite recent work to eliminate\n> such situations). Okay.\n"},{"id":"374579","messageId":"20190426213241.GP6316@genre.crustytoothpaste.net","threadId":"50991","inReplyTo":"CABPp-BEsSpFYVto2E998n-UUwD6UcVRVP61VPVg1nEgRUL1UzQ@mail.gmail.com","subject":"Re: [PATCH 0/5] Fix and extend encoding handling in fast export/import","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-04-26T21:32:41Z","receivedAt":"2019-04-26T21:32:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Apr 25, 2019 at 09:55:11AM -0600, Elijah Newren wrote:\n> On Thu, Apr 25, 2019 at 9:51 AM Elijah Newren <newren@gmail.com> wrote:\n> >\n> > While stress testing `git filter-repo`, I noticed an issue with\n> > encoding; further digging led to the fixes and features in this series.\n> > See the individual commit messages for details.\n> \n> Whoops, forgot to cc Brian; I'm curious if my understanding is correct\n> about the sha256sum transition plans that the intent in the short term\n> is using fast-export & fast-import to transition to-and-from a\n> sha256sum repo on the fly; if so, I believe that transition work\n> should use the new --reencode=yes option in patch five.\n\nThe plan is to convert using fast-import and fast-export, yes, but\non the fly, no. You'll convert your repository up front using\nfast-import and fast-export and then conversion will happen on the fly\nas needed internally. The latter is a thing I'm working on.\n\nSo individual users will want to use the --reencode option, but\ninternally we probably won't get as far as actually decoding most of the\ncommit object, so we'll keep the bytes in place.\n\nI do appreciate the CC, though.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"}]}