1186748 - Switch all builds to Gtk+3

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Bug 1186003 didn't enable Gtk+3 on *all* builds, and landing bug 1186229 broke several builds as a consequence. The fixup was to manually switch to Gtk+2 in the relevant mozconfigs. This bug is about actually switching those builds to Gtk+3.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Move gtk-related things in a separate mozconfig

Some mozconfigs don't include mozconfig.linux*, and don't get gtk-related
definitions, so move them in a separate mozconfig. To avoid having two
files, one for 32-bit builds and one for 64-bit builds, rely on the
includer to set PKG_CONFIG_LIBDIR appropriately.

At the same time, move all the --enable-default-toolkit=cairo-gtk2 in that
new file in the case the gtk3 package wasn't pulled from tooltool.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Enable Gtk+3 on all build jobs except valgrind

valgrind builds will be dealt with later, they require more work still.

Comment 3

[Michael Shal [:mshal]]

Comment on attachment 8637863 [details] [diff] [review]
Enable Gtk+3 on all build jobs except valgrind

>--- a/browser/config/mozconfigs/linux64/hazards
>+++ b/browser/config/mozconfigs/linux64/hazards
>+TOOLTOOL_DIR="$(dirname $topsrcdir)"

What is TOOLTOOL_DIR for? I don't see it used anywhere - was it left in accidentally?

Comment 4

[Mike Hommey [:glandium]]

(In reply to Michael Shal [:mshal] from comment #3)
> Comment on attachment 8637863 [details] [diff] [review]
> Enable Gtk+3 on all build jobs except valgrind
> 
> >--- a/browser/config/mozconfigs/linux64/hazards
> >+++ b/browser/config/mozconfigs/linux64/hazards
> >+TOOLTOOL_DIR="$(dirname $topsrcdir)"
> 
> What is TOOLTOOL_DIR for? I don't see it used anywhere - was it left in
> accidentally?

No, I forgot a part of the change when I moved a part to the patch for valgrind builds that are not covered here.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Enable Gtk+3 on all build jobs except valgrind

Comment 6

[Michael Shal [:mshal]]

Comment on attachment 8638134 [details] [diff] [review]
Enable Gtk+3 on all build jobs except valgrind

Can we get rid of TOOLTOOL_DIR if hazard_build.py ran tooltool in the source directory instead of abs_work_dir? No need to block on that, but if that's reasonable we can file a followup.

Comment 7

[Mike Hommey [:glandium]]

I'm not the right person to ask. That would be a question for sfink, I guess.

Comment 8

[u43589]

Will gtk2 still be "supported" or will that code be removed?

Comment 9

[Mike Hommey [:glandium]]

Attachment: Move gtk-related things in a separate mozconfig

Carrying over r+. This rebases the previous patch on top of bug 1187245

Comment 10

[Mike Hommey [:glandium]]

Attachment: Enable Gtk+3 on all build jobs

Like the previous, but covers more platforms, including valgrind. Note this may be landed in smaller parts, so that all platforms that are currently completely green can be turned to Gtk+3 without waiting for the more problematic ones.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Now that all builds are pulling the Gtk+3 tooltool package, remove the Gtk+2 fallback in mozconfig.gtk

This is for when the "Enable Gtk+3 on all build jobs" part is fully landed, at which point we don't need to fall back to --enable-default-toolkit=cairo-gtk2.

Comment 12

[Michael Shal [:mshal]]

Comment on attachment 8638972 [details] [diff] [review]
Now that all builds are pulling the Gtk+3 tooltool package, remove the Gtk+2 fallback in mozconfig.gtk

> # $TOOLTOOL_DIR/gtk3 comes from tooltool, when the tooltool manifest contains it.

Now that all tooltool manifests contain it, maybe "$TOOLTOOL_DIR/gtk3 comes from tooltool, which must be included in the tooltool manifest" or some such?

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f212df3de6f

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6f212df3de6f

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ada6cd4da281

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c0064f00a27

Comment 17

[Mike Hommey [:glandium]]

Comment on attachment 8638971 [details] [diff] [review]
Enable Gtk+3 on all build jobs

This patch was landed partially. I'll attach the remaining parts as separate patches.

Comment 18

[Mike Hommey [:glandium]]

Attachment: Switch valgrind builds to Gtk+3. r=mshal

This is rebased on top of bug 1188780

Comment 19

[Mike Hommey [:glandium]]

Attachment: Switch mulet builds to Gtk+3

This is rebased on top of bug 1188780

Comment 20

[Mike Hommey [:glandium]]

Attachment: Now that all builds are pulling the Gtk+3 tooltool package, remove the Gtk+2 fallback in mozconfig.gtk

Rebased as well.

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ada6cd4da281
https://hg.mozilla.org/mozilla-central/rev/9c0064f00a27

Comment 22

[Ryan VanderMeulen [:RyanVM]]

(In reply to Carsten Book [:Tomcat] from comment #21)
> https://hg.mozilla.org/mozilla-central/rev/ada6cd4da281

Backed out for causing various intermittent ASAN mochitest failures.
https://treeherder.mozilla.org/logviewer.html#?job_id=12304513&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=12304517&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=12302907&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=12295749&repo=mozilla-inbound

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

Comment 23

[Mike Hommey [:glandium]]

Can you give more details wrt the decision to backout because all I see is oranges to possibly file here? Also note that the current state of non-ASan is to use Gtk+3, and that ASan builds not covering that is kind of a problem.

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d093c57c6835
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8eacfe11fc5

Comment 25

[Mike Hommey [:glandium]]

Attachment: Switch ASan builds to Gtk+3

Didn't reland ASan waiting for an answer to comment 23.

This is the part switching for ASan, rebased on top of bug 1188780.

Comment 26

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebca2b984bd4

Comment 27

[Ryan VanderMeulen [:RyanVM]]

(In reply to Mike Hommey [:glandium] from comment #23)
> Can you give more details wrt the decision to backout because all I see is
> oranges to possibly file here? Also note that the current state of non-ASan
> is to use Gtk+3, and that ASan builds not covering that is kind of a problem.

They were frequent enough that they warranted backout, which is a right we've always reserved. Sorry that it's inconvenient to you.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Note that we've been seeing similar screentopng errors on regular Linux builds since the GTK3 switch was made, so there appears to be a real regression there. That breaks screenshotting after mochitest failures, i.e.

https://treeherder.mozilla.org/logviewer.html#?job_id=1926821&repo=mozilla-central - which yields "04:42:03 INFO - Failed to start /builds/slave/test/build/tests/bin/screentopng for screenshot: No such file or directory"

Which then yields http://mozilla-releng-blobs.s3.amazonaws.com/blobs/mozilla-central/sha512/cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e

Comment 29

[Mike Hommey [:glandium]]

The screentopng thing is bug 1189526

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d093c57c6835
https://hg.mozilla.org/mozilla-central/rev/b8eacfe11fc5
https://hg.mozilla.org/mozilla-central/rev/ebca2b984bd4

Comment 31

[Pulsebot]

Backout:
https://hg.mozilla.org/mozilla-central/rev/aca6bb843bed
https://hg.mozilla.org/mozilla-central/rev/c4f162861c86

Comment 32

[Carsten Book [:Tomcat]]

had to back this out again for causing Bug 1190180 / Bug 1190317 or depend on the backout of https://hg.mozilla.org/mozilla-central/rev/d093c57c6835

Comment 33

[Julien Wajsberg [:julienw]]

I think the backout didn't actually fix the issue we saw.

Comment 34

[Mike Hommey [:glandium]]

So, after being backed out with barely an explanation, now, it's backed out without even being able to find those jobs that are failing, because neither mentioned bug gives treeherder links, and those jobs don't seem to appear on treeherder...

Also, thank you for backing out the whole thing, instead of just the relevant part...

</frustration>

Comment 35

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d43dd3837256
https://hg.mozilla.org/integration/mozilla-inbound/rev/d87145bd6adf

Comment 36

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/48a62f1685e1

Comment 37

[Mike Hommey [:glandium]]

Attachment: Switch B2G desktop builds to Gtk+3

And the not landed B2G desktop part, rebased on bug 1188780

Comment 38

[Julien Wajsberg [:julienw]]

(In reply to Mike Hommey [:glandium] from comment #34)
> So, after being backed out with barely an explanation, now, it's backed out
> without even being able to find those jobs that are failing, because neither
> mentioned bug gives treeherder links, and those jobs don't seem to appear on
> treeherder...


yeah, it's not _really_ easy to move back from a log to a treeherder job.
With the task name (for example h1dtMu9PRNacSNkGu2PzfA) you can find the task with taskcluster's task inspector; from there you have some information in https://tools.taskcluster.net/task-inspector/#h1dtMu9PRNacSNkGu2PzfA/.

But I agree it would have been easier with all the information in the first place, sorry for this.

> 
> Also, thank you for backing out the whole thing, instead of just the
> relevant part...
> 

Just FYI I know Carsten tried to backout only the relevant part, but there was some conflicts and he thought it would be less error-prone to backout more things without conflicts.

Comment 39

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d43dd3837256
https://hg.mozilla.org/mozilla-central/rev/d87145bd6adf
https://hg.mozilla.org/mozilla-central/rev/48a62f1685e1

Comment 40

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a6cee238cad

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08cdf088f429

Comment 42

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0a6cee238cad
https://hg.mozilla.org/mozilla-central/rev/08cdf088f429

Comment 43

[Neon]

On my CentOS 6.6 64-bit machine, I am no longer able to run lastest Firefox Dev Edition (42.0a2), since it does not have GTK3. Are you guys dropping support for CentOS 6?

XPCOMGlueLoad error for file /data/apps/firefox-dev-ed/libmozgtk.so:
libgtk-3.so.0: cannot open shared object file: No such file or directory
Couldn't load XPCOM.

ldd libmozgtk.so
	linux-vdso.so.1 =>  (0x00007fff20509000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fefd7622000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007fefd741d000)
	libgtk-3.so.0 => not found
	libgdk-3.so.0 => not found
	libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x00007fefd7116000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fefd6d82000)
	/lib64/ld-linux-x86-64.so.2 (0x0000003985200000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fefd6afe000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fefd68e7000)

Comment 44

[Mike Hommey [:glandium]]

(In reply to Neon from comment #43)
> On my CentOS 6.6 64-bit machine, I am no longer able to run lastest Firefox
> Dev Edition (42.0a2), since it does not have GTK3. Are you guys dropping
> support for CentOS 6?

In practice, yes. But I'd expect CentOS 6 to have its own firefox packages with GTK2 support.

Comment 45

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f37c01f7c064

Comment 46

[Carsten Book [:Tomcat]]

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

Comment 47

[Mike Hommey [:glandium]]

Attachment: Make ASAN builds happy with Gtk+3

Flagging Andrew for the LSAN suppression (which is something that would have been added when LSAN was enabled had Gtk+3 been enabled for ASAN back then), and Bobby for the e10s mochitest skip (because running ASAN on the same thing we're planning to ship is more important than a single test that intermittently fails only on ASAN (I thought it failed on normal builds too, but it looks like it doesn't, or at a _very_ low rate))

Comment 48

[Andrew McCreight [:mccr8]]

Comment on attachment 8682327 [details] [diff] [review]
Make ASAN builds happy with Gtk+3

Review of attachment 8682327 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for the suppression changes.

::: build/sanitizers/lsan_suppressions.txt
@@ +115,5 @@
>  leak:libstdc++.so
>  leak:libXrandr.so
>  leak:pthread_setspecific_internal
>  leak:swrast_dri.so
> +leak:libpixman-1.so

Please insert this earlier so that this list at the bottom remains in alphabetical order.

Comment 49

[Bobby Holley (:bholley)]

Comment on attachment 8682327 [details] [diff] [review]
Make ASAN builds happy with Gtk+3

Review of attachment 8682327 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 50

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3a831ebbcfa
https://hg.mozilla.org/integration/mozilla-inbound/rev/38fe6c419f08
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7844fe50165

Comment 51

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f3a831ebbcfa
https://hg.mozilla.org/mozilla-central/rev/38fe6c419f08
https://hg.mozilla.org/mozilla-central/rev/d7844fe50165

Comment 52

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/f3a831ebbcfa
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/38fe6c419f08
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/d7844fe50165

Comment 53

[Nicholas Nethercote [inactive]]

There are various blocks like this in the code:

> #if (MOZ_WIDGET_GTK == 2)
> ...
> #else
> ...
> #endif

And also ones with |MOZ_WIDGET_GTK == 3|. Are these removable now, or soon? I.e. when can we drop support for GTK 2?

Comment 54

[Martin Stránský [:stransky] (ni? me)]

(In reply to Nicholas Nethercote [:njn] from comment #53)
> There are various blocks like this in the code:
> 
> > #if (MOZ_WIDGET_GTK == 2)
> > ...
> > #else
> > ...
> > #endif
> 
> And also ones with |MOZ_WIDGET_GTK == 3|. Are these removable now, or soon?
> I.e. when can we drop support for GTK 2?

Please don't remove support for Gtk2. We (Red Hat) need to ship Firefox on non-Gtk3 systems like RHEL5, 6, CentOS 6 and so. I volunteer to maintain this gtk2 target.

Comment 55

[Mike Hommey [:glandium]]

cf. Martin's answer.

Comment 56

[Steve Fink [:sfink] [:s:]]

(In reply to Michael Shal [:mshal] from comment #6)
> Comment on attachment 8638134 [details] [diff] [review]
> Enable Gtk+3 on all build jobs except valgrind
> 
> Can we get rid of TOOLTOOL_DIR if hazard_build.py ran tooltool in the source
> directory instead of abs_work_dir? No need to block on that, but if that's
> reasonable we can file a followup.

I've always hated where tooltool runs from, but changing it broke a bunch of stuff. I am working on retiring all mozharness hazard builds in bug 1250709, in favor of shell script driven taskcluster builds, so this will shortly become irrelevant.