1139429 - Build fixes for ICU on SunOS/OpenSolaris

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[ijdt.editor]

Attachment: configure.diff

+++ This bug was initially created as a clone of Bug #853301 +++

Enable the implementation of the ECMAScript Internationalization API, which has so far been landed disabled, for desktop Firefox.

Configure fails on OpenSolaris/OpenIndiana because SunOS is not mentioned as platform where ICU coompiles when in fact it compiles just fine. Please review the attached diffs and provided they are OK please apply them to the source tree.

Comment 1

[Steven R. Loomis]

Comment on attachment 8572631 [details] [diff] [review]
configure.diff

I question why this line isn't "*)".  What platforms do you build on where ICU isn't available?

Comment 2

[ijdt.editor]

I build on SunOS and ICU is available on the system and compiles just fine. However, when I try to compile Firefox the configure script complains that the "ECMAScript Internationalization API" has not been ported on this platform!

Comment 3

[Steven R. Loomis]

(In reply to ijdt.editor from comment #2)
> I build on SunOS and ICU is available on the system and compiles just fine.
> However, when I try to compile Firefox the configure script complains that
> the "ECMAScript Internationalization API" has not been ported on this
> platform!

Yes, it was a question for the maintainers. I'd think for unknown platforms it should at least try instead of failing. But it's a policy decision.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8572631 [details] [diff] [review]
configure.diff

Review of attachment 8572631 [details] [diff] [review]:
-----------------------------------------------------------------

Those changes to configure are not necessary, since they aren't under source version control.

For future contributions, please refer to:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 5

[Matt Brubeck (:mbrubeck)]

This patch has been approved.  Congratulations!  Next steps for the author of the patch:

* Apply the same changes to "configure.in" and "js/src/configure.in".
* Follow the directions from the link in comment 4 to generate a patch for check-in.  Include the bug number and "r=glandium" in the commit message.
* Attach that patch to this bug, and mark the previous attachment "obsolete".

We'll take a final look and the patch and then get it checked in for you.  Thanks!

If you have any questions or problems with any of the steps, feel free to comment here, or email me, or ask in #introduction on irc.mozilla.org.

Comment 6

[ijdt.editor]

Instead of changing  "configure.in" and "js/src/configure.in" one needs to change build/autoconf/icu.m4 and the patch for this is included in the original patch. I am a bit confused...

Comment 7

[Matt Brubeck (:mbrubeck)]

(In reply to ijdt.editor from comment #6)
> Instead of changing  "configure.in" and "js/src/configure.in" one needs to
> change build/autoconf/icu.m4 and the patch for this is included in the
> original patch. I am a bit confused...

Sorry, my mistake.  Just follow the directions to generate a patch using Mercurial and you should be fine.  The patch will *not* include changes to "configure" or "js/src/configure" because these files are not stored in Mercurial.

Comment 8

[ijdt.editor]

Attachment: icu.diff

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: patch for checkin

This is the same patch as comment 8, with Mercurial headers and a commit message added.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/88b7e9e2555d

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/88b7e9e2555d