Skia without skia-gpu build broken

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gaston, Unassigned)

Tracking

({regression})

51 Branch
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 unaffected, firefox49 unaffected, firefox50 unaffected, firefox51 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
Probably since skia got enabled by default, build is failing on openbsd (previously it was https://bugzilla.mozilla.org/show_bug.cgi?id=1234494 that prompted to disable skia gpu on open/netbsd in https://dxr.mozilla.org/mozilla-central/source/toolkit/moz.configure#738)

First failure: for some reason USE_SKIA_GPU ends up defined to nothing in mozilla-config.h, which badly breaks because code checks for #if USE_SKIA_GPU

CanvasRenderingContext2D.cpp:1631:17: error: expected value in expression #if USE_SKIA_GPU

If i 'fix' mozilla-config.h to properly define USE_SKIA_GPU to 0, builds fails later on:

/home/othersrc/mozilla-central/gfx/skia/skia/include/core/../gpu/GrTestUtils.h:16:10: fatal error: 'SkRandom.h' file not found

Probably because SkRandom.h isnt copied to the objdir.

So question is, are we still supposed to be able to build skia without skia gpu ?
Reporter

Comment 1

3 years ago
And re-trying to enable skia gpu still fails on the locale_t thing from bug #1299485.

28:08.40 In file included from /home/othersrc/mozilla-central/gfx/skia/skia/src/gpu/gl/builders/GrGLProgramBuilder.cpp:10:
28:08.41 /home/othersrc/mozilla-central/gfx/skia/skia/src/gpu/GrAutoLocaleSetter.h:79:5: error: unknown type name 'locale_t'
28:08.41     locale_t fOldLocale;
28:08.41     ^
28:08.42 /home/othersrc/mozilla-central/gfx/skia/skia/src/gpu/GrAutoLocaleSetter.h:80:5: error: unknown type name 'locale_t'
28:08.42     locale_t fLocale;
28:08.42     ^
28:08.44 /home/othersrc/mozilla-central/gfx/skia/skia/src/gpu/GrAutoLocaleSetter.h:48:19: error: use of undeclared identifier 'newlocale'
28:08.44         fLocale = newlocale(LC_ALL, name, 0);
28:08.44                   ^
28:08.44 /home/othersrc/mozilla-central/gfx/skia/skia/src/gpu/GrAutoLocaleSetter.h:52:38: error: unknown type name 'locale_t'
28:08.44             fOldLocale = static_cast<locale_t>(0);
Comment hidden (mozreview-request)

Updated

3 years ago
Blocks: 1137305
Keywords: regression

Comment 3

3 years ago
Comment on attachment 8786788 [details]
Bug 1299485 - Unbreak build with Skia disabled by default after bug 1137305.

set_define(, False) appears to redefine C macro with empty value in mozilla-config.h which maybe unexpected given the following description:

python/mozbuild/mozbuild/configure/__init__.py:
    def set_define_impl(self, name, value):
        '''Implementation of set_define().
        Set the define with the given name to the given value. Both `name` and
        `value` can be references to @depends functions, in which case the
        result from these functions is used. If the result of either function
        is None, the define is not set. If the result is False, the define is
        explicitly undefined (-U).
        '''

--disable-skia-gpu builds fine here (USE_SKIA_GPU undefined) but putting my platform on the list of disabled by default is indeed broken (USE_SKIA_GPU is empty). --disable-skia is also broken but fails earlier due to other issues.

Does this patch fix OpenBSD build?
Attachment #8786788 - Flags: feedback?(landry)
Additional small fix to supplement the config fix from Jan Beich. This just make the #ifs be #ifdefs like they're supposed to.
Attachment #8786910 - Flags: review?(mchang)
Attachment #8786910 - Flags: review?(mchang) → review+

Comment 5

3 years ago
mozreview-review
Comment on attachment 8786788 [details]
Bug 1299485 - Unbreak build with Skia disabled by default after bug 1137305.

https://reviewboard.mozilla.org/r/75720/#review73734
Attachment #8786788 - Flags: review?(mh+mozilla) → review+
Reporter

Comment 6

3 years ago
Testing jan's patch locally, but i also think attachment 8786910 [details] [diff] [review] makes sense. Results tmrw..
Reporter

Comment 8

3 years ago
Comment on attachment 8786788 [details]
Bug 1299485 - Unbreak build with Skia disabled by default after bug 1137305.

Patch fixes the build here, thanks!
Attachment #8786788 - Flags: feedback?(landry) → feedback+

Updated

3 years ago
Version: unspecified → 51 Branch

Updated

3 years ago
Keywords: checkin-needed

Comment 9

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed0fd767dcb9
Unbreak build with Skia disabled by default after bug 1137305. r=glandium
Keywords: checkin-needed

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ed0fd767dcb9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.