{"thread":{"id":"54977","subject":"[PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","startedAt":"2021-01-12T08:48:28Z","lastAt":"2021-01-19T15:55:02Z","messageCount":31,"participants":["Johannes Schindelin via GitGitGadget","SZEDER Gábor","Jeff King","Ævar Arnfjörð Bjarmason","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"414101","messageId":"pull.836.git.1610441262.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":null,"subject":"[PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:31Z","receivedAt":"2021-01-12T08:48:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This patch series is incomplete because I lack the time to finish it, and I\nhope that there are interested developers who can help with it.\n\nIn https://lore.kernel.org/git/20181022153633.31757-1-pclouds@gmail.com/,\nDuy proposed introducing a fake language they called \"Ook\" to be able to\ntest translations better. \"Ook\" would \"translate\" messages without any\nprintf format specifiers to \"Eek\", otherwise convert all upper-case letters\nto \"Ook\" and lower-case letters to \"ook\".\n\nGábor replied with a different proposal that has the advantage of being\nbijective, i.e. it is possible to reconstruct the untranslated message from\nthe translated one.\n\nÆvar suggested recently\n[https://lore.kernel.org/git/xmqqim836v6m.fsf@gitster.c.googlers.com/T/#m6fdc43d4f1eb3f20f841096c59e985b69c84875e]\nthat this whole GETTEXT_POISON business is totally useless.\n\nI do not believe that it is useless. To back up my belief with something\ntangible, I implemented a GETTEXT_POISON=rot13 mode and ran the test suite\nto see whether we erroneously expect translated messages where they aren't,\nand related problems.\n\nAnd the experiment delivered. It is just a demonstration (I only addressed a\nhandful of the test suite failures, 124 test scripts still need to be\ninspected to find out why they fail), of course. Yet I think that finding\ne.g. the missing translations of sha1dc-cb and parse-options show that it\nwould be very useful to complete this patch series and then use the rot13\nmode in our CI jobs (instead of the current GETTEXT_POISON=true mode, which\nreally is less useful).\n\nNote: in its current form, this patch series fails t0013 and t9902 in the\nGETTEXT_POISON job\n[https://github.com/gitgitgadget/git/pull/836/checks?check_run_id=1686715225#step:4:1590],\ntherefore it cannot be integrated into Git as-is, even if it does not switch\nthe GETTEXT_POISON job to use the rot13 mode yet.\n\nSadly, even though I want to see this patch series go further, I am\noperating under serious time constraints and therefore had to admit to\nmyself, grudgingly, that I can't. So again, I hope that somebody else can\npush this effort further.\n\nJohannes Schindelin (11):\n  tests: remove unnecessary GIT_TEST_GETTEXT_POISON=false constructs\n  Support GIT_TEST_GETTEXT_POISON=rot13\n  parse-options: add forgotten translations\n  sha1dc: mark forgotten message for translation\n  t0006: use `test_i18ncmp` only for C locales\n  t0041: stop using `test_i18ngrep` on untranslated output\n  t0027: mark a function as requiring the C locale\n  t6301: do not expect the output of `for-each-ref` to be translated\n  GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`\n  GETTEXT_POISON=rot13: perform actual checks in `test_i18ngrep`\n  test-tool i18n: do not complain about empty messages\n\n Makefile                       |   1 +\n gettext.c                      |  65 ++++++++++++++++-\n gettext.h                      |   5 +-\n parse-options-cb.c             |   4 +-\n sha1dc_git.c                   |   2 +-\n t/helper/test-i18n.c           | 129 +++++++++++++++++++++++++++++++++\n t/helper/test-tool.c           |   1 +\n t/helper/test-tool.h           |   1 +\n t/t0006-date.sh                |   4 +-\n t/t0017-env-helper.sh          |   6 +-\n t/t0027-auto-crlf.sh           |   3 +\n t/t0041-usage.sh               |   2 +-\n t/t1305-config-include.sh      |   6 +-\n t/t6301-for-each-ref-errors.sh |   2 +-\n t/t7201-co.sh                  |   4 +-\n t/t9902-completion.sh          |   1 -\n t/test-lib-functions.sh        |  14 +++-\n t/test-lib.sh                  |   1 +\n 18 files changed, 228 insertions(+), 23 deletions(-)\n create mode 100644 t/helper/test-i18n.c\n\n\nbase-commit: 72c4083ddf91b489b7b7b812df67ee8842177d98\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-836%2Fdscho%2Fgettext-poison-should-rot13-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-836/dscho/gettext-poison-should-rot13-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/836\n-- \ngitgitgadget\n"},{"id":"414103","messageId":"978567b137e85d672c9f0591667d643945aa2bbb.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 03/11] parse-options: add forgotten translations","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:34Z","receivedAt":"2021-01-12T08:48:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n parse-options-cb.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex 4542d4d3f92..00862af486e 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -91,7 +91,7 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset)\n \tif (!arg)\n \t\treturn -1;\n \tif (get_oid(arg, &oid))\n-\t\treturn error(\"malformed object name %s\", arg);\n+\t\treturn error(_(\"malformed object name %s\"), arg);\n \tcommit = lookup_commit_reference(the_repository, &oid);\n \tif (!commit)\n \t\treturn error(\"no such commit %s\", arg);\n@@ -110,7 +110,7 @@ int parse_opt_commit(const struct option *opt, const char *arg, int unset)\n \tif (!arg)\n \t\treturn -1;\n \tif (get_oid(arg, &oid))\n-\t\treturn error(\"malformed object name %s\", arg);\n+\t\treturn error(_(\"malformed object name %s\"), arg);\n \tcommit = lookup_commit_reference(the_repository, &oid);\n \tif (!commit)\n \t\treturn error(\"no such commit %s\", arg);\n-- \ngitgitgadget\n\n"},{"id":"414104","messageId":"ab7832fc7ac783bc8f8c10014fc9747128fc37bc.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 02/11] Support GIT_TEST_GETTEXT_POISON=rot13","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:33Z","receivedAt":"2021-01-12T08:48:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n gettext.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--\n gettext.h |  5 +++--\n 2 files changed, 66 insertions(+), 4 deletions(-)\n\ndiff --git a/gettext.c b/gettext.c\nindex 35d2c1218db..7e8966d8326 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -68,11 +68,72 @@ const char *get_preferred_languages(void)\n int use_gettext_poison(void)\n {\n \tstatic int poison_requested = -1;\n-\tif (poison_requested == -1)\n-\t\tpoison_requested = git_env_bool(\"GIT_TEST_GETTEXT_POISON\", 0);\n+\tif (poison_requested == -1) {\n+\t\tconst char *v = getenv(\"GIT_TEST_GETTEXT_POISON\");\n+\t\tif (v && !strcmp(v, \"rot13\"))\n+\t\t\tpoison_requested = 2;\n+\t\telse\n+\t\t\tpoison_requested =\n+\t\t\t\tgit_env_bool(\"GIT_TEST_GETTEXT_POISON\", 0);\n+\t}\n \treturn poison_requested;\n }\n \n+static inline char do_rot13(char c)\n+{\n+\tif (c >= 'a' && c <= 'm')\n+\t\treturn c + 'n' - 'a';\n+\tif (c >= 'n' && c <= 'z')\n+\t\treturn c + 'a' - 'n';\n+\tif (c >= 'A' && c <= 'M')\n+\t\treturn c + 'N' - 'A';\n+\tif (c >= 'N' && c <= 'Z')\n+\t\treturn c + 'A' - 'N';\n+\treturn c;\n+}\n+\n+const char *gettext_maybe_rot13(const char *msgid)\n+{\n+\tstatic struct strbuf round_robin[4] = {\n+\t\tSTRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT\n+\t};\n+\tstatic int current;\n+\tstruct strbuf *buf;\n+\n+\tif (use_gettext_poison() != 2)\n+\t\treturn \"# GETTEXT POISON #\";\n+\n+\tbuf = &round_robin[current++];\n+\tif (current >= ARRAY_SIZE(round_robin))\n+\t\tcurrent = 0;\n+\n+\tstrbuf_reset(buf);\n+\twhile (*msgid) {\n+\t\tconst char *p = strchrnul(msgid, '%'), *spec;\n+\n+\t\twhile (*p && p[1] == '%')\n+\t\t\tp = strchrnul(p + 2, '%');\n+\n+\t\tif (p != msgid) {\n+\t\t\tstrbuf_addstr(buf, \"<rot13>\");\n+\t\t\twhile (p != msgid)\n+\t\t\t\tstrbuf_addch(buf, do_rot13(*(msgid++)));\n+\t\t\tstrbuf_addstr(buf, \"</rot13>\");\n+\t\t}\n+\n+\t\tif (!*p)\n+\t\t\tbreak;\n+\n+\t\tspec = strpbrk(p + 1, \"diouxXeEfFgGaAcsCSpnm%\");\n+\t\tif (!spec)\n+\t\t\tBUG(\"Unrecognized format string: %s\", p);\n+\t\tstrbuf_add(buf, p, spec + 1 - p);\n+\t\tmsgid = spec + 1;\n+\t}\n+\n+\treturn buf->buf;\n+}\n+\n #ifndef NO_GETTEXT\n static int test_vsnprintf(const char *fmt, ...)\n {\ndiff --git a/gettext.h b/gettext.h\nindex bee52eb1134..298adda7e15 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -29,6 +29,7 @@\n #define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))\n \n int use_gettext_poison(void);\n+const char *gettext_maybe_rot13(const char *msgid);\n \n #ifndef NO_GETTEXT\n void git_setup_gettext(void);\n@@ -48,14 +49,14 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)\n {\n \tif (!*msgid)\n \t\treturn \"\";\n-\treturn use_gettext_poison() ? \"# GETTEXT POISON #\" : gettext(msgid);\n+\treturn use_gettext_poison() ? gettext_maybe_rot13(msgid) : gettext(msgid);\n }\n \n static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)\n const char *Q_(const char *msgid, const char *plu, unsigned long n)\n {\n \tif (use_gettext_poison())\n-\t\treturn \"# GETTEXT POISON #\";\n+\t\treturn gettext_maybe_rot13(n < 2 ? msgid : plu);\n \treturn ngettext(msgid, plu, n);\n }\n \n-- \ngitgitgadget\n\n"},{"id":"414105","messageId":"004f90026031cb7ce71689481fabd27aa63485dd.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 01/11] tests: remove unnecessary GIT_TEST_GETTEXT_POISON=false constructs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:32Z","receivedAt":"2021-01-12T08:48:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe idea of the `GETTEXT_POISON` mode is to test translated messages, at\nleast _somewhat_.\n\nThere is not really any point in turning off that mode by force, except\n_maybe_ to test the mode itself.\n\nSo let's avoid overriding `GIT_TEST_GETTEXT_POISON` in the test suite\nunless testing the `GETTEXT_POISON` functionality itself.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t0017-env-helper.sh     | 6 ++----\n t/t1305-config-include.sh | 6 ++----\n t/t7201-co.sh             | 4 ++--\n t/t9902-completion.sh     | 1 -\n 4 files changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t0017-env-helper.sh b/t/t0017-env-helper.sh\nindex c1ecf6aeac6..e0931310306 100755\n--- a/t/t0017-env-helper.sh\n+++ b/t/t0017-env-helper.sh\n@@ -85,10 +85,8 @@ test_expect_success 'env--helper reads config thanks to trace2' '\n \tgit config -f home/.gitconfig include.path cycle &&\n \tgit config -f home/cycle include.path .gitconfig &&\n \n-\ttest_must_fail \\\n-\t\tenv HOME=\"$(pwd)/home\" GIT_TEST_GETTEXT_POISON=false \\\n-\t\tgit config -l 2>err &&\n-\tgrep \"exceeded maximum include depth\" err &&\n+\ttest_must_fail env HOME=\"$(pwd)/home\" git config -l 2>err &&\n+\ttest_i18ngrep \"exceeded maximum include depth\" err &&\n \n \ttest_must_fail \\\n \t\tenv HOME=\"$(pwd)/home\" GIT_TEST_GETTEXT_POISON=true \\\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex f1e1b289f98..308c5d530b3 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -352,10 +352,8 @@ test_expect_success 'include cycles are detected' '\n \tgit init --bare cycle &&\n \tgit -C cycle config include.path cycle &&\n \tgit config -f cycle/cycle include.path config &&\n-\ttest_must_fail \\\n-\t\tenv GIT_TEST_GETTEXT_POISON=false \\\n-\t\tgit -C cycle config --get-all test.value 2>stderr &&\n-\tgrep \"exceeded maximum include depth\" stderr\n+\ttest_must_fail git -C cycle config --get-all test.value 2>stderr &&\n+\ttest_i18ngrep \"exceeded maximum include depth\" stderr\n '\n \n test_done\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex b36a93056fd..630406a73c5 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -245,8 +245,8 @@ test_expect_success 'checkout to detach HEAD' '\n \trev=$(git rev-parse --short renamer^) &&\n \tgit checkout -f renamer &&\n \tgit clean -f &&\n-\tGIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&\n-\tgrep \"HEAD is now at $rev\" messages &&\n+\tgit checkout renamer^ 2>messages &&\n+\ttest_i18ngrep \"HEAD is now at $rev\" messages &&\n \ttest_line_count -gt 1 messages &&\n \tH=$(git rev-parse --verify HEAD) &&\n \tM=$(git show-ref -s --verify refs/heads/master) &&\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex a1c4f1f6d40..e5adee27d41 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2363,7 +2363,6 @@ test_expect_success 'sourcing the completion script clears cached commands' '\n '\n \n test_expect_success 'sourcing the completion script clears cached merge strategies' '\n-\tGIT_TEST_GETTEXT_POISON=false &&\n \t__git_compute_merge_strategies &&\n \tverbose test -n \"$__git_merge_strategies\" &&\n \t. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" &&\n-- \ngitgitgadget\n\n"},{"id":"414106","messageId":"d8e645f5b85a8c9dbdfa9f87b7570622f470ae59.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 06/11] t0041: stop using `test_i18ngrep` on untranslated output","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:37Z","receivedAt":"2021-01-12T08:49:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t0041-usage.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t0041-usage.sh b/t/t0041-usage.sh\nindex 5b927b76fe5..27309b1a8f8 100755\n--- a/t/t0041-usage.sh\n+++ b/t/t0041-usage.sh\n@@ -42,7 +42,7 @@ test_expect_success 'tag usage error' '\n \n test_expect_success 'branch --contains <existent_commit>' '\n \tgit branch --contains \"master\" >actual 2>actual.err &&\n-\ttest_i18ngrep \"master\" actual &&\n+\tgrep \"master\" actual &&\n \ttest_line_count = 0 actual.err\n '\n \n-- \ngitgitgadget\n\n"},{"id":"414107","messageId":"a6ea5a7327e5fe9079ac2f73816c5c1b6ad05253.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 05/11] t0006: use `test_i18ncmp` only for C locales","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:36Z","receivedAt":"2021-01-12T08:49:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWe cannot really test anything else, as we do not control the output of\n`strftime()`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t0006-date.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 75ee9a96b80..9f27e202d36 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -9,7 +9,7 @@ GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW\n check_relative() {\n \tt=$(($GIT_TEST_DATE_NOW - $1))\n \techo \"$t -> $2\" >expect\n-\ttest_expect_${3:-success} \"relative date ($2)\" \"\n+\ttest_expect_${3:-success} C_LOCALE_OUTPUT \"relative date ($2)\" \"\n \ttest-tool date relative $t >actual &&\n \ttest_i18ncmp expect actual\n \t\"\n@@ -137,7 +137,7 @@ check_approxidate '2009-12-01' '2009-12-01 19:20:00'\n check_date_format_human() {\n \tt=$(($GIT_TEST_DATE_NOW - $1))\n \techo \"$t -> $2\" >expect\n-\ttest_expect_success \"human date $t\" '\n+\ttest_expect_success C_LOCALE_OUTPUT \"human date $t\" '\n \t\ttest-tool date human $t >actual &&\n \t\ttest_i18ncmp expect actual\n '\n-- \ngitgitgadget\n\n"},{"id":"414108","messageId":"5aa21ffc7f94262a5c31fca80d998115d3a5522f.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 07/11] t0027: mark a function as requiring the C locale","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:38Z","receivedAt":"2021-01-12T08:49:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe `check_warning` function uses regular expressions matching English\ntext, it therefore won't work reliably with anything but the C locale.\n\nIn particular, we won't be able to match the needle 'in' under\nGETTEXT_POISON=rot13.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t0027-auto-crlf.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex 9fcd56fab37..baa5c6f435b 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -76,6 +76,9 @@ create_NNO_MIX_files () {\n }\n \n check_warning () {\n+\t# The `grep` and `sed` below expect the locale to be \"C\"\n+\ttest_have_prereq C_LOCALE_OUTPUT || return 0\n+\n \tcase \"$1\" in\n \tLF_CRLF) echo \"warning: LF will be replaced by CRLF\" >\"$2\".expect ;;\n \tCRLF_LF) echo \"warning: CRLF will be replaced by LF\" >\"$2\".expect ;;\n-- \ngitgitgadget\n\n"},{"id":"414109","messageId":"82e38c68f4406cb5bbaea3b4102b33097e29ea67.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 08/11] t6301: do not expect the output of `for-each-ref` to be translated","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:39Z","receivedAt":"2021-01-12T08:49:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe `test_i18ncmp` function is reserved for output that we expect to be\ntranslated.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t6301-for-each-ref-errors.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh\nindex 809854fc0ce..0cf2a41c26c 100755\n--- a/t/t6301-for-each-ref-errors.sh\n+++ b/t/t6301-for-each-ref-errors.sh\n@@ -20,7 +20,7 @@ test_expect_success 'Broken refs are reported correctly' '\n \ttest_when_finished \"rm -f .git/$r\" &&\n \techo \"warning: ignoring broken ref $r\" >broken-err &&\n \tgit for-each-ref >out 2>err &&\n-\ttest_i18ncmp full-list out &&\n+\ttest_cmp full-list out &&\n \ttest_i18ncmp broken-err err\n '\n \n-- \ngitgitgadget\n\n"},{"id":"414110","messageId":"0a56ebfcad79a03a6a8701db367cabb7953a5fa4.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 10/11] GETTEXT_POISON=rot13: perform actual checks in `test_i18ngrep`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:41Z","receivedAt":"2021-01-12T08:49:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-i18n.c    | 42 ++++++++++++++++++++++++++++++++++++++++-\n t/test-lib-functions.sh |  6 ++++++\n 2 files changed, 47 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-i18n.c b/t/helper/test-i18n.c\nindex 4b572e6efad..8a16df6b40f 100644\n--- a/t/helper/test-i18n.c\n+++ b/t/helper/test-i18n.c\n@@ -1,8 +1,10 @@\n #include \"test-tool.h\"\n #include \"cache.h\"\n+#include \"grep.h\"\n \n static const char *usage_msg = \"\\n\"\n-\"  test-tool i18n cmp <file1> <file2>\\n\";\n+\"  test-tool i18n cmp <file1> <file2>\\n\"\n+\"  test-tool i18n grep <regex> <file>\\n\";\n \n static inline char do_rot13(char c)\n {\n@@ -75,6 +77,42 @@ static int i18n_cmp(const char **argv)\n \treturn 0;\n }\n \n+static int i18n_grep(const char **argv)\n+{\n+\tint dont_match = 0;\n+\tconst char *pattern, *path;\n+\n+\tstruct grep_opt opt;\n+\tstruct grep_source source;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint hit;\n+\n+\tif (*argv && !strcmp(\"!\", *argv)) {\n+\t\tdont_match = 1;\n+\t\targv++;\n+\t}\n+\n+\tpattern = *(argv++);\n+\tpath = *(argv++);\n+\n+\tif (!pattern || !path || *argv)\n+\t\tusage(usage_msg);\n+\n+\tgrep_init(&opt, the_repository, NULL);\n+\tappend_grep_pattern(&opt, pattern, \"command line\", 0, GREP_PATTERN);\n+\tcompile_grep_patterns(&opt);\n+\n+\tif (strbuf_read_file(&buf, path, 0) < 0)\n+\t\tdie_errno(\"could not read %s\", path);\n+\tunrot13_strbuf(&buf);\n+\tgrep_source_init(&source, GREP_SOURCE_BUF, path, path, path);\n+\tsource.buf = buf.buf;\n+\tsource.size = buf.len;\n+\thit = grep_source(&opt, &source);\n+\tstrbuf_release(&buf);\n+\treturn dont_match ^ !hit;\n+}\n+\n int cmd__i18n(int argc, const char **argv)\n {\n \targv++;\n@@ -82,6 +120,8 @@ int cmd__i18n(int argc, const char **argv)\n \t\tusage(usage_msg);\n \tif (!strcmp(*argv, \"cmp\"))\n \t\treturn i18n_cmp(argv+1);\n+\telse if (!strcmp(*argv, \"grep\"))\n+\t\treturn i18n_grep(argv+1);\n \telse\n \t\tusage(usage_msg);\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 08731bae854..394fd2ef5be 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1021,6 +1021,12 @@ test_i18ngrep () {\n \n \tif test_have_prereq !C_LOCALE_OUTPUT\n \tthen\n+\t\tif test rot13 = \"$GIT_TEST_GETTEXT_POISON\"\n+\t\tthen\n+\t\t\ttest-tool i18n grep \"$@\"\n+\t\t\treturn $?\n+\t\tfi\n+\n \t\t# pretend success\n \t\treturn 0\n \tfi\n-- \ngitgitgadget\n\n"},{"id":"414111","messageId":"4669ccbb1ae40f0c58a2d8e3c8b3a34d82176c7a.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:40Z","receivedAt":"2021-01-12T08:49:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Makefile                |  1 +\n t/helper/test-i18n.c    | 89 +++++++++++++++++++++++++++++++++++++++++\n t/helper/test-tool.c    |  1 +\n t/helper/test-tool.h    |  1 +\n t/test-lib-functions.sh |  8 +++-\n t/test-lib.sh           |  1 +\n 6 files changed, 100 insertions(+), 1 deletion(-)\n create mode 100644 t/helper/test-i18n.c\n\ndiff --git a/Makefile b/Makefile\nindex 7b64106930a..d49f80cb524 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -710,6 +710,7 @@ TEST_BUILTINS_OBJS += test-genzeros.o\n TEST_BUILTINS_OBJS += test-hash-speed.o\n TEST_BUILTINS_OBJS += test-hash.o\n TEST_BUILTINS_OBJS += test-hashmap.o\n+TEST_BUILTINS_OBJS += test-i18n.o\n TEST_BUILTINS_OBJS += test-index-version.o\n TEST_BUILTINS_OBJS += test-json-writer.o\n TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o\ndiff --git a/t/helper/test-i18n.c b/t/helper/test-i18n.c\nnew file mode 100644\nindex 00000000000..4b572e6efad\n--- /dev/null\n+++ b/t/helper/test-i18n.c\n@@ -0,0 +1,89 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+\n+static const char *usage_msg = \"\\n\"\n+\"  test-tool i18n cmp <file1> <file2>\\n\";\n+\n+static inline char do_rot13(char c)\n+{\n+\tif (c >= 'a' && c <= 'm')\n+\t\treturn c + 'n' - 'a';\n+\tif (c >= 'n' && c <= 'z')\n+\t\treturn c + 'a' - 'n';\n+\tif (c >= 'A' && c <= 'M')\n+\t\treturn c + 'N' - 'A';\n+\tif (c >= 'N' && c <= 'Z')\n+\t\treturn c + 'A' - 'N';\n+\treturn c;\n+}\n+\n+static size_t unrot13(char *buf)\n+{\n+\tchar *p = buf, *q = buf;\n+\n+\twhile (*p) {\n+\t\tconst char *begin = strstr(p, \"<rot13>\"), *end;\n+\n+\t\tif (!begin)\n+\t\t\tbreak;\n+\n+\t\twhile (p != begin)\n+\t\t\t*(q++) = *(p++);\n+\n+\t\tp += strlen(\"<rot13>\");\n+\t\tend = strstr(p, \"</rot13>\");\n+\t\tif (!end)\n+\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n+\n+\t\twhile (p != end)\n+\t\t\t*(q++) = do_rot13(*(p++));\n+\t\tp += strlen(\"</rot13>\");\n+\t}\n+\n+\twhile (*p)\n+\t\t*(q++) = *(p++);\n+\n+\treturn q - buf;\n+}\n+\n+static void unrot13_strbuf(struct strbuf *buf)\n+{\n+\tsize_t len = unrot13(buf->buf);\n+\n+\tif (len == buf->len)\n+\t\tdie(\"not ROT13'ed:\\n%s\", buf->buf);\n+\tbuf->len = len;\n+}\n+\n+static int i18n_cmp(const char **argv)\n+{\n+\tconst char *path1 = *(argv++), *path2 = *(argv++);\n+\tstruct strbuf a = STRBUF_INIT, b = STRBUF_INIT;\n+\n+\tif (!path1 || !path2 || *argv)\n+\t\tusage(usage_msg);\n+\n+\tif (strbuf_read_file(&a, path1, 0) < 0)\n+\t\tdie_errno(\"could not read %s\", path1);\n+\tif (strbuf_read_file(&b, path2, 0) < 0)\n+\t\tdie_errno(\"could not read %s\", path2);\n+\tunrot13_strbuf(&b);\n+\n+\tif (a.len != b.len || memcmp(a.buf, b.buf, a.len))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+int cmd__i18n(int argc, const char **argv)\n+{\n+\targv++;\n+\tif (!*argv)\n+\t\tusage(usage_msg);\n+\tif (!strcmp(*argv, \"cmp\"))\n+\t\treturn i18n_cmp(argv+1);\n+\telse\n+\t\tusage(usage_msg);\n+\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 9d6d14d9293..7e1680ac108 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -34,6 +34,7 @@ static struct test_cmd cmds[] = {\n \t{ \"genzeros\", cmd__genzeros },\n \t{ \"hashmap\", cmd__hashmap },\n \t{ \"hash-speed\", cmd__hash_speed },\n+\t{ \"i18n\", cmd__i18n },\n \t{ \"index-version\", cmd__index_version },\n \t{ \"json-writer\", cmd__json_writer },\n \t{ \"lazy-init-name-hash\", cmd__lazy_init_name_hash },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex a6470ff62c4..43282a520ea 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -24,6 +24,7 @@ int cmd__genrandom(int argc, const char **argv);\n int cmd__genzeros(int argc, const char **argv);\n int cmd__hashmap(int argc, const char **argv);\n int cmd__hash_speed(int argc, const char **argv);\n+int cmd__i18n(int argc, const char **argv);\n int cmd__index_version(int argc, const char **argv);\n int cmd__json_writer(int argc, const char **argv);\n int cmd__lazy_init_name_hash(int argc, const char **argv);\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 999982fe4a9..08731bae854 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -993,7 +993,13 @@ test_cmp_bin () {\n # under GIT_TEST_GETTEXT_POISON this pretends that the command produced expected\n # results.\n test_i18ncmp () {\n-\t! test_have_prereq C_LOCALE_OUTPUT || test_cmp \"$@\"\n+\tif test rot13 = \"$GIT_TEST_GETTEXT_POISON\"\n+\tthen\n+\t\ttest-tool i18n cmp \"$@\"\n+\telif test_have_prereq C_LOCALE_OUTPUT\n+\tthen\n+\t\ttest_cmp \"$@\"\n+\tfi\n }\n \n # Use this instead of \"grep expected-string actual\" to see if the\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9fa7c1d0f6d..c9f9e2804fd 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1537,6 +1537,7 @@ then\n fi\n \n test_lazy_prereq C_LOCALE_OUTPUT '\n+\ttest rot13 != \"$GIT_TEST_GETTEXT_POISON\" &&\n \t! test_bool_env GIT_TEST_GETTEXT_POISON false\n '\n \n-- \ngitgitgadget\n\n"},{"id":"414112","messageId":"e1e9d5c27377cad3009e325bc353271aac31c770.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 11/11] test-tool i18n: do not complain about empty messages","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:42Z","receivedAt":"2021-01-12T08:49:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThey might, or might not be 'translated'. Let's not assume that they\naren't marked with `_()` for translation.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-i18n.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-i18n.c b/t/helper/test-i18n.c\nindex 8a16df6b40f..82efc66f1f5 100644\n--- a/t/helper/test-i18n.c\n+++ b/t/helper/test-i18n.c\n@@ -52,7 +52,7 @@ static void unrot13_strbuf(struct strbuf *buf)\n {\n \tsize_t len = unrot13(buf->buf);\n \n-\tif (len == buf->len)\n+\tif (len && len == buf->len)\n \t\tdie(\"not ROT13'ed:\\n%s\", buf->buf);\n \tbuf->len = len;\n }\n-- \ngitgitgadget\n"},{"id":"414113","messageId":"8f2c08474a75793c24af7d4ae44d73d2b23920bc.1610441263.git.gitgitgadget@gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"[PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-12T08:47:35Z","receivedAt":"2021-01-12T08:49:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n sha1dc_git.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sha1dc_git.c b/sha1dc_git.c\nindex 5c300e812e0..fecf5da1483 100644\n--- a/sha1dc_git.c\n+++ b/sha1dc_git.c\n@@ -18,7 +18,7 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n {\n \tif (!SHA1DCFinal(hash, ctx))\n \t\treturn;\n-\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n+\tdie(_(\"SHA-1 appears to be part of a collision attack: %s\"),\n \t    hash_to_hex_algop(hash, &hash_algos[GIT_HASH_SHA1]));\n }\n \n-- \ngitgitgadget\n\n"},{"id":"414121","messageId":"20210112090708.GW8396@szeder.dev","threadId":"54977","inReplyTo":"004f90026031cb7ce71689481fabd27aa63485dd.1610441263.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/11] tests: remove unnecessary GIT_TEST_GETTEXT_POISON=false constructs","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-12T09:07:08Z","receivedAt":"2021-01-12T09:07:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jan 12, 2021 at 08:47:32AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> The idea of the `GETTEXT_POISON` mode is to test translated messages, at\n> least _somewhat_.\n> \n> There is not really any point in turning off that mode by force, except\n> _maybe_ to test the mode itself.\n> \n> So let's avoid overriding `GIT_TEST_GETTEXT_POISON` in the test suite\n> unless testing the `GETTEXT_POISON` functionality itself.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index a1c4f1f6d40..e5adee27d41 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -2363,7 +2363,6 @@ test_expect_success 'sourcing the completion script clears cached commands' '\n>  '\n>  \n>  test_expect_success 'sourcing the completion script clears cached merge strategies' '\n> -\tGIT_TEST_GETTEXT_POISON=false &&\n\nI think this change caused the failure in t9902 that you mentioned in\nthe cover letter.  \n\nIf 'git merge' is invoked with a nonexisting merge strategy, then it\nerrors out with an error message that contains a list of available\nmerge strategies.  The completion script relies on this behavior and\n\"parses\" this error message to get the available merge strategies, but\nit breaks when it can't find the expected text because it was\npoisoned.\n\n>  \t__git_compute_merge_strategies &&\n>  \tverbose test -n \"$__git_merge_strategies\" &&\n>  \t. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" &&\n> -- \n> gitgitgadget\n> \n"},{"id":"414131","messageId":"X/2JTsoUj3nAQcMi@coredump.intra.peff.net","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"Re: [PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-01-12T11:34:38Z","receivedAt":"2021-01-12T11:35:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 12, 2021 at 08:47:31AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> Ævar suggested recently\n> [https://lore.kernel.org/git/xmqqim836v6m.fsf@gitster.c.googlers.com/T/#m6fdc43d4f1eb3f20f841096c59e985b69c84875e]\n> that this whole GETTEXT_POISON business is totally useless.\n> \n> I do not believe that it is useless. To back up my belief with something\n> tangible, I implemented a GETTEXT_POISON=rot13 mode and ran the test suite\n> to see whether we erroneously expect translated messages where they aren't,\n> and related problems.\n> \n> And the experiment delivered. It is just a demonstration (I only addressed a\n> handful of the test suite failures, 124 test scripts still need to be\n> inspected to find out why they fail), of course. Yet I think that finding\n> e.g. the missing translations of sha1dc-cb and parse-options show that it\n> would be very useful to complete this patch series and then use the rot13\n> mode in our CI jobs (instead of the current GETTEXT_POISON=true mode, which\n> really is less useful).\n\nI'm not entirely convinced by this. The original point of the poison\ncode was not to find opportunities to translate strings, but to make\nsure we did not accidentally translate a string that some script was\nrelying on. And I don't see any fixes for the latter here (and as Ævar\nsuggested in the linked thread, the fact that we're not combing through\nexisting code looking for translations makes such an error a lot less\nlikely).\n\nWhich isn't to say repurposing it in the other direction might not be\nworthwhile. But I suspect a lot of the test failures are just false\npositives. Until now it was always reasonable to conservatively use\ntest_i18ngrep for cases which could reasonably translated, even if they\nwere not yet.\n\nLikewise, I'm not sure that one can reliably rot13 an output for\ntest_i18ncmp. It could contain mixed translated and untranslated bits\nfrom different messages.\n\nSo I dunno. You did find two spots where translations could be used.\nBut if nobody actually saw them to care that they got translated, were\nthey important? I may be a bit biased as somebody who would not use the\ntranslations in the first place, of course.\n\n-Peff\n"},{"id":"414132","messageId":"X/2J8KL/Jig/xttF@coredump.intra.peff.net","threadId":"54977","inReplyTo":"8f2c08474a75793c24af7d4ae44d73d2b23920bc.1610441263.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-01-12T11:37:20Z","receivedAt":"2021-01-12T11:38:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 12, 2021 at 08:47:35AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> diff --git a/sha1dc_git.c b/sha1dc_git.c\n> index 5c300e812e0..fecf5da1483 100644\n> --- a/sha1dc_git.c\n> +++ b/sha1dc_git.c\n> @@ -18,7 +18,7 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n>  {\n>  \tif (!SHA1DCFinal(hash, ctx))\n>  \t\treturn;\n> -\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n> +\tdie(_(\"SHA-1 appears to be part of a collision attack: %s\"),\n>  \t    hash_to_hex_algop(hash, &hash_algos[GIT_HASH_SHA1]));\n\nI didn't find any list discussion, but I think I may have actually left\nthis untranslated intentionally. Like a BUG(), we'd expect it to come up\nbasically never. And when it does, being able to search for the exact\nwording online may be more important than providing a translated\nversion.\n\nIt probably doesn't matter that much either way, though.\n\n-Peff\n"},{"id":"414150","messageId":"87mtxe5lhj.fsf@evledraar.gmail.com","threadId":"54977","inReplyTo":"pull.836.git.1610441262.gitgitgadget@gmail.com","subject":"Re: [PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-12T13:32:40Z","receivedAt":"2021-01-12T13:33:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jan 12 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> This patch series is incomplete because I lack the time to finish it, and I\n> hope that there are interested developers who can help with it.\n>\n> In https://lore.kernel.org/git/20181022153633.31757-1-pclouds@gmail.com/,\n> Duy proposed introducing a fake language they called \"Ook\" to be able to\n> test translations better. \"Ook\" would \"translate\" messages without any\n> printf format specifiers to \"Eek\", otherwise convert all upper-case letters\n> to \"Ook\" and lower-case letters to \"ook\".\n>\n> Gábor replied with a different proposal that has the advantage of being\n> bijective, i.e. it is possible to reconstruct the untranslated message from\n> the translated one.\n>\n> Ævar suggested recently\n> [https://lore.kernel.org/git/xmqqim836v6m.fsf@gitster.c.googlers.com/T/#m6fdc43d4f1eb3f20f841096c59e985b69c84875e]\n> that this whole GETTEXT_POISON business is totally useless.\n\nTo clarify what I really mean, but admittedly am not always the best at\narticulating: I do not think GETTEXT_POISON is useless, it's useful.\n\nThe interesting question is \"useful enough to bother?\", or in economics\nterms \"is this investment in time/money worth the opportunity cost?\".\n\nAny hoops we make people jump through when writing tests takes away time\nand attention from other things.\n\nMy motivation here is that I feel this whole poison business is all my\nfault. Every time some newbie submits a patch and needs to re-roll a v2\nbecause they used \"grep\" instead of \"test_i18ngrep\" in a case that\nclearly didn't involve a plumbing string I cringe a little about the\ntechnical debt.\n\nSo while I still think my series (just nuke it) would be better overall,\nI'm secretly rooting for yours. If the consensus is that this is worth\nkeeping and improving perhaps we haven't been mostly wasting our time :)\n\nOn to discussing this series:\n\n> I do not believe that it is useless. To back up my belief with something\n> tangible, I implemented a GETTEXT_POISON=rot13 mode and ran the test suite\n> to see whether we erroneously expect translated messages where they aren't,\n> and related problems.\n\nI agree that rot13ing it makes it much more useful in the sense that\nbefore we could over-tag things with test_i18n* and we'd just \"return 0\"\nthere in the poison mode, now we actually check if the string is\npoisoned.\n\nWe could get most of the way there by checking e.g. if \"GETTEXT POISON\"\nis in the output, but it wouldn't assert that the message is 100%\ntranslated as this WIP series does.\n\nIn our off-list discussion you maintained that\n\"GIT_TEST_GETTEXT_POISON=false <cmd>\" was an anti-pattern. Was it\nbecause you thought \"test_i18n*\" et al was actually doing some \"is\npoison?\" comparison as we now do, or just because you didn't\nconceptually like a pattern like:\n\n    GIT_TEST_GETTEXT_POISON=false git cmd >out &&\n    grep string out\n\nBeing changed later to:\n\n    GIT_TEST_GETTEXT_POISON=false git cmd >out &&\n    grep string out &&\n    grep plumbing-string out\n\nOr whatever, as opposed to:\n\n    git cmd >out &&\n    test_i18ngrep string out &&\n    test_i18ngrep plumbing-string out\n\nI get the aesthetic preference, I'm just wondering if I'm missing\nsomething else about it...\n\n> And the experiment delivered. It is just a demonstration (I only addressed a\n> handful of the test suite failures, 124 test scripts still need to be\n> inspected to find out why they fail), of course. Yet I think that finding\n> e.g. the missing translations of sha1dc-cb and parse-options show that it\n> would be very useful to complete this patch series and then use the rot13\n> mode in our CI jobs (instead of the current GETTEXT_POISON=true mode, which\n> really is less useful).\n\nThe following patch on top cuts down on the failures by more than half:\n\n-----------------\ndiff --git a/git-sh-i18n.sh b/git-sh-i18n.sh\nindex 8eef60b43f..f9239d2917 100644\n--- a/git-sh-i18n.sh\n+++ b/git-sh-i18n.sh\n@@ -17,7 +17,10 @@ export TEXTDOMAINDIR\n \n # First decide what scheme to use...\n GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough\n-if test -n \"$GIT_TEST_GETTEXT_POISON\" &&\n+if test -n \"$GIT_TEST_GETTEXT_POISON\" -a \"$GIT_TEST_GETTEXT_POISON\" = \"rot13\"\n+then\n+\tGIT_INTERNAL_GETTEXT_SH_SCHEME=rot13poison\n+elif  test -n \"$GIT_TEST_GETTEXT_POISON\" &&\n \t    git env--helper --type=bool --default=0 --exit-code \\\n \t\tGIT_TEST_GETTEXT_POISON\n then\n@@ -63,6 +66,21 @@ gettext_without_eval_gettext)\n \t\t)\n \t}\n \t;;\n+rot13poison)\n+\t# Emit garbage so that tests that incorrectly rely on translatable\n+\t# strings will fail.\n+\tgettext () {\n+\t\tprintf \"%s\" \"# SHELL GETTEXT POISON #\"\n+\t}\n+\n+\teval_gettext () {\n+\t\tprintf \"%s\" \"# SHELL GETTEXT POISON #\"\n+\t}\n+\n+\teval_ngettext () {\n+\t\tprintf \"%s\" \"# SHELL GETTEXT POISON #\"\n+\t}\n+\t;;\n poison)\n \t# Emit garbage so that tests that incorrectly rely on translatable\n \t# strings will fail.\ndiff --git a/t/helper/test-i18n.c b/t/helper/test-i18n.c\nindex 82efc66f1f..1f0fa3d041 100644\n--- a/t/helper/test-i18n.c\n+++ b/t/helper/test-i18n.c\n@@ -1,6 +1,8 @@\n #include \"test-tool.h\"\n #include \"cache.h\"\n #include \"grep.h\"\n+#include \"config.h\"\n \n static const char *usage_msg = \"\\n\"\n \"  test-tool i18n cmp <file1> <file2>\\n\"\n@@ -34,8 +36,12 @@ static size_t unrot13(char *buf)\n \n \t\tp += strlen(\"<rot13>\");\n \t\tend = strstr(p, \"</rot13>\");\n-\t\tif (!end)\n-\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n+\t\tif (!end) {\n+\t\t\tif (git_env_bool(\"GIT_TEST_GETTEXT_POISON_PEDANTIC\", 0))\n+\t\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n+\t\t\telse\n+\t\t\t\tbreak;\n+\t\t}\n \n \t\twhile (p != end)\n \t\t\t*(q++) = do_rot13(*(p++));\n@@ -67,8 +73,12 @@ static int i18n_cmp(const char **argv)\n \n \tif (strbuf_read_file(&a, path1, 0) < 0)\n \t\tdie_errno(\"could not read %s\", path1);\n+\tif (strstr(a.buf, \"# SHELL GETTEXT POISON #\"))\n+\t\treturn 0;\n \tif (strbuf_read_file(&b, path2, 0) < 0)\n \t\tdie_errno(\"could not read %s\", path2);\n+\tif (strstr(b.buf, \"# SHELL GETTEXT POISON #\"))\n+\t\treturn 0;\n \tunrot13_strbuf(&b);\n \n \tif (a.len != b.len || memcmp(a.buf, b.buf, a.len))\n@@ -79,7 +89,6 @@ static int i18n_cmp(const char **argv)\n \n static int i18n_grep(const char **argv)\n {\n-\tint dont_match = 0;\n \tconst char *pattern, *path;\n \n \tstruct grep_opt opt;\n@@ -87,11 +96,6 @@ static int i18n_grep(const char **argv)\n \tstruct strbuf buf = STRBUF_INIT;\n \tint hit;\n \n-\tif (*argv && !strcmp(\"!\", *argv)) {\n-\t\tdont_match = 1;\n-\t\targv++;\n-\t}\n-\n \tpattern = *(argv++);\n \tpath = *(argv++);\n \n@@ -104,13 +108,15 @@ static int i18n_grep(const char **argv)\n \n \tif (strbuf_read_file(&buf, path, 0) < 0)\n \t\tdie_errno(\"could not read %s\", path);\n+\tif (strstr(buf.buf, \"# SHELL GETTEXT POISON #\"))\n+\t\treturn 0;\n \tunrot13_strbuf(&buf);\n \tgrep_source_init(&source, GREP_SOURCE_BUF, path, path, path);\n \tsource.buf = buf.buf;\n \tsource.size = buf.len;\n \thit = grep_source(&opt, &source);\n \tstrbuf_release(&buf);\n-\treturn dont_match ^ !hit;\n+\treturn !hit;\n }\n \n int cmd__i18n(int argc, const char **argv)\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 394fd2ef5b..6c580a3000 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1019,14 +1019,12 @@ test_i18ngrep () {\n \t\tBUG \"too few parameters to test_i18ngrep\"\n \tfi\n \n-\tif test_have_prereq !C_LOCALE_OUTPUT\n+\tgrep_cmd=grep\n+\tif test \"$GIT_TEST_GETTEXT_POISON\" == \"rot13\"\n+\tthen\n+\t\tgrep_cmd=\"test-tool i18n grep\"\n+\telif test_have_prereq !C_LOCALE_OUTPUT\n \tthen\n-\t\tif test rot13 = \"$GIT_TEST_GETTEXT_POISON\"\n-\t\tthen\n-\t\t\ttest-tool i18n grep \"$@\"\n-\t\t\treturn $?\n-\t\tfi\n-\n \t\t# pretend success\n \t\treturn 0\n \tfi\n@@ -1034,13 +1032,13 @@ test_i18ngrep () {\n \tif test \"x!\" = \"x$1\"\n \tthen\n \t\tshift\n-\t\t! grep \"$@\" && return 0\n+\t\t! $grep_cmd \"$@\" && return 0\n \n-\t\techo >&4 \"error: '! grep $@' did find a match in:\"\n+\t\techo >&4 \"error: '! $grep_cmd $@' did find a match in:\"\n \telse\n-\t\tgrep \"$@\" && return 0\n+\t\t$grep_cmd \"$@\" && return 0\n \n-\t\techo >&4 \"error: 'grep $@' didn't find a match in:\"\n+\t\techo >&4 \"error: '$grep_cmd $@' didn't find a match in:\"\n \tfi\n \n \tif test -s \"$last_arg\"\n-----------------\n\nI.e. most of this was because you didn't add any support for this to the\nshellscript translations.\n\nWe still punt on that here, it should ideally preserve the shell\nvariables like the format specifiers, but I think that's not worth the\neffort with us actively cutting down our shell code to nothing.\n\nWe still have failures e.g. due to \"test_i18ngrep -F fixed file\" not\nbeing supported. Wouldn't a better/simpler approach be to just have a\nlight rot13 helper, and then call the actual \"cmp\"/\"grep\" with the\nmunged input/output?\n"},{"id":"414194","messageId":"xmqqmtxe3q0u.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"ab7832fc7ac783bc8f8c10014fc9747128fc37bc.1610441263.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/11] Support GIT_TEST_GETTEXT_POISON=rot13","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T19:37:37Z","receivedAt":"2021-01-12T19:38:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> +const char *gettext_maybe_rot13(const char *msgid)\n> +{\n> +...\n> +\twhile (*msgid) {\n> +\t\tconst char *p = strchrnul(msgid, '%'), *spec;\n> +\n> +\t\twhile (*p && p[1] == '%')\n> +\t\t\tp = strchrnul(p + 2, '%');\n\nWe are at '%', and while the next is '%' (i.e. a literal '%' is\nasked), we skip these two bytes and then we go back to the\nequivalent of the initialization of 'p'.  Effects?  We leave the\nloop when we are at the end of the string, or at '%' that is not\nfollowed by a '%'.\n\nIt would have been easier to understand what is going on in the loop\nif there weren't duplicated strchrnul() call, perhaps\n\n\tconst char *p = msgid;\n\twhile (*(p = strchrnul(p, '%')) == '%' && p[1] == '%')\n        \tp += 2;\n\nThat reads \"find '%' and if the next char is also '%', skip these\ntwo and redo the loop\", which would be equivalent.  And it would be\nmuch easier to follow the logic.\n\n> +\t\tif (p != msgid) {\n\nAnd if 'p' is different (actually we know p is the same as, or\nalways ahead of, msgid, so \"if (msgid < p)\" would be easier to\nfollow for readers) ...\n\n> +\t\t\tstrbuf_addstr(buf, \"<rot13>\");\n> +\t\t\twhile (p != msgid)\n\nDitto.\n\n> +\t\t\t\tstrbuf_addch(buf, do_rot13(*(msgid++)));\n> +\t\t\tstrbuf_addstr(buf, \"</rot13>\");\n\n... we add a section of obfuscated output enclosed with an xml\nlooking marker.\n\n> +\t\t}\n> +\n> +\t\tif (!*p)\n> +\t\t\tbreak;\n\nAnd if we are at the end, we are done.\n\n> +\t\tspec = strpbrk(p + 1, \"diouxXeEfFgGaAcsCSpnm%\");\n> +\t\tif (!spec)\n> +\t\t\tBUG(\"Unrecognized format string: %s\", p);\n\nIt is a bit surprising that things like \"%.*d\" and \"%.7f\" do not\nappear in our translatable strings (or perhaps this is one of the\nplaces why the series is marked RFH and is not complete?).\n\n> +\t\tstrbuf_add(buf, p, spec + 1 - p);\n> +\t\tmsgid = spec + 1;\n> +\t}\n> +\n> +\treturn buf->buf;\n> +}\n\nAnyway, thanks for a fun reading.\n\n"},{"id":"414195","messageId":"xmqqim823pwa.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"X/2J8KL/Jig/xttF@coredump.intra.peff.net","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T19:40:21Z","receivedAt":"2021-01-12T19:41:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Jan 12, 2021 at 08:47:35AM +0000, Johannes Schindelin via GitGitGadget wrote:\n>\n>> diff --git a/sha1dc_git.c b/sha1dc_git.c\n>> index 5c300e812e0..fecf5da1483 100644\n>> --- a/sha1dc_git.c\n>> +++ b/sha1dc_git.c\n>> @@ -18,7 +18,7 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n>>  {\n>>  \tif (!SHA1DCFinal(hash, ctx))\n>>  \t\treturn;\n>> -\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n>> +\tdie(_(\"SHA-1 appears to be part of a collision attack: %s\"),\n>>  \t    hash_to_hex_algop(hash, &hash_algos[GIT_HASH_SHA1]));\n>\n> I didn't find any list discussion, but I think I may have actually left\n> this untranslated intentionally. Like a BUG(), we'd expect it to come up\n> basically never. And when it does, being able to search for the exact\n> wording online may be more important than providing a translated\n> version.\n>\n> It probably doesn't matter that much either way, though.\n\nI can believe if this was intentionally left untranslated for the\nreasons you stated.  Without introducing \"message ID\" that can be\nuniversally readable, so that customers can report \"We got E42234\nwhen running Git version 1.2.3.4\", that is the best we could do.\n\nBut I am OK to see this translated, too.\n\nThanks.\n"},{"id":"414196","messageId":"xmqqeeiq3pkf.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"4669ccbb1ae40f0c58a2d8e3c8b3a34d82176c7a.1610441263.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T19:47:28Z","receivedAt":"2021-01-12T19:48:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> +static size_t unrot13(char *buf)\n> +{\n> +\tchar *p = buf, *q = buf;\n> +\n> +\twhile (*p) {\n> +\t\tconst char *begin = strstr(p, \"<rot13>\"), *end;\n\nAFAIR from my reading of [02/11], the encoding side did not special\ncase the payload that has <ebg13> or </ebg13>; if we want to make it\nreversible conversion (which is excellent improvement over the\ncurrent \"# GETTEXT_POISON #\" obfuscation), we'd need to do something\nabout it, I think.\n\nBut on second thought, nobody can prevent a caller to die(_(\"%s\", msg));\nto have \"<rot13>\" in the msg part, so perhaps punting like this\nseries does is sufficient.  I dunno.\n\n> +\t\tif (!begin)\n> +\t\t\tbreak;\n> +\n> +\t\twhile (p != begin)\n> +\t\t\t*(q++) = *(p++);\n> +\n> +\t\tp += strlen(\"<rot13>\");\n> +\t\tend = strstr(p, \"</rot13>\");\n> +\t\tif (!end)\n> +\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n\nAnd the user of this looks quite straightforward and nice.\n\n>  test_i18ncmp () {\n> -\t! test_have_prereq C_LOCALE_OUTPUT || test_cmp \"$@\"\n> +\tif test rot13 = \"$GIT_TEST_GETTEXT_POISON\"\n> +\tthen\n> +\t\ttest-tool i18n cmp \"$@\"\n> +\telif test_have_prereq C_LOCALE_OUTPUT\n> +\tthen\n> +\t\ttest_cmp \"$@\"\n> +\tfi\n>  }\n\n"},{"id":"414198","messageId":"xmqqa6te3per.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"X/2JTsoUj3nAQcMi@coredump.intra.peff.net","subject":"Re: [PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T19:50:52Z","receivedAt":"2021-01-12T19:51:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Likewise, I'm not sure that one can reliably rot13 an output for\n> test_i18ncmp. It could contain mixed translated and untranslated bits\n> from different messages.\n\nAs long as <rot13>...</rot13> marking can be trusted (and it cannot\nbe, but if we declare it is OK to punt, then by definition we can),\nI think the messages can be compared.\n\n> So I dunno. You did find two spots where translations could be used.\n> But if nobody actually saw them to care that they got translated, were\n> they important? I may be a bit biased as somebody who would not use the\n> translations in the first place, of course.\n\nI viewed the series with interest, mostly (i.e. 80%+) for its \"fun\nvalue\"; I tend to agree with you that I doubt its usefulness.\n"},{"id":"414244","messageId":"X/6h+8UC6N1MfZ0Q@coredump.intra.peff.net","threadId":"54977","inReplyTo":"xmqqa6te3per.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-01-13T07:32:11Z","receivedAt":"2021-01-13T07:33:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 12, 2021 at 11:50:52AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Likewise, I'm not sure that one can reliably rot13 an output for\n> > test_i18ncmp. It could contain mixed translated and untranslated bits\n> > from different messages.\n> \n> As long as <rot13>...</rot13> marking can be trusted (and it cannot\n> be, but if we declare it is OK to punt, then by definition we can),\n> I think the messages can be compared.\n\nGood point. I should have looked at the implementation more carefully\nbefore responding (and I agree that while not foolproof, tagging like\nthat would work OK in practice).\n\n-Peff\n"},{"id":"414453","messageId":"nycvar.QRO.7.76.6.2101151639030.52@tvgsbejvaqbjf.bet","threadId":"54977","inReplyTo":"X/2J8KL/Jig/xttF@coredump.intra.peff.net","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-15T15:43:05Z","receivedAt":"2021-01-15T15:45:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Tue, 12 Jan 2021, Jeff King wrote:\n\n> On Tue, Jan 12, 2021 at 08:47:35AM +0000, Johannes Schindelin via GitGitGadget wrote:\n>\n> > diff --git a/sha1dc_git.c b/sha1dc_git.c\n> > index 5c300e812e0..fecf5da1483 100644\n> > --- a/sha1dc_git.c\n> > +++ b/sha1dc_git.c\n> > @@ -18,7 +18,7 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n> >  {\n> >  \tif (!SHA1DCFinal(hash, ctx))\n> >  \t\treturn;\n> > -\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n> > +\tdie(_(\"SHA-1 appears to be part of a collision attack: %s\"),\n> >  \t    hash_to_hex_algop(hash, &hash_algos[GIT_HASH_SHA1]));\n>\n> I didn't find any list discussion, but I think I may have actually left\n> this untranslated intentionally. Like a BUG(), we'd expect it to come up\n> basically never. And when it does, being able to search for the exact\n> wording online may be more important than providing a translated\n> version.\n\nI disagree with that reasoning. By that rationale, any message we deem to\nbe somewhat rare should be _untranslated_.\n\nA much better rule, at least from my perspective is: is the target\naudience the Git users? If so, the message is to be translated. If not,\nthen not.\n\nIn this instance, it is quite obviously targeting the Git users who need\nto understand why the command they tried to run was failing. The test in\nt0013 is totally agreeing with this:\n\n\ttest_expect_success 'test-sha1 detects shattered pdf' '\n\t\ttest_must_fail test-tool sha1 <\"$TEST_DATA/shattered-1.pdf\" 2>err &&\n\t\ttest_i18ngrep collision err &&\n\t\tgrep 38762cf7f55934b34d179ae6a4c80cadccbb7f0a err\n\t'\n\nNotice that `test_i18ngrep`? It tells me that we expect this message to be\ntranslated.\n\nCiao,\nDscho\n"},{"id":"414454","messageId":"nycvar.QRO.7.76.6.2101151643410.52@tvgsbejvaqbjf.bet","threadId":"54977","inReplyTo":"xmqqeeiq3pkf.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-15T15:44:18Z","receivedAt":"2021-01-15T15:46:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 12 Jan 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > +static size_t unrot13(char *buf)\n> > +{\n> > +\tchar *p = buf, *q = buf;\n> > +\n> > +\twhile (*p) {\n> > +\t\tconst char *begin = strstr(p, \"<rot13>\"), *end;\n>\n> AFAIR from my reading of [02/11], the encoding side did not special\n> case the payload that has <ebg13> or </ebg13>; if we want to make it\n> reversible conversion (which is excellent improvement over the\n> current \"# GETTEXT_POISON #\" obfuscation), we'd need to do something\n> about it, I think.\n\nDo you expect any message to be translated _twice_?\n\nCiao,\nDscho\n\n> But on second thought, nobody can prevent a caller to die(_(\"%s\", msg));\n> to have \"<rot13>\" in the msg part, so perhaps punting like this\n> series does is sufficient.  I dunno.\n>\n> > +\t\tif (!begin)\n> > +\t\t\tbreak;\n> > +\n> > +\t\twhile (p != begin)\n> > +\t\t\t*(q++) = *(p++);\n> > +\n> > +\t\tp += strlen(\"<rot13>\");\n> > +\t\tend = strstr(p, \"</rot13>\");\n> > +\t\tif (!end)\n> > +\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n>\n> And the user of this looks quite straightforward and nice.\n>\n> >  test_i18ncmp () {\n> > -\t! test_have_prereq C_LOCALE_OUTPUT || test_cmp \"$@\"\n> > +\tif test rot13 = \"$GIT_TEST_GETTEXT_POISON\"\n> > +\tthen\n> > +\t\ttest-tool i18n cmp \"$@\"\n> > +\telif test_have_prereq C_LOCALE_OUTPUT\n> > +\tthen\n> > +\t\ttest_cmp \"$@\"\n> > +\tfi\n> >  }\n>\n>\n"},{"id":"414455","messageId":"nycvar.QRO.7.76.6.2101151648230.52@tvgsbejvaqbjf.bet","threadId":"54977","inReplyTo":"87mtxe5lhj.fsf@evledraar.gmail.com","subject":"Re: [PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-15T16:13:25Z","receivedAt":"2021-01-15T16:15:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\n\nOn Tue, 12 Jan 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> On Tue, Jan 12 2021, Johannes Schindelin via GitGitGadget wrote:\n>\n> > I implemented a GETTEXT_POISON=rot13 mode and ran the test suite to\n> > see whether we erroneously expect translated messages where they\n> > aren't, and related problems.\n>\n> I agree that rot13ing it makes it much more useful in the sense that\n> before we could over-tag things with test_i18n* and we'd just \"return 0\"\n> there in the poison mode, now we actually check if the string is\n> poisoned.\n\nPrecisely.\n\n> In our off-list discussion you maintained that\n> \"GIT_TEST_GETTEXT_POISON=false <cmd>\" was an anti-pattern. Was it\n> because you thought \"test_i18n*\" et al was actually doing some \"is\n> poison?\" [...]\n\nI maintain this view because the whole point of having a GETTEXT_POISON\njob is to have the entire test suite run in that mode. Using this\nanti-pattern says to me \"the author could not make this work correctly in\nGETTEXT_POISON mode\".\n\nEven more, _iff_ we expect certain validations to be skipped in the\nGETTEXT_POISON job, using `GIT_TEST_GETTEXT_POISON=false` opens the\npossibility that this job fails when it never should have failed.\n\n> > And the experiment delivered. It is just a demonstration (I only addressed a\n> > handful of the test suite failures, 124 test scripts still need to be\n> > inspected to find out why they fail), of course. Yet I think that finding\n> > e.g. the missing translations of sha1dc-cb and parse-options show that it\n> > would be very useful to complete this patch series and then use the rot13\n> > mode in our CI jobs (instead of the current GETTEXT_POISON=true mode, which\n> > really is less useful).\n>\n> The following patch on top cuts down on the failures by more than half:\n\nInteresting! I forgot the shell half... Curious that the scripted parts\n_still_ affect so much of our test suite :-(\n\n> -----------------\n> diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh\n> index 8eef60b43f..f9239d2917 100644\n> --- a/git-sh-i18n.sh\n> +++ b/git-sh-i18n.sh\n> @@ -17,7 +17,10 @@ export TEXTDOMAINDIR\n>\n>  # First decide what scheme to use...\n>  GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough\n> -if test -n \"$GIT_TEST_GETTEXT_POISON\" &&\n> +if test -n \"$GIT_TEST_GETTEXT_POISON\" -a \"$GIT_TEST_GETTEXT_POISON\" = \"rot13\"\n> +then\n> +\tGIT_INTERNAL_GETTEXT_SH_SCHEME=rot13poison\n> +elif  test -n \"$GIT_TEST_GETTEXT_POISON\" &&\n>  \t    git env--helper --type=bool --default=0 --exit-code \\\n>  \t\tGIT_TEST_GETTEXT_POISON\n>  then\n> @@ -63,6 +66,21 @@ gettext_without_eval_gettext)\n>  \t\t)\n>  \t}\n>  \t;;\n> +rot13poison)\n> +\t# Emit garbage so that tests that incorrectly rely on translatable\n> +\t# strings will fail.\n> +\tgettext () {\n> +\t\tprintf \"%s\" \"# SHELL GETTEXT POISON #\"\n> +\t}\n> +\n> +\teval_gettext () {\n> +\t\tprintf \"%s\" \"# SHELL GETTEXT POISON #\"\n> +\t}\n> +\n> +\teval_ngettext () {\n> +\t\tprintf \"%s\" \"# SHELL GETTEXT POISON #\"\n> +\t}\n\nRight, and for that, I'd rather use the `test-tool i18n` helper (it is\nappropriate to expect `test-tool` to be in the `PATH` in GETTEXT_POISON\nmode, right?)\n\n> +\t;;\n>  poison)\n>  \t# Emit garbage so that tests that incorrectly rely on translatable\n>  \t# strings will fail.\n> diff --git a/t/helper/test-i18n.c b/t/helper/test-i18n.c\n> index 82efc66f1f..1f0fa3d041 100644\n> --- a/t/helper/test-i18n.c\n> +++ b/t/helper/test-i18n.c\n> @@ -1,6 +1,8 @@\n>  #include \"test-tool.h\"\n>  #include \"cache.h\"\n>  #include \"grep.h\"\n> +#include \"config.h\"\n>\n>  static const char *usage_msg = \"\\n\"\n>  \"  test-tool i18n cmp <file1> <file2>\\n\"\n> @@ -34,8 +36,12 @@ static size_t unrot13(char *buf)\n>\n>  \t\tp += strlen(\"<rot13>\");\n>  \t\tend = strstr(p, \"</rot13>\");\n> -\t\tif (!end)\n> -\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n> +\t\tif (!end) {\n> +\t\t\tif (git_env_bool(\"GIT_TEST_GETTEXT_POISON_PEDANTIC\", 0))\n> +\t\t\t\tBUG(\"could not find </rot13> in\\n%s\", buf);\n> +\t\t\telse\n> +\t\t\t\tbreak;\n> +\t\t}\n>\n>  \t\twhile (p != end)\n>  \t\t\t*(q++) = do_rot13(*(p++));\n> @@ -67,8 +73,12 @@ static int i18n_cmp(const char **argv)\n>\n>  \tif (strbuf_read_file(&a, path1, 0) < 0)\n>  \t\tdie_errno(\"could not read %s\", path1);\n> +\tif (strstr(a.buf, \"# SHELL GETTEXT POISON #\"))\n> +\t\treturn 0;\n>  \tif (strbuf_read_file(&b, path2, 0) < 0)\n>  \t\tdie_errno(\"could not read %s\", path2);\n> +\tif (strstr(b.buf, \"# SHELL GETTEXT POISON #\"))\n> +\t\treturn 0;\n>  \tunrot13_strbuf(&b);\n>\n>  \tif (a.len != b.len || memcmp(a.buf, b.buf, a.len))\n> @@ -79,7 +89,6 @@ static int i18n_cmp(const char **argv)\n>\n>  static int i18n_grep(const char **argv)\n>  {\n> -\tint dont_match = 0;\n>  \tconst char *pattern, *path;\n>\n>  \tstruct grep_opt opt;\n> @@ -87,11 +96,6 @@ static int i18n_grep(const char **argv)\n>  \tstruct strbuf buf = STRBUF_INIT;\n>  \tint hit;\n>\n> -\tif (*argv && !strcmp(\"!\", *argv)) {\n> -\t\tdont_match = 1;\n> -\t\targv++;\n> -\t}\n> -\n>  \tpattern = *(argv++);\n>  \tpath = *(argv++);\n>\n> @@ -104,13 +108,15 @@ static int i18n_grep(const char **argv)\n>\n>  \tif (strbuf_read_file(&buf, path, 0) < 0)\n>  \t\tdie_errno(\"could not read %s\", path);\n> +\tif (strstr(buf.buf, \"# SHELL GETTEXT POISON #\"))\n> +\t\treturn 0;\n>  \tunrot13_strbuf(&buf);\n>  \tgrep_source_init(&source, GREP_SOURCE_BUF, path, path, path);\n>  \tsource.buf = buf.buf;\n>  \tsource.size = buf.len;\n>  \thit = grep_source(&opt, &source);\n>  \tstrbuf_release(&buf);\n> -\treturn dont_match ^ !hit;\n> +\treturn !hit;\n>  }\n>\n>  int cmd__i18n(int argc, const char **argv)\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 394fd2ef5b..6c580a3000 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -1019,14 +1019,12 @@ test_i18ngrep () {\n>  \t\tBUG \"too few parameters to test_i18ngrep\"\n>  \tfi\n>\n> -\tif test_have_prereq !C_LOCALE_OUTPUT\n> +\tgrep_cmd=grep\n> +\tif test \"$GIT_TEST_GETTEXT_POISON\" == \"rot13\"\n> +\tthen\n> +\t\tgrep_cmd=\"test-tool i18n grep\"\n> +\telif test_have_prereq !C_LOCALE_OUTPUT\n>  \tthen\n> -\t\tif test rot13 = \"$GIT_TEST_GETTEXT_POISON\"\n> -\t\tthen\n> -\t\t\ttest-tool i18n grep \"$@\"\n> -\t\t\treturn $?\n> -\t\tfi\n> -\n>  \t\t# pretend success\n>  \t\treturn 0\n>  \tfi\n> @@ -1034,13 +1032,13 @@ test_i18ngrep () {\n>  \tif test \"x!\" = \"x$1\"\n>  \tthen\n>  \t\tshift\n> -\t\t! grep \"$@\" && return 0\n> +\t\t! $grep_cmd \"$@\" && return 0\n>\n> -\t\techo >&4 \"error: '! grep $@' did find a match in:\"\n> +\t\techo >&4 \"error: '! $grep_cmd $@' did find a match in:\"\n>  \telse\n> -\t\tgrep \"$@\" && return 0\n> +\t\t$grep_cmd \"$@\" && return 0\n>\n> -\t\techo >&4 \"error: 'grep $@' didn't find a match in:\"\n> +\t\techo >&4 \"error: '$grep_cmd $@' didn't find a match in:\"\n>  \tfi\n>\n>  \tif test -s \"$last_arg\"\n> -----------------\n>\n> I.e. most of this was because you didn't add any support for this to the\n> shellscript translations.\n>\n> We still punt on that here, it should ideally preserve the shell\n> variables like the format specifiers, but I think that's not worth the\n> effort with us actively cutting down our shell code to nothing.\n\nI'm not sure how fast we'll get there, what with `git submodule` being\nslow to get over the finish line, and with `mergetool`/`difftool` still\nbeing scripted, and probably for a long time, too.\n\n> We still have failures e.g. due to \"test_i18ngrep -F fixed file\" not\n> being supported. Wouldn't a better/simpler approach be to just have a\n> light rot13 helper, and then call the actual \"cmp\"/\"grep\" with the\n> munged input/output?\n\nHmm. I hoped that this work would open the door to introducing more C\nhelpers in the test suite, thereby accelerating it on Windows.\n\nWe could also potentially move the `cmp`/`grep` parts into separate\nhelpers that give more useful output in case of a failure. (Right now, if\nanything fails in CI involving `grep` or `cmp`, the user investigating\nthis pretty much _has_ to try to reproduce the issue locally because the\noutput is so not helpful, and reproducing it might be a challenge e.g.\nwhen the failure is macOS-specific and the developer does not have access\nto a macOS setup).\n\nCiao,\nDscho\n"},{"id":"414457","messageId":"YAHC9faa4ykNOWj6@coredump.intra.peff.net","threadId":"54977","inReplyTo":"nycvar.QRO.7.76.6.2101151639030.52@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-01-15T16:29:41Z","receivedAt":"2021-01-15T16:30:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 15, 2021 at 04:43:05PM +0100, Johannes Schindelin wrote:\n\n> > > -\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n> > > +\tdie(_(\"SHA-1 appears to be part of a collision attack: %s\"),\n> > >  \t    hash_to_hex_algop(hash, &hash_algos[GIT_HASH_SHA1]));\n> >\n> > I didn't find any list discussion, but I think I may have actually left\n> > this untranslated intentionally. Like a BUG(), we'd expect it to come up\n> > basically never. And when it does, being able to search for the exact\n> > wording online may be more important than providing a translated\n> > version.\n> \n> I disagree with that reasoning. By that rationale, any message we deem to\n> be somewhat rare should be _untranslated_.\n> \n> A much better rule, at least from my perspective is: is the target\n> audience the Git users? If so, the message is to be translated. If not,\n> then not.\n\nThat's what I was getting at. The audience is really Git developers,\njust like it would be for a BUG(). We don't expect either of those\nthings to happen.\n\n> In this instance, it is quite obviously targeting the Git users who need\n> to understand why the command they tried to run was failing. The test in\n> t0013 is totally agreeing with this:\n> \n> \ttest_expect_success 'test-sha1 detects shattered pdf' '\n> \t\ttest_must_fail test-tool sha1 <\"$TEST_DATA/shattered-1.pdf\" 2>err &&\n> \t\ttest_i18ngrep collision err &&\n> \t\tgrep 38762cf7f55934b34d179ae6a4c80cadccbb7f0a err\n> \t'\n> \n> Notice that `test_i18ngrep`? It tells me that we expect this message to be\n> translated.\n\nWell, I wrote both that line and the untranslated original code, so I'm\nnot sure what we can deduce from that. ;)\n\nBut yeah, I am not strongly opposed to translating this. I brought it up\nmore in the line of \"I don't think it's that big a deal that it was not\ntranslated\".\n\n-Peff\n"},{"id":"414468","messageId":"xmqqlfcurn05.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"nycvar.QRO.7.76.6.2101151643410.52@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-15T19:58:02Z","receivedAt":"2021-01-15T19:58:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Tue, 12 Jan 2021, Junio C Hamano wrote:\n>\n>> > +static size_t unrot13(char *buf)\n>> > +{\n>> > +\tchar *p = buf, *q = buf;\n>> > +\n>> > +\twhile (*p) {\n>> > +\t\tconst char *begin = strstr(p, \"<rot13>\"), *end;\n>>\n>> AFAIR from my reading of [02/11], the encoding side did not special\n>> case the payload that has <ebg13> or </ebg13>; if we want to make it\n>> reversible conversion (which is excellent improvement over the\n>> current \"# GETTEXT_POISON #\" obfuscation), we'd need to do something\n>> about it, I think.\n>\n> Do you expect any message to be translated _twice_?\n\nNot at all.\n\nBut what I had in mind, when I wrote the above, was that the\nprogrammers are entitled to expect that they are allowed to say:\n\n\tdie(_(\"message with <ebg13/>, <ebg13> and <rot13> in it\"));\n\nThis will be rot13'd, and the entire thing will be enclosed inside\n\"<rot13>...</rot13>\"; I would expect that somewhere inside \"...\" the\nreceiving end that attempts to parse it by relying on these markers\nwill be confused.\n\n>> But on second thought, nobody can prevent a caller to die(_(\"%s\", msg));\n>> to have \"<rot13>\" in the msg part, so perhaps punting like this\n>> series does is sufficient.  I dunno.\n\nAnd this comment still stands.\n"},{"id":"414490","messageId":"xmqqv9bxpesr.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"xmqqa6te3per.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 00/11] [RFH] Introduce support for GETTEXT_POISON=rot13","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-16T06:38:12Z","receivedAt":"2021-01-16T06:39:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n> ...\n>> So I dunno. You did find two spots where translations could be used.\n>> But if nobody actually saw them to care that they got translated, were\n>> they important? I may be a bit biased as somebody who would not use the\n>> translations in the first place, of course.\n>\n> I viewed the series with interest, mostly (i.e. 80%+) for its \"fun\n> value\"; I tend to agree with you that I doubt its usefulness.\n\nActually, \"I doubt its usefulness\" is way too strong than what I\nreally wanted to say.  I was just skeptical, and I still somewhat\nam.  I'd just need a bit more convincing ;-)\n\n"},{"id":"414592","messageId":"nycvar.QRO.7.76.6.2101181525370.52@tvgsbejvaqbjf.bet","threadId":"54977","inReplyTo":"YAHC9faa4ykNOWj6@coredump.intra.peff.net","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-18T14:26:42Z","receivedAt":"2021-01-18T14:29:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 15 Jan 2021, Jeff King wrote:\n\n> On Fri, Jan 15, 2021 at 04:43:05PM +0100, Johannes Schindelin wrote:\n>\n> > > > -\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n> > > > +\tdie(_(\"SHA-1 appears to be part of a collision attack: %s\"),\n> > > >  \t    hash_to_hex_algop(hash, &hash_algos[GIT_HASH_SHA1]));\n> > >\n> > > I didn't find any list discussion, but I think I may have actually left\n> > > this untranslated intentionally. Like a BUG(), we'd expect it to come up\n> > > basically never. And when it does, being able to search for the exact\n> > > wording online may be more important than providing a translated\n> > > version.\n> >\n> > I disagree with that reasoning. By that rationale, any message we deem to\n> > be somewhat rare should be _untranslated_.\n> >\n> > A much better rule, at least from my perspective is: is the target\n> > audience the Git users? If so, the message is to be translated. If not,\n> > then not.\n>\n> That's what I was getting at. The audience is really Git developers,\n> just like it would be for a BUG(). We don't expect either of those\n> things to happen.\n\nWhile a SHA-1 collision might not be anything we expect to happen, I am\nfairly certain it won't be a bug in Git causing it. Nor will it be\nanything that core Git developers have to react on. For those reasons, I\ndisagree that core Git developers are the target audience of this message.\n\nCiao,\nDscho\n"},{"id":"414593","messageId":"nycvar.QRO.7.76.6.2101181526550.52@tvgsbejvaqbjf.bet","threadId":"54977","inReplyTo":"xmqqlfcurn05.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-18T14:36:32Z","receivedAt":"2021-01-18T14:38:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 15 Jan 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> > On Tue, 12 Jan 2021, Junio C Hamano wrote:\n> >\n> >> > +static size_t unrot13(char *buf)\n> >> > +{\n> >> > +\tchar *p = buf, *q = buf;\n> >> > +\n> >> > +\twhile (*p) {\n> >> > +\t\tconst char *begin = strstr(p, \"<rot13>\"), *end;\n> >>\n> >> AFAIR from my reading of [02/11], the encoding side did not special\n> >> case the payload that has <ebg13> or </ebg13>; if we want to make it\n> >> reversible conversion (which is excellent improvement over the\n> >> current \"# GETTEXT_POISON #\" obfuscation), we'd need to do something\n> >> about it, I think.\n> >\n> > Do you expect any message to be translated _twice_?\n>\n> Not at all.\n>\n> But what I had in mind, when I wrote the above, was that the\n> programmers are entitled to expect that they are allowed to say:\n>\n> \tdie(_(\"message with <ebg13/>, <ebg13> and <rot13> in it\"));\n>\n> This will be rot13'd, and the entire thing will be enclosed inside\n> \"<rot13>...</rot13>\"; I would expect that somewhere inside \"...\" the\n> receiving end that attempts to parse it by relying on these markers\n> will be confused.\n\nOh, _that_ is what you meant.\n\nYep, I don't expect any of Git's messages to contain \"<ebg13>\", ever.\n\n> >> But on second thought, nobody can prevent a caller to die(_(\"%s\", msg));\n> >> to have \"<rot13>\" in the msg part, so perhaps punting like this\n> >> series does is sufficient.  I dunno.\n>\n> And this comment still stands.\n\nSlightly pedantic: the call would be `die(_(\"%s\"), msg)` (note that the\n`msg` is not inside the `_(...)`).\n\nSimilar to my point above, I do not expect any of Git's tests to emit a\nmessage like that, and certainly not once/if we turn on rot13 mode in the\nGETTEXT_POISON job of the CI build.\n\nA blocker would have been if there might be the possibility that we _must_\nadd a test in the future that wants to emit a `msg` containing `<rot13>`.\nBut that seems utterly unlikely. And even if: then we'd just change the\nrot13 mode to use a different needle.\n\nCiao,\nDscho\n"},{"id":"414626","messageId":"xmqqim7uj6p5.fsf@gitster.c.googlers.com","threadId":"54977","inReplyTo":"nycvar.QRO.7.76.6.2101181525370.52@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-18T21:06:30Z","receivedAt":"2021-01-18T21:08:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> That's what I was getting at. The audience is really Git developers,\n>> just like it would be for a BUG(). We don't expect either of those\n>> things to happen.\n>\n> While a SHA-1 collision might not be anything we expect to happen, I am\n> fairly certain it won't be a bug in Git causing it. Nor will it be\n> anything that core Git developers have to react on. For those reasons, I\n> disagree that core Git developers are the target audience of this message.\n\nI do not know if this is what Peff meant by \"the audience is really\nGit developers\", but when any end-user encounters this message, we\nwant to learn about it a lot more urgently than all the ordinary\n\"there is no such command line option\", so in that sense, even it\nis not \"a bug in git\", it is more special than ordinary errors.\n\n"},{"id":"414685","messageId":"nycvar.QRO.7.76.6.2101191648400.52@tvgsbejvaqbjf.bet","threadId":"54977","inReplyTo":"xmqqim7uj6p5.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 04/11] sha1dc: mark forgotten message for translation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-19T15:52:26Z","receivedAt":"2021-01-19T15:55:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 18 Jan 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> >> That's what I was getting at. The audience is really Git developers,\n> >> just like it would be for a BUG(). We don't expect either of those\n> >> things to happen.\n> >\n> > While a SHA-1 collision might not be anything we expect to happen, I am\n> > fairly certain it won't be a bug in Git causing it. Nor will it be\n> > anything that core Git developers have to react on. For those reasons, I\n> > disagree that core Git developers are the target audience of this message.\n>\n> I do not know if this is what Peff meant by \"the audience is really\n> Git developers\", but when any end-user encounters this message, we\n> want to learn about it a lot more urgently than all the ordinary\n> \"there is no such command line option\", so in that sense, even it\n> is not \"a bug in git\", it is more special than ordinary errors.\n\nI suggest that we modify the message to state exactly that: \"Please\ncontact the Git mailing list at git@vger.kernel.org about this\". And then\nmark the message for translation, so that even Git users with low/no\nknowledge of the English language are in a position to help us.\n\nCiao,\nDscho\n\nP.S.: Yes, I realize that this means we could receive a message reporting\na SHA-1 collision written in, say, Chinese. Is this a problem? I don't\nthink so. In any case, this would still be much better than keeping the\nmessage untranslated and _not_ receiving a mail about it.\n"}]}