New issue
Advanced search Search tips

Issue 773309 link

Starred by 1 user
Status: Fixed
Owner:
eseckler@chromium.org
Closed: Oct 2017
Cc:
bunge...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

freetype: Chromium build targets may include different versions of ftconfig.h

Project Member Reported by eseckler@chromium.org, Oct 10 2017 
The FT_CONFIG_CONFIG_H macro is only set within freetype implementation targets, which can lead to incorrect config headers being included from other build targets.

When compiling with an #error preprocessor directive in /third_party/freetype/src/include/freetype/config/ftheader.h:110, I could see the problem arising in at least these places:

In file included from ../../third_party/skia/src/ports/SkFontHost_FreeType_common.cpp:17:
In file included from ../../third_party/freetype/src/include/ft2build.h:37:
../../third_party/freetype/src/include/freetype/config/ftheader.h:110:2: error: "FT_CONFIG_CONFIG_H not defined"

In file included from ../../third_party/harfbuzz-ng/src/hb-ft.cc:32:
In file included from ../../third_party/harfbuzz-ng/src/hb-ft.h:34:
In file included from ../../third_party/freetype/src/include/ft2build.h:37:
../../third_party/freetype/src/include/freetype/config/ftheader.h:110:2: error: "FT_CONFIG_CONFIG_H not defined"

In file included from ../../third_party/skia/src/ports/SkFontHost_FreeType.cpp:31:
In file included from ../../third_party/freetype/src/include/ft2build.h:37:
../../third_party/freetype/src/include/freetype/config/ftheader.h:110:2: error: "FT_CONFIG_CONFIG_H not defined"
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc6e6a2cbabaa211711f73fe613e371fe6cc6495

commit fc6e6a2cbabaa211711f73fe613e371fe6cc6495
Author: Eric Seckler <eseckler@chromium.org>
Date: Wed Oct 11 13:41:48 2017

[freetype] Ensure correct config headers are used

Previously, the FT_CONFIG_CONFIG_H macro was only set within freetype
targets, which could lead to incorrect config headers being included
from other targets.

This patch resolves the issue by moving the macro into the
freetype_config definition.

While the other FT_CONFIG_*_H macros currently affect only freetype's
implementation, it makes sense to move those into the freetype_config,
too, in case other targets decide to include those headers in the
future.

Change-Id: Ia773d80a0a4b827a122a9040e18a546a66e32d97

Bug:  773309 
Change-Id: Ia773d80a0a4b827a122a9040e18a546a66e32d97
Reviewed-on: https://chromium-review.googlesource.com/708800
Commit-Queue: Ben Wagner <bungeman@chromium.org>
Reviewed-by: Ben Wagner <bungeman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507966}
[modify] https://crrev.com/fc6e6a2cbabaa211711f73fe613e371fe6cc6495/third_party/freetype/BUILD.gn

Comment 2 by eseckler@chromium.org, Oct 13 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment