Re: [PATCH v6 0/5] Reroll patches against v1.8.3.1
On Tue, Jun 25, 2013 at 12:28:02PM -0700, Junio C Hamano wrote:
Alexey Shumkin alex.crez...@gmail.com writes:
4. [PATCH v6 4/5] pretty: Add failing tests: --format output should honor
logOutputEncoding
iso8859-5 encoding reverted back to cp1251 encoding (as it was in v4
series)
Thanks for a reroll, but why this change?
The reason I asked you to avoid 8859-5 is because our test do not
use that encoding and I do not want to add new dependency to people
when they run test. cp1251 shares exactly the same issue, doesn't
it? So in that sense, this change does not make anything better.
That is why I asked you if the breakage you are trying to
demonstrate about non-ASCII non-UTF8 encoding was specific to
Cyrillic/Russian. I do not recall seeing your answer, but what is
the right thing to do depend on it.
- If the answer is yes, then we would need to add dependency either
way, and 8859-5 is just as fine as cp1251.
- If the breakage is not specific to Cyrillic, it should be
reproducible using 8859-1 (latin-1), and our tests already depend
on 8859-1, so we wouldn't be adding new dependencies on people.
I suppose you've missed my answer somehow. It was:
---8---
Alexey Shumkin alex.crez...@gmail.com writes:
@@ -19,7 +23,8 @@ add_file () {
echo $name $name
git add $name
test_tick
- git commit -m Add $name || exit
+ msg_added_iso88595=$(echo Add $name ($added $name) |
iconv -f utf-8 -t iso88595)
+ git -c 'i18n.commitEncoding=iso88595' commit -m
$msg_added_iso88595
Hmph. Do we know 8859-5 is available or do these need to be
protected with prereq?
Opps, this encoding is absent even in my Cygwin box :)
Actually, previuosly, there was a Windows-1251 encoding,
you said we'd prefer to limit different encodings used in tests,
And I've made an attempt to avoid this but I'm way off the mark.
Can these tests be done with 8859-1 i.e. something we already depend
on, by changing that $added message to latin-1, or is there something
very Russian specific breakage we want to test here?
Well, actually, most popular Russian 8-bit codepages are Windows-1251 (aka
cp1251) and KOI8-R.
Cygwin (as a Linux box on Windows) uses cp1251.
Iconv cannot even convert Russian messages from cp1251(or UTF-8) to latin1.
It fails.
$ echo коммит | LANG= iconv -t latin1 -f UTF-8
iconv: illegal input sequence at position 0
(коммит is a commit in Russian)
If the former, please redo this entire patch (not just t4041) with
8859-1.
If there is some Russian specific breakage you cannot demonstrate
with 8859-1, then please explain it in the log message.
Well, it's not a Russian specific. It's codepage conversion specific,
but I cannot use latin1 codepage because I do not know any language that
uses iso8859-1/latin1 codepage (as German, Spanish, for example) to
write a commit message in it.
If someone can do the same with latin1, I'd be happy.
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index d32e65e..36e4cc0 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
...
-# usage: test_format name format_string expected_output
+# usage: test_format [failure] name format_string expected_output
test_format () {
+ local must_fail=0
This breaks tests for non-bash users. local is not even in POSIX.
---8---
But today I've taken a look to Cygwin's locales more closely and found
out that I've used incorrect encoding name (`iso88595` instead of canonic
`iso-8859-5` that Cygwin has and understands)
Nevertheless, as I've already said that is not a Russian locale specific
issue.
The problem in tests for me now is a language (that uses iso-8859-1
encoding) I do not speak or even write ;)
--
Alexey Shumkin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] Reroll patches against v1.8.3.1
Alexey Shumkin alex.crez...@gmail.com writes: On Tue, Jun 25, 2013 at 12:28:02PM -0700, Junio C Hamano wrote: ... If someone can do the same with latin1, I'd be happy. ... But today I've taken a look to Cygwin's locales more closely and found out that I've used incorrect encoding name (`iso88595` instead of canonic `iso-8859-5` that Cygwin has and understands) Nevertheless, as I've already said that is not a Russian locale specific issue. The problem in tests for me now is a language (that uses iso-8859-1 encoding) I do not speak or even write ;) Many of the people on thee list don't, either, and that is perfectly OK. For the purpose of the test, áëîõù should be sufficient. In a sense, using such an artificial string would make it even clearer than a real message that what we are testing, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/5] Reroll patches against v1.8.3.1
It's all started here [http://thread.gmane.org/gmane.comp.version-control.git/177634] and recently continued later [http://thread.gmane.org/gmane.comp.version-control.git/214419] v6 of this patch series includes Junio's suggestions against v5: 1. [PATCH v6 1/5] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs using modern 'git rev-parse HEAD:' git syntax to get commit tree ID 2. [PATCH v6 2/5] t7102 (reset): don't hardcode SHA-1 in expected outputs untouched 3. [PATCH v6 3/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs using 'rev-parse --short' instead of 'git rev-list --max-count=1 --abbrev-commit' 4. [PATCH v6 4/5] pretty: Add failing tests: --format output should honor logOutputEncoding iso8859-5 encoding reverted back to cp1251 encoding (as it was in v4 series) reworded log message 5. [PATCH v6 5/5] pretty: --format output should honor logOutputEncoding reworded log message Alexey Shumkin (5): t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs t7102 (reset): don't hardcode SHA-1 in expected outputs t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs pretty: Add failing tests: --format output should honor logOutputEncoding pretty: --format output should honor logOutputEncoding builtin/reset.c | 6 +- builtin/rev-list.c | 1 + builtin/shortlog.c | 1 + log-tree.c | 1 + submodule.c | 1 + t/t4041-diff-submodule-option.sh | 25 +++-- t/t4205-log-pretty-formats.sh| 179 - t/t6006-rev-list-format.sh | 207 --- t/t7102-reset.sh | 37 ++- 9 files changed, 293 insertions(+), 165 deletions(-) -- 1.8.3.1.15.g5c23c1e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] Reroll patches against v1.8.3.1
Alexey Shumkin alex.crez...@gmail.com writes: 4. [PATCH v6 4/5] pretty: Add failing tests: --format output should honor logOutputEncoding iso8859-5 encoding reverted back to cp1251 encoding (as it was in v4 series) Thanks for a reroll, but why this change? The reason I asked you to avoid 8859-5 is because our test do not use that encoding and I do not want to add new dependency to people when they run test. cp1251 shares exactly the same issue, doesn't it? So in that sense, this change does not make anything better. That is why I asked you if the breakage you are trying to demonstrate about non-ASCII non-UTF8 encoding was specific to Cyrillic/Russian. I do not recall seeing your answer, but what is the right thing to do depend on it. - If the answer is yes, then we would need to add dependency either way, and 8859-5 is just as fine as cp1251. - If the breakage is not specific to Cyrillic, it should be reproducible using 8859-1 (latin-1), and our tests already depend on 8859-1, so we wouldn't be adding new dependencies on people. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
