Re: [PATCH 2/2] t8005: convert CP1251 character set to ISO8859-5

From: Brandon Casey <casey@nrlssc.navy.mil>
Date: May 26, 2009, 14:56 UTC
Message-ID: <zik8bR55cIiktB2euEByCZIZJ1jtx64yA5n1Ki8IMrW1wLBzuBR8Yw@cipher.nrlssc.navy.mil>
In-Reply-To: <20090525092027.GA22382@coredump.intra.peff.net>

Jeff King wrote:

Show 7 quoted lines

> On Fri, May 22, 2009 at 06:06:06PM -0700, Junio C Hamano wrote:
> 
>> It also appears that we do not need OLD_ICONV anymore, as our Makefile
>> suggests for older releases.
>>
>> It would be nice if people with older Solaris can check if I am not
>> breaking things for them...

I will.

> Git builds fine on my Solaris 8 test box with this patch. It also builds
> fine if I drop OLD_ICONV, so it may be that we can unconditionally do
> so (but I don't have any older boxen to test with).

It also appears from looking at /usr/include/iconv.h that Solaris 10 does not need it as long as _XPG6 is defined since that file has:

  #ifdef _XPG6
  extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
                  size_t *_RESTRICT_KYWD, char **_RESTRICT_KYWD,
                  size_t *_RESTRICT_KYWD);
  #else
  extern size_t   iconv(iconv_t, const char **_RESTRICT_KYWD,
                  size_t *_RESTRICT_KYWD, char **_RESTRICT_KYWD,
                  size_t *_RESTRICT_KYWD);
  #endif

I haven't tested, but I will.  I think Solaris 7 needs it though.

Show 6 quoted lines

> With current 'next' and NO_PERL set, I can pass every test except:
> 
>   t3900 - still problems in the eucJP test. I haven't looked closely,
>           but my understanding is that this might need extra language
>           packs installed (I know virtually nothing about Solaris
>           administration and the box is not mine).

t3900.2[23] ?

Yes, Solaris 7 and 10 fail for me on those tests. Neither can convert between eucJP and ISO-2022-JP. The other tests pass because it can convert from either of those to UTF-8 and vice versa.

The iconv libraries seem to be named according to the conversions that they can perform. So in /usr/lib/iconv/ you'll see:

   UTF-8%eucJP.so
   eucJP%UTF-8.so
   ISO-2022-JP%UTF-8.so
   UTF-8%ISO-2022-JP.so
   etc.

but you won't see (at least I don't)

   eucJP%ISO-2022-JP.so
   ISO-2022-JP.so%eucJP

>   t8005 - git produces incorrect (or at least not expected) results for
>           the iso8859-5 to sjis conversion. It all looks like control
>           characters to me, so I'm not sure how to diagnose (and it may
>           just be an installation issue again).

t8005.[23]

Same reason as above, no conversion between ISO8859-5 and SJIS. t8005.4 passes since conversion for both of those to UTF-8 is installed.

>   t4116,t5000,t5001 - Solaris tar doesn't like pax headers that
>                       git-archive generates

Same here.

On Solaris 7, I additionally must skip:

   t5100.[56] t5100.1[026-9] t5100.2[0-6]

This is because iconv on Solaris 7 doesn't understand charset 'us-ascii'. If I change 'charset=us-ascii' to 'charset=646' everywhere in t5100/sample.mbox and t5100/rfc2047-samples.mbox, then all of t5100 passes. It seems '646' is the only name by which Solaris 7 recognizes ISO 646 aka ascii. Other platforms do not recognize '646'.

Also, if you want to test with the Korn shell (you'll need a couple minor tweaks to the test lib), t6030.1[23] must be skipped. It seems even Solaris 10 has what I consider a bug in ksh which affects how traps are handled. A trap set in one function, will not be called in the second sub-function.

i.e.

   f1()
     sets a trap on EXIT
     calls f2()
       calls f3()
         calls exit

   The trap set in f1() will _not_ be executed.

I have only observed this behavior on Solaris. git-bisect sets such a trap, but I think it is being rewritten in c, so hopefully this issue will go away soon.

-brandon