1319374 - [--disable-skia] fatal error: skia/include/core/SkCanvas.h: No such file or directory

Status: RESOLVED FIXED

Description

[Mike Hommey [:glandium]]

Building with --disable-skia (which is the default when you build on a big-endian architecture (skia doesn't support big-endian)) fails with:

 0:00.64 In file included from gfx/layers/composite/LayerManagerComposite.cpp:14:0,
 0:00.64                  from obj-x86_64-pc-linux-gnu/gfx/layers/Unified_cpp_gfx_layers5.cpp:65:
 0:00.64 gfx/layers/composite/PaintCounter.h:12:40: fatal error: skia/include/core/SkCanvas.h: No such file or directory

Comment 1

[Vincent Liu[:vliu]]

This build break relatives to the feature of Bug 1294121. ni? Mason to figure out the better solution.

Comment 2

[Mason Chang [Inactive] [:mchang]]

Probably just have to add MOZ_ENABLE_SKIA around some things. Do we even support big endian architectures?

Comment 3

[Mason Chang [Inactive] [:mchang]]

Should be USE_SKIA

Comment 4

[Mason Chang [Inactive] [:mchang]]

Created attachment 8815112 [details] [diff] [review]
Wrap PaintCounter with ifdef USE_SKIA

It should build w/o disable-skia now, but it won't work on mac. 

Do we have any options on when we're going to stop enabling gecko builds without skia?

Comment 5

[Mason Chang [Inactive] [:mchang]]

Try looks good - https://treeherder.mozilla.org/#/jobs?repo=try&revision=e22ebe17325c6c34c28e93e72ffa3ae2bdbe64ee

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1e18cb7926e
Fix --disable-skia builds. r=lsalzman

Comment 7

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/c1e18cb7926e

Comment 8

[Gerry Chang [:gchang]]

Too late for 51. Mark 51 won't fix.

Comment 9

[Gerry Chang [:gchang]]

Hi :mchang,
Do you think this is worth uplifting to Aurora52?

Comment 10

[Mason Chang [Inactive] [:mchang]]

(In reply to Gerry Chang [:gchang] from comment #9)
> Hi :mchang,
> Do you think this is worth uplifting to Aurora52?

Not really. We don't officially support any builds without skia. Some platforms won't even work without skia such as OS X.

Comment 11

[Lee Salzman [:lsalzman]]

(In reply to Mason Chang [:mchang] from comment #10)
> (In reply to Gerry Chang [:gchang] from comment #9)
> > Hi :mchang,
> > Do you think this is worth uplifting to Aurora52?
> 
> Not really. We don't officially support any builds without skia. Some
> platforms won't even work without skia such as OS X.

The cutoff for officially retiring non-Skia builds is currently 53 (see bug 1323303). I would at least recommend keeping 52 (for ESR) in whatever quasi-unknown-working state we can. So as annoying as it is, if this breaks the non-Skia builds in 52, I would recommend uplifting this already. That way we can always point people to the ESR if they complain about Skia requirements in 53 onward.

Comment 12

[Lee Salzman [:lsalzman]]

Created attachment 8828461 [details] [diff] [review]
Wrap PaintCounter with ifdef USE_SKIA (52)

The original patch bitrotted a bit against 52. I merely cleaned this up to apply to 52 but made no other changes.

Comment 13

[Lee Salzman [:lsalzman]]

Comment on attachment 8828461 [details] [diff] [review]
Wrap PaintCounter with ifdef USE_SKIA (52)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1294121
[User impact if declined]: Builds with Skia disabled will fail. This is mainly used for tier-3 platforms which we'd like to keep supporting up to the 52 ESR cutoff. With 53 onward we're no longer supporting building without Skia, but part of that plan is to point such users back to 52.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: aurora (52)
[Is the change risky?]: no
[Why is the change risky/not risky?]: Doesn't affect any of our normal builds which always use Skia, and we've already tested this in 52.
[String changes made/needed]: None

Comment 14

[Julien Cristau [:jcristau]]

Comment on attachment 8828461 [details] [diff] [review]
Wrap PaintCounter with ifdef USE_SKIA (52)

fix builds without skia, aurora52+

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/42afdb8f7e6b