MinGW port and ICU patches

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

MinGW port and ICU patches

Eike Rathke
Hi,

Does someone (Vladimir? you applied the patches) know off-hand who did
the MinGW port and created the patches that are applied to ICU? I would
need a "clean" copy of the current patch set, clean in the sense of not
mangled with other patches we apply to the ICU, maybe such set is around
on someone's disk? Background is that we want to upstream that so we can
get rid of the patches for the next version of ICU.

Thanks
  Eike

--
 OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 SunSign   0x87F8D412 : 2F58 5236 DB02 F335 8304  7D6C 65C9 F9B5 87F8 D412
 OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
 Please don't send personal mail to the [hidden email] account, which I use for
 mailing lists only and don't read from outside Sun. Use [hidden email] Thanks.

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Takashi Ono
Hi Eike,

I am responsible for MinGW port. I think it is difficult to get rid of patches
because of unicode handlings.

In ooo sources, unicode characters are represented by sal_Unicode and it is
typedef'ed to unsigned short. In the meantime in MinGW environment, wchart_t is an
built-in type of 16 bit unsigned integer type but is incompatible with unsigned
short.

As Windows APIs require wchar_t, There are so many static_cast's here and there and
therefore I prefer patch to let icu to handle unicode by using unsigned short even if
there exists usable wchar_t not to have more static_cast's.

Other patches to cope with strange behaviour of recent versions of cygwin tools as
workarounds, and make dll's to have similar names to MSVC build are also included.

Anyways IMHO most of the patches for MinGW port are quite local and have to be
reviewed before being proposed to upstream.

Takashi Ono ([hidden email])

In message "[porting-dev] MinGW port and ICU patches",
Eike Rathke wrote...
 >Hi,
 >
 >Does someone (Vladimir? you applied the patches) know off-hand who did
 >the MinGW port and created the patches that are applied to ICU? I would
 >need a "clean" copy of the current patch set, clean in the sense of not
 >mangled with other patches we apply to the ICU, maybe such set is around
 >on someone's disk? Background is that we want to upstream that so we can
 >get rid of the patches for the next version of ICU.
 >
 >Thanks
 >  Eike
 >
 >--
 > OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 > SunSign   0x87F8D412 : 2F58 5236 DB02 F335 8304  7D6C 65C9 F9B5 87F8 D412
 > OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
 > Please don't send personal mail to the [hidden email] account, which I use for
 > mailing lists only and don't read from outside Sun. Use [hidden email] Thanks.
 >______________________________________________________________________
 >Content-Type: application/pgp-signature
 >Content-Disposition: inline
 >
 >-----BEGIN PGP SIGNATURE-----
 >Version: GnuPG v1.4.7 (GNU/Linux)
 >
 >iD8DBQFH8+SCZcn5tYf41BIRAjV1AJ0aRVbyHnmMRU7yIYBQ/TOt0t9SXQCfe9Qs
 >0o1yoZUmYWF5kNWwr1tbx9g=
 >=Vp5z
 >-----END PGP SIGNATURE-----


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Eike Rathke
Hi Takashi,

On Sunday, 2008-04-06 21:55:06 +0900, Takashi Ono wrote:

> I am responsible for MinGW port. I think it is difficult to get rid of patches
> because of unicode handlings.
>
> In ooo sources, unicode characters are represented by sal_Unicode and it is
> typedef'ed to unsigned short. In the meantime in MinGW environment, wchart_t is an
> built-in type of 16 bit unsigned integer type but is incompatible with unsigned
> short.

How is sal_Unicode related to ICU? ICU doesn't use sal_Unicode.

> As Windows APIs require wchar_t, There are so many static_cast's here and there and
> therefore I prefer patch to let icu to handle unicode by using unsigned short even if
> there exists usable wchar_t not to have more static_cast's.

static_cast where? In ICU? According to
http://source.icu-project.org/repos/icu/icu/tags/release-3-8-1/readme.html#HowToBuildSupported
MinGW is supported but rarely tested. Patches necessary to make it build
should be upstreamed.

If patches to ICU are necessary because of how OOo uses it, we should
find another solution than patching ICU.

Patches are lost respectively would had to be reapplied by the port
maintainer (you ;-) whenever we upgraded to a newer version of ICU,
which IMHO is unnecessary time consuming work.

> Other patches to cope with strange behaviour of recent versions of cygwin tools as
> workarounds,

These may be worth upstreaming, if it isn't just because cygwin is
buggy.

> and make dll's to have similar names to MSVC build are also included.

Those of course wouldn't make it upstream. Is that really needed?
Shouldn't it be better addressed in solenv/inc/libs.mk and packaging?

> Anyways IMHO most of the patches for MinGW port are quite local and have to be
> reviewed before being proposed to upstream.

Please add details to
http://wiki.services.openoffice.org/wiki/ICU/bugs_and_patches#MinGW_platform
and for the patches that should go upstream please file an ICU bug at
http://bugs.icu-project.org/trac/ and add the ticket number to the wiki
similar to the other patches mentioned there, so we can track progress.

Thanks
  Eike

--
 OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 SunSign   0x87F8D412 : 2F58 5236 DB02 F335 8304  7D6C 65C9 F9B5 87F8 D412
 OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
 Please don't send personal mail to the [hidden email] account, which I use for
 mailing lists only and don't read from outside Sun. Use [hidden email] Thanks.

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Takashi Ono
Hi Eike,

In message "Re: [porting-dev] MinGW port and ICU patches",
Eike Rathke wrote...
 >Hi Takashi,
 >
 >On Sunday, 2008-04-06 21:55:06 +0900, Takashi Ono wrote:
 >
 >> I am responsible for MinGW port. I think it is difficult to get rid of patches
 >> because of unicode handlings.
 >>
 >> In ooo sources, unicode characters are represented by sal_Unicode and it is
 >> typedef'ed to unsigned short. In the meantime in MinGW environment, wchart_t is
an
 >> built-in type of 16 bit unsigned integer type but is incompatible with unsigned
 >> short.
 >
 >How is sal_Unicode related to ICU? ICU doesn't use sal_Unicode.
 >

With the configure script that comes with icu distribution, icu prefer using wchar_t
to represent unicode with wchar_t if wchar_t is available and is a 16 bit integer
type. So the pointer to UChar is not compatible with sal_Unicode unless we do not use
reinterpret_cast's

 >> As Windows APIs require wchar_t, There are so many static_cast's here and there
and
 >> therefore I prefer patch to let icu to handle unicode by using unsigned short
even if
 >> there exists usable wchar_t not to have more static_cast's.
 >
 >static_cast where? In ICU? According to
 >http://source.icu-project.org/repos/icu/icu/tags/release-3-8-1/readme.html#HowToBuildSupported
 >MinGW is supported but rarely tested. Patches necessary to make it build
 >should be upstreamed.
 >

It is buildable with MinGW tools but we have to use reinterpret_cast whereever we
assign a pointer to UChar to a pointer to sal_Unicode.

 >If patches to ICU are necessary because of how OOo uses it, we should
 >find another solution than patching ICU.
 >
 >Patches are lost respectively would had to be reapplied by the port
 >maintainer (you ;-) whenever we upgraded to a newer version of ICU,
 >which IMHO is unnecessary time consuming work.
 >
 >> Other patches to cope with strange behaviour of recent versions of cygwin tools
as
 >> workarounds,
 >
 >These may be worth upstreaming, if it isn't just because cygwin is
 >buggy.
 >

Some people may think they are bugs but they may be worth upstreaming.

 >> and make dll's to have similar names to MSVC build are also included.
 >
 >Those of course wouldn't make it upstream. Is that really needed?
 >Shouldn't it be better addressed in solenv/inc/libs.mk and packaging?
 >

It is possible but we have to have many conditionals there and some OOo developpers
may not like it. I have had a comment that the commenter likes to have even less.

 >> Anyways IMHO most of the patches for MinGW port are quite local and have to be
 >> reviewed before being proposed to upstream.
 >
 >Please add details to
 >http://wiki.services.openoffice.org/wiki/ICU/bugs_and_patches#MinGW_platform
 >and for the patches that should go upstream please file an ICU bug at
 >http://bugs.icu-project.org/trac/ and add the ticket number to the wiki
 >similar to the other patches mentioned there, so we can track progress.
 >

I will do so when I have time.

 >Thanks
 >  Eike
 >
 >--
 > OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 > SunSign   0x87F8D412 : 2F58 5236 DB02 F335 8304  7D6C 65C9 F9B5 87F8 D412
 > OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
 > Please don't send personal mail to the [hidden email] account, which I use for
 > mailing lists only and don't read from outside Sun. Use [hidden email] Thanks.
 >______________________________________________________________________
 >Content-Type: application/pgp-signature
 >Content-Disposition: inline
 >
 >-----BEGIN PGP SIGNATURE-----
 >Version: GnuPG v1.4.7 (GNU/Linux)
 >
 >iD8DBQFH/gOfZcn5tYf41BIRArKHAJ4hieUQDUPe8eUAfInF6hGeiWHOaQCfZWm+
 >I+yFCTfD3cLx/cm1qUMhhxM=
 >=3olr
 >-----END PGP SIGNATURE-----

----
 Takashi Ono(HK Freak)
 mailto:[hidden email] or [hidden email]
        (Personal Address, checked every morning/evening and holidays)
 mailto:[hidden email]
        (Address for business, checked every working days)
 http://www.hkfreak.net

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Eike Rathke
Hi Takashi,

On Thursday, 2008-04-10 21:34:24 +0900, Takashi Ono wrote:

>  >How is sal_Unicode related to ICU? ICU doesn't use sal_Unicode.
>  >
>
> With the configure script that comes with icu distribution, icu prefer using wchar_t
> to represent unicode with wchar_t if wchar_t is available and is a 16 bit integer
> type. So the pointer to UChar is not compatible with sal_Unicode unless we do not use
> reinterpret_cast's
> [...]
>  >static_cast where? In ICU? According to
>  >http://source.icu-project.org/repos/icu/icu/tags/release-3-8-1/readme.html#HowToBuildSupported
>  >MinGW is supported but rarely tested. Patches necessary to make it build
>  >should be upstreamed.
>  >
>
> It is buildable with MinGW tools but we have to use reinterpret_cast whereever we
> assign a pointer to UChar to a pointer to sal_Unicode.
Do you know off-hand or could give me a pointer where assigning UChar*
to sal_Unicode* or vice versa was done? I consider relying on such
implementation detail a misuse ... if it wasn't for very good reason.


>  >> and make dll's to have similar names to MSVC build are also included.
>  >
>  >Those of course wouldn't make it upstream. Is that really needed?
>  >Shouldn't it be better addressed in solenv/inc/libs.mk and packaging?
>  >
>
> It is possible but we have to have many conditionals there and some OOo developpers
> may not like it. I have had a comment that the commenter likes to have even less.

And I want to have less patches against ICU ;-)

Having conditionals or macros in libs.mk and packaging would probably be
a one-time effort with little maintenance needed, while platform patches
against ICU would had to be reapplied and maintained for every ICU
upgrade. Given the awful unstructured all-in-one patch bulk I consider
having to handle that much worse than any "I don't like conditionals"
comment.

  Eike

--
 OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 SunSign   0x87F8D412 : 2F58 5236 DB02 F335 8304  7D6C 65C9 F9B5 87F8 D412
 OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
 Please don't send personal mail to the [hidden email] account, which I use for
 mailing lists only and don't read from outside Sun. Use [hidden email] Thanks.

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Takashi Ono
Dear Eike,

In message "Re: [porting-dev] MinGW port and ICU patches",
Eike Rathke wrote...

 >Do you know off-hand or could give me a pointer where assigning UChar*
 >to sal_Unicode* or vice versa was done? I consider relying on such
 >implementation detail a misuse ... if it wasn't for very good reason.
 >

In both line #185 and 186 in i18npool/source/breakiterator/breakiterator_unicode.cxx,
constructor of UnicodeString, which requires const UChar* as a 1st argument, is
called with const sal_Unicode* string.

I may be possible to implement a switch in icu configure script to force using
unsigned short instead of wchar_t and propose it upstream.

 >And I want to have less patches against ICU ;-)
 >
 >Having conditionals or macros in libs.mk and packaging would probably be
 >a one-time effort with little maintenance needed, while platform patches
 >against ICU would had to be reapplied and maintained for every ICU
 >upgrade. Given the awful unstructured all-in-one patch bulk I consider
 >having to handle that much worse than any "I don't like conditionals"
 >comment.

I think your coment makes sense and will not implement more modifications in patches
for 3rd party modules to have less conditionals elsewhere. Probably I can make the
patch for icu a bit smaller in the future with the strategy.

Takashi Ono ([hidden email])

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Eike Rathke
Hi Takashi,

On Friday, 2008-04-18 06:21:11 +0900, Takashi Ono wrote:

>  >Do you know off-hand or could give me a pointer where assigning UChar*
>  >to sal_Unicode* or vice versa was done? I consider relying on such
>  >implementation detail a misuse ... if it wasn't for very good reason.
>  >
>
> In both line #185 and 186 in i18npool/source/breakiterator/breakiterator_unicode.cxx,
> constructor of UnicodeString, which requires const UChar* as a 1st argument, is
> called with const sal_Unicode* string.

That indeed should be changed. Thanks for the pointer. Is that the only
place where you encountered such type mismatch?

> I may be possible to implement a switch in icu configure script to force using
> unsigned short instead of wchar_t and propose it upstream.

But not using wchar_t wouldn't go with the Windows API, would it?

>  >[... conditionals in build environment vs. bulk patches ...]
>
> I think your coment makes sense and will not implement more modifications in patches
> for 3rd party modules to have less conditionals elsewhere. Probably I can make the
> patch for icu a bit smaller in the future with the strategy.

Sounds promising :-)

Thanks
  Eike

--
 OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 SunSign   0x87F8D412 : 2F58 5236 DB02 F335 8304  7D6C 65C9 F9B5 87F8 D412
 OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
 Please don't send personal mail to the [hidden email] account, which I use for
 mailing lists only and don't read from outside Sun. Use [hidden email] Thanks.

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MinGW port and ICU patches

Takashi Ono
Hi Eike,

In message "Re: [porting-dev] MinGW port and ICU patches",
Eike Rathke wrote...
 >That indeed should be changed. Thanks for the pointer. Is that the only
 >place where you encountered such type mismatch?

I decided to modify icu because I have encountered so many simiar code. Although it was
more than a year ago, I am afraid there still may be many. I just guessed i18npool
should have one example.

 >> I may be possible to implement a switch in icu configure script to force using
 >> unsigned short instead of wchar_t and propose it upstream.
 >
 >But not using wchar_t wouldn't go with the Windows API, would it?
 >

Actually wchar_t is typedef'ed to unsigned short in Microsoft headers.

Takashi Ono ([hidden email])

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]