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 [off-list ref] writes:

quoted

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 [off-list ref] writes:

quoted

@@ -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.

quoted

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