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

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

Reply via email to