Re: [PATCH v6 0/5] Reroll patches against v1.8.3.1

[Alexey Shumkin <alex.crezoff@gmail.com>]

On Tue, Jun 25, 2013 at 12:28:02PM -0700, Junio C Hamano wrote:
> Alexey Shumkin <Alex.Crezoff@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.Crezoff@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