491855 - Enabling CoreText should be explicit, instead of magic based on the SDK

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Benjamin Smedberg]

Our policy with configure options is that non-default options should be explicitly specified, and features should not be selected based solely on configure tests.

To build with coretext we should be using --enable-coretext instead of magically enabling coretext with the 10.5 SDK.

Comment 1

[Benjamin Smedberg]

Attachment: Explicit --enable-coretext flag, rev. 1

Tested correct behavior with 10.4/10.5 --enable-coretext and not.

Comment 2

[Benjamin Smedberg]

In the future please have Ted or me review substantive changes to configure such as this.

Comment 3

[Jonathan Kew [:jfkthame]]

(In reply to comment #2)
> In the future please have Ted or me review substantive changes to configure
> such as this.

Sorry; relatively new here, and not always sure who should review what, so still trying to learn from those who've been around longer.

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to comment #0)
> Our policy with configure options is that non-default options should be
> explicitly specified, and features should not be selected based solely on
> configure tests.

OK; but the intention is for coretext to become the default (eventually only) option, AFAIK.

> To build with coretext we should be using --enable-coretext instead of
> magically enabling coretext with the 10.5 SDK.

What do you expect to happen when building for 64-bit; should an explicit --enable-coretext still be required?

Comment 5

[Benjamin Smedberg]

We can flip the default at an appropriate time.

I think for the time being requiring 64-bit builders to specify --enable-coretext is ok, but we could also set an explicit default for 64-bit.

Comment 6

[Josh Aas]

Please make an explicit default for 64-bit, mozconfigs for that are complicated enough already and ATSUI is not an option at all on 64-bit.

Comment 7

[Benjamin Smedberg]

Attachment: Explicit --enable-coretext flag, default on for x86-64, rev. 2

Comment 8

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/1507fddc9b51