1387332 - Remove the code behind ENABLE_INTL_API==no condition

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

With bug 1344625 fixed in 56, we can look into removing all the code that lives in m-c just for the ENABLE_INTL_API==no condition.

Comment 1

[Jonathan Kew [:jfkthame]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #0)
> With bug 1344625 fixed in 56, we can look into removing all the code that
> lives in m-c just for the ENABLE_INTL_API==no condition.

I'm eager to get this done, but I would suggest we don't actually start removing stuff until we've seen bug 1344625 successfully make it through to Release without problems.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

That's fair. We can wait.

My only thought is that the assumption that we can turn it off again is quickly becoming an illusion. People are turning on things in mobile that depend on mozIntl and Intl APIs.

But in case something goes wrong, I guess it's easier to revert it without this patch here, so let's axe it in 58.

Comment 3

[Daniel Holbert [:dholbert]]

Before proceeding much further here, could you land a patch to make...
  ac_add_options --without-intl-api
...unrecognized by our build system?

Otherwise that previously-working mozconfig option is just becoming a footgun, guarding a recently-working-but-now-broken build configuration.

If I add it to my mozconfig in current mozilla-central, my build fails with "ERROR PROCESSING MOZBUILD FILE" for netwerk/dns/moz.build , since the mozconfig option successfully prevents ICU from being built, and that causes problems for directories like DNS that now depend on ICU unconditionally as of bug 1402048.  Ideally, our configuration process shouldn't honor --without-intl-api at all anymore, since it is now unsupported I guess.  And then I think it'll error out early in the build process, with a clearer* build error

* (although still slightly inscrutable; see bug 1401255)

Comment 4

[Jonathan Kew [:jfkthame]]

Yes, I agree we should do this; I'm not sure quite how it should all work, though, given that the JS engine wants to retain the option to build without ICU (and hence without ENABLE_INTL_API) for the sake of embedders, although this won't be used in any Firefox builds. (See bug 1402235.)

Which means, I think, that the option support in icu.m4 needs to stick around for now; we want to block Firefox builds from using it while leaving it available to standalone spidermonkey builds.

:gandalf, can you look into this and maybe get advice from the build-system folk as to how best to handle things?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

> :gandalf, can you look into this and maybe get advice from the build-system folk as to how best to handle things?

I'm not sure if I understand the question.

I believe we're on the path to remove the mozconfig option `--without-intl-api` and removing ENABLE_INTL_API config flag from all of Mozilla except of SpiderMonkey which will retrain ENABLE_INTL_API and its own build config flag specific for SM.

NI :till to confirm that.

Comment 6

[Jonathan Kew [:jfkthame]]

I guess my question is how to actually achieve this via the configure scripts. Currently, I believe the --with-intl-api option is handled by the icu.m4 file; but that is shared (IIUC) between normal Firefox builds and standalone Spidermonkey ones. In the one case, we want to remove support for the option; but not in the other.

There are probably several possible ways to go about this, but rather than me starting to hack about in there, we should find out how the build peers think it should be handled.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

My naive guess is that we should move the `--with-intl-api` to SpiderMonkey specific build script and make it always on when building as part of Gecko/Firefox.
But I'll let :till answer this from their use case perspective :)

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1387332 - Remove leftover ENABLE_INTL_API conditions.

Review commit: https://reviewboard.mozilla.org/r/194350/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194350/

Comment 9

[Jonathan Kew [:jfkthame]]

Comment on attachment 8923188 [details]
Bug 1387332 - Remove leftover ENABLE_INTL_API conditions.

https://reviewboard.mozilla.org/r/194350/#review199300

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5879aecaeff1
Remove leftover ENABLE_INTL_API conditions. r=jfkthame

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5879aecaeff1