Statically link own version of FreeType on Linux

Statically link own version of FreeType on Linux

For color font support, CFF and CFF2 rasterization, and OpenType font
variations, latest security fixes, Chrome needs an up-to-date version of
FreeType (more details on the bug). Since distributions are too far
behind in the shipped system FreeType versions, we need to start linking
in our own copy.

For distributions that build their own Chromium this CL introduces a
switch use_system_freetype which can be used to keep the old linking
configuration at the expense of risking introduction of text rendering
and security regressions if the system FreeType is too old.

BUG=274030
Review-Url: https://codereview.chromium.org/2863063003
Cr-Commit-Position: refs/heads/master@{#470097}
Committed: https://chromium.googlesource.com/chromium/src/+/c15bda3bf4bbe231186176da9d809fed356f8cbf

[drott, 2017-05-05 13:51:04]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 13:51:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 14:56:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 14:56:11]

Dry run: This issue passed the CQ dry run.

[drott, 2017-05-05 15:24:33]

drott@chromium.org changed reviewers:
+ bungeman@chromium.org, dpranke@chromium.org, eae@chromium.org

[bungeman-chromium, 2017-05-05 15:57:05]

lgtm

[drott, 2017-05-05 16:00:37]

On 2017/05/05 at 15:57:05, bungeman wrote:
> lgtm

Thanks for the quick review. I'll wait a bit before landing and give people time
to respond to the post on blink-dev and chromium-dev.

[drott, 2017-05-05 16:15:36]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 16:15:56]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eae, 2017-05-05 16:28:36]

LGTM

[Dirk Pranke, 2017-05-05 16:37:19]

I'm not sure who we can get to update the chromeos ebuild configs these days;
maybe vapier@ or stevenjb@ can advise.

lgtm w/ the comment below addressed.

https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
File build/config/freetype/BUILD.gn (right):

https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
build/config/freetype/BUILD.gn:19: if (is_chromeos || use_system_freetype) {
Nit: is_chromeos does not mean this is an actual chromeos build, unfortunately.
We set target_os="chromeos" on the "desktop chromeos" builds as well, like 

https://luci-milo.appspot.com/buildbot/chromium.chromiumos/Linux%20ChromiumOS...

so this won't quite have the desired effect.

If you want a "true" chromeos build, you could check for `host_toolchain ==
"//build/toolchain/cros:host". However, it's probably better to just use
use_system_freetype and make sure they set the GN arg in their builds.

[commit-bot: I haz the power, 2017-05-05 17:44:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 17:44:51]

Dry run: This issue passed the CQ dry run.

[vapier, 2017-05-05 17:50:50]

vapier@chromium.org changed reviewers:
+ vapier@chromium.org

[vapier, 2017-05-05 17:50:51]

https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
File build/config/freetype/BUILD.gn (right):

https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
build/config/freetype/BUILD.gn:19: if (is_chromeos || use_system_freetype) {
we should just update the Chrome OS ebuild to pass use_system_freetype=true, and
then we don't need this "is_chromeos" check at all

will GN reject unknown args ?  or is it safe to land that ebuild change now even
if the Chrome source doesn't check that arg ?

[Dirk Pranke, 2017-05-05 19:46:22]

On 2017/05/05 17:50:51, vapier wrote:
>
https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
> File build/config/freetype/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
> build/config/freetype/BUILD.gn:19: if (is_chromeos || use_system_freetype) {
> we should just update the Chrome OS ebuild to pass use_system_freetype=true,
and
> then we don't need this "is_chromeos" check at all
> 
> will GN reject unknown args ?  or is it safe to land that ebuild change now
even
> if the Chrome source doesn't check that arg ?

I think it's safe to land the ebuild change now. By default, GN will report that
argument is 
unused (with an "ERROR" message), but the build doesn't actually fail; the
return code will 
be zero. You can pass --fail-on-unused-args to change that, but I don't think
the ebuild
does that, IIRC.

[vapier, 2017-05-05 19:53:19]

i've posted https://chromium-review.googlesource.com/497167 then for the CrOS
side

[bungeman-chromium, 2017-05-05 20:59:54]

https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
File build/config/freetype/BUILD.gn (right):

https://codereview.chromium.org/2863063003/diff/20001/build/config/freetype/B...
build/config/freetype/BUILD.gn:15: use_system_freetype = false
Should this (or its negation) be used in skia/BUILD.gn when deciding to define
SK_FREETYPE_MINIMUM_RUNTIME_VERSION ? Seems like that may simplify the logic
there and makes it more obvious what it's used for.

[drott, 2017-05-08 08:46:00]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-08 08:46:44]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-08 10:23:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-08 10:23:30]

Dry run: This issue passed the CQ dry run.

[drott, 2017-05-08 19:56:15]

On 2017/05/05 at 19:53:19, vapier wrote:
> i've posted https://chromium-review.googlesource.com/497167 then for the CrOS
side

This landed, so I'll go ahead and try to land the Chromium/Blink side change.
Thanks for your help, vapier@.

[drott, 2017-05-08 19:56:19]

The CQ bit was checked by drott@chromium.org

[drott, 2017-05-08 19:56:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
bungeman@chromium.org, dpranke@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2863063003/#ps40001
(title: "Rely on ChromeOS using the use_system_freetype switch")

[commit-bot: I haz the power, 2017-05-08 19:57:13]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-08 20:06:40]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494273379973500,
"parent_rev": "0e108ddb699df8f9b1cbd409cf2677739b4058a1", "commit_rev":
"c15bda3bf4bbe231186176da9d809fed356f8cbf"}

[commit-bot: I haz the power, 2017-05-08 20:07:13]

Description was changed from

==========
Statically link own version of FreeType on Linux

For color font support, CFF and CFF2 rasterization, and OpenType font
variations, latest security fixes, Chrome needs an up-to-date version of
FreeType (more details on the bug). Since distributions are too far
behind in the shipped system FreeType versions, we need to start linking
in our own copy.

For distributions that build their own Chromium this CL introduces a
switch use_system_freetype which can be used to keep the old linking
configuration at the expense of risking introduction of text rendering
and security regressions if the system FreeType is too old.

BUG=274030
==========

to

==========
Statically link own version of FreeType on Linux

For color font support, CFF and CFF2 rasterization, and OpenType font
variations, latest security fixes, Chrome needs an up-to-date version of
FreeType (more details on the bug). Since distributions are too far
behind in the shipped system FreeType versions, we need to start linking
in our own copy.

For distributions that build their own Chromium this CL introduces a
switch use_system_freetype which can be used to keep the old linking
configuration at the expense of risking introduction of text rendering
and security regressions if the system FreeType is too old.

BUG=274030

Review-Url: https://codereview.chromium.org/2863063003
Cr-Commit-Position: refs/heads/master@{#470097}
Committed:
https://chromium.googlesource.com/chromium/src/+/c15bda3bf4bbe231186176da9d80...
==========

[commit-bot: I haz the power, 2017-05-08 20:07:15]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c15bda3bf4bbe231186176da9d80...

[Tom Anderson, 2017-05-08 22:17:41]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2868793004/ by thomasanderson@chromium.org.

The reason for reverting is: Deps change picked up by official builder:
https://luci-milo.appspot.com/buildbot/chromium.chrome/Google%20Chrome%20Linu...

Please remove freetype from these files:
chrome/installer/linux/debian/expected_deps_ia32_jessie
chrome/installer/linux/debian/expected_deps_x64_jessie
chrome/installer/linux/rpm/expected_deps_i386
chrome/installer/linux/rpm/expected_deps_x86_64.

[findit-for-me, 2017-05-09 04:21:43]

Findit (https://goo.gl/kROfz5) confirmed this CL at revision 470097 as the
culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...