1328886 - [Tracking SM-beta] prepare the c-b tree for the next beta release.

Status: RESOLVED FIXED

(SeaMonkey :: Release Engineering, defect)

Description

[Edmund Wong (:ewong)]

With the possibility of doing a beta release next, 
we need to get a lot of the 'bad' mozconfig stuff
out of the way.

Comment 1

[Edmund Wong (:ewong)]

Attachment: [mozconfigs] patch

Comment 2

[Philip Chee]

Comment on attachment 8824083 [details] [diff] [review]
[mozconfigs] patch

I assume this applies to comm-beta? r=me a=me

Comment 3

[Edmund Wong (:ewong)]

https://hg.mozilla.org/releases/comm-beta/rev/354302165ac64508b55d88063faef669382c0e5a
Bug 1328886 - Clean up mozconfigs for beta release. r+a=RattyAway

Comment 4

[Edmund Wong (:ewong)]

Should c-b be without gtk3?

Comment 5

[Frank-Rainer Grahl (:frg)]

Personally I would say yes unless someone has time to check out gtk3. Should be decided in tomorrows status meeting.

Comment 6

[Edmund Wong (:ewong)]

(In reply to Edmund Wong (:ewong) from comment #3)
> https://hg.mozilla.org/releases/comm-beta/rev/
> 354302165ac64508b55d88063faef669382c0e5a
> Bug 1328886 - Clean up mozconfigs for beta release. r+a=RattyAway

This needs to be also applied to c-c, and c-a.  Since we're not releasing
2.47,  c-b will become c-r.

Comment 7

[Edmund Wong (:ewong)]

Comment on attachment 8824083 [details] [diff] [review]
[mozconfigs] patch

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: none
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):  Release builds will break.
String changes made by this patch: none

Comment 8

[Edmund Wong (:ewong)]

Attachment: [mozconfigs] remove no_tooltool=1 from release configs.

This is similar to the c-b mozconfig patch minus the macosx-universal/release-l10n.  (this is going to be fixed in bug 1329379)

Comment 9

[Edmund Wong (:ewong)]

https://hg.mozilla.org/comm-central/rev/433bc33a6ed30e0f8fee6a817b6d82b756463dd7
Bug 1328886 - Remove no_tooltool=1 from release mozconfigs. r+a=RattyAway

Comment 10

[Edmund Wong (:ewong)]

Comment on attachment 8825657 [details] [diff] [review]
[mozconfigs] remove no_tooltool=1 from release configs.

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: Release build/repack will fail.
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch: None

Comment 11

[Edmund Wong (:ewong)]

(In reply to Frank-Rainer Grahl from comment #5)
> Personally I would say yes unless someone has time to check out gtk3. Should
> be decided in tomorrows status meeting.

Fwiw, in order to disable gtk3, we need to change c-b's build/unix/mozconfig.gtk
as well as m-b's build/unix/mozconfig.gtk.  The problem with this is that
we need to push to both trees (before the said trees are tagged) and this
would affect m-b's tree.  

If we do disable gtk3, I think we'll need to 'sacrifice' build 1 to a bustage,
and then push the required disable-gtk3 changes to the relbranch, which
won't affect c-b and m-b proper.  Is this the right approach, Callek?

Comment 12

[Robert Kaiser]

I think we should really try to have gtk3 enabled. Do we know of anything specific that breaks with gtk3 in SM?

Comment 13

[Frank-Rainer Grahl (:frg)]

> I think we should really try to have gtk3 enabled.
> Do we know of anything specific that breaks with gtk3 in SM?

Mostly theme bugs and incompatibilities with 3.20+ I think e.g. Scrollbar problems. A gtk3 SeaMonkey on my CentOS7 with KDE desktop will just abort and not even start because of a base OS theme bug. Firefox is the same. Rsx11m should know more.

Personally I would still build 2.48 with gtk2 to not add any new bugs to the release. Then switch 2.49ESR over to gtk3 and iron it out. If SeaMonkey ( and Firefox<g> ) are still in business after the ESR52 release cycle it should be ok then. Suspect gtk2 will be gone then too.

Comment 14

[rsx11m]

Yepp, no scrollbars and menu rendering sucks with GTK3 on some desktop themes. The 2.46 GTK2 builds work fine for me (except for a focus-ring issue which seems to be specific to the OpenSUSE 13.2 desktop theme). For reference, OpenSUSE still builds even current Firefox releases with GTK2 only.

The other issue is still supported long-term distros. GTK3 builds don't run on SLES 11 for me, possibly on other LTS distros as well. Thus, we'd implicitly EOL SeaMonkey for those unless the distro steps in with their own builds.

While I see that GTK2 isn't unproblematic on the long run either, I'd agree with FRG to stick with it for a while longer until we are confident that remaining issues are negligible (for 2.49ESR we'd have to make the pick, given that switching over on the fly may not be possible or advised).

Comment 15

[Frank-Rainer Grahl (:frg)]

gtk2 Support has been removed in Bug 1278282 for Gecko 53. SeaMonkey 2.50 will need to enable gtk3

Comment 16

[Ian Neal]

(In reply to Frank-Rainer Grahl from comment #15)
> gtk2 Support has been removed in Bug 1278282 for Gecko 53. SeaMonkey 2.50
> will need to enable gtk3

So far it is just the ability to build rather than any code removal (so easy to back out), but I suspect that for SeaMonkey 2.51 (Gecko 54) we will have no choice.

Comment 17

[rsx11m]

Ok, so the main question is what to do with the 2.49 releases if and when moving to the ESR branch.

Comment 18

[Robert Kaiser]

(In reply to Frank-Rainer Grahl from comment #13)
> Mostly theme bugs and incompatibilities with 3.20+ I think e.g. Scrollbar
> problems. A gtk3 SeaMonkey on my CentOS7 with KDE desktop will just abort
> and not even start because of a base OS theme bug. Firefox is the same.
> Rsx11m should know more.

If Firefox is the same, you should repair your system but it should not impact how we release SeaMonkey.

> Personally I would still build 2.48 with gtk2 to not add any new bugs to the
> release.

Given that gtk2 is unsupported by Firefox now, I'm pretty sure turning off gtk3 will introduce a number of bugs that nobody in the platform teams cares about. I don't support that.

Comment 19

[Frank-Rainer Grahl (:frg)]

>>
If Firefox is the same, you should repair your system but it should not impact how we release SeaMonkey.
<<

Known bug with KDE and gtk theme integration which is fixed in almost all distributions but centos 7 not it appears. Switched to another theme which worked.

Comment 20

[rsx11m]

(In reply to Robert Kaiser from comment #18)
> Given that gtk2 is unsupported by Firefox now, I'm pretty sure turning off
> gtk3 will introduce a number of bugs that nobody in the platform teams cares
> about. I don't support that.

Well, yeah - maybe. With gtk3, we know that there are issues and likely more of them where our themes don't cover the respective items (whatever they may be), thus a bunch of new bugs would likely show up here as well. While you are right that the gtk2 code isn't effectively maintained any more by the Firefox people, at least it's what we've used up to now (and still seems to work for 2.46, given that I don't see any issues reported in MZ three weeks into the release).

Comment 21

[Tony Mechelynck [:tonymec]]

(In reply to Frank-Rainer Grahl from comment #15)
> gtk2 Support has been removed in Bug 1278282 for Gecko 53. SeaMonkey 2.50
> will need to enable gtk3

Current Linux trunk nightlies and hourlies have GTK3 enabled and AFAICT they work reasonably well.

(In reply to Edmund Wong (:ewong) from comment #4)
> Should c-b be without gtk3?

No opinion; but see KaiRo's reply, comment #12, and frg's answer, comment #13.

Comment 22

[Edmund Wong (:ewong)]

Ok.  Given everyone's comments, 2.48 will be the last beta/release to have GTK2.

Comment 23

[Edmund Wong (:ewong)]

Attachment: [mozconfigs] use GTK2 if USE_GTK2 is set.

Since USE_GTK2 isn't set in TB's configs, it should go directly to using GTK3.  For suite's linux* mozconfigs, setting USE_GTK2 will allow us to use GTK2 
instead.

Since this touches c-c's build/, and it does affect TB's builds, I'm asking :rkent for review.

Comment 24

[rsx11m]

(In reply to Edmund Wong (:ewong) from comment #23)
> Since this touches c-c's build/, and it does affect TB's builds, I'm asking :rkent for review.

Given mozilla-central changeset 529160328d96, shouldn't this apply up to c-a only at most = not c-c?

Comment 25

[Edmund Wong (:ewong)]

(In reply to rsx11m from comment #24)
> (In reply to Edmund Wong (:ewong) from comment #23)
> > Since this touches c-c's build/, and it does affect TB's builds, I'm asking :rkent for review.
> 
> Given mozilla-central changeset 529160328d96, shouldn't this apply up to c-a
> only at most = not c-c?

I'm stupid.   What I meant by c-c was in the sense of 'comm-central' trees; but 
should have been specific with comm-beta, which is what this patch was based
on.

Comment 26

[Philip Chee]

Comment on attachment 8827312 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set.

r=me for the SeaMonkey part.

> +if [ -z "${USE_GTK2}" ]; then
> +
>  TOOLTOOL_DIR=${TOOLTOOL_DIR:-$topsrcdir}
Hmm. Don't you need to indent the new if block?

Comment 27

[Kent James (:rkent)]

Comment on attachment 8827312 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set.

Since Jorg is managing the tree for the Thunderbird 52 release, I'm redirecting the review to him.

As for me, I know little about this, my opinion is that Thunderbird should be using gtk3 if Firefox is. I know we disabled it at some point, not sure of the current status.

Comment 28

[Jorg K (CEST = GMT+2)]

Comment on attachment 8827312 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set.

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

Nitpicker at work ;-)
I've been (unjustly) accused of putting too much emphasis on comments ;-(

::: build/unix/mozconfig.gtk
@@ +1,3 @@
>  # To do try builds with Gtk+2, uncomment the following line, and remove
>  # everything after that.
>  #ac_add_options --enable-default-toolkit=cairo-gtk2

Please update this comment:
To do builds with GTK2, set USE_GTK2, or words to that effect.

@@ +1,5 @@
>  # To do try builds with Gtk+2, uncomment the following line, and remove
>  # everything after that.
>  #ac_add_options --enable-default-toolkit=cairo-gtk2
>  
> +if [ -z "${USE_GTK2}" ]; then

Please add comment above:
SM still uses GTK2, or words to that effect.

Comment 29

[Jorg K (CEST = GMT+2)]

And please adhere to the new landing policy: When landing "small things" on C-C, it's best to land your changes after an M-C merge to trigger a build. Otherwise, add "checkin-needed" and we'll take care of it. Or just ignore the policy and land it anyway ;-)

Comment 30

[Edmund Wong (:ewong)]

(In reply to Jorg K (GMT+1) from comment #29)
> And please adhere to the new landing policy: When landing "small things" on
> C-C, it's best to land your changes after an M-C merge to trigger a build.
> Otherwise, add "checkin-needed" and we'll take care of it. Or just ignore
> the policy and land it anyway ;-)

Hi Jorg, 

wrt to the build/unix/mozconfig.gtk change, we need to ensure m-b's
build/unix/mozconfig.gtk is the same right?

Comment 31

[Edmund Wong (:ewong)]

Attachment: [mozconfigs] build GTK2 if USE_GTK2 set.

Comment 32

[Edmund Wong (:ewong)]

Attachment: [mozconfigs] (m-b) do GTK2 build when env.var USE_GTK2 is set.

Comment 33

[Edmund Wong (:ewong)]

(In reply to Jorg K (GMT+1) from comment #28)
> Comment on attachment 8827312 [details] [diff] [review]
> [mozconfigs] use GTK2 if USE_GTK2 is set.
> 
> Review of attachment 8827312 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nitpicker at work ;-)
> I've been (unjustly) accused of putting too much emphasis on comments ;-(
> 
> ::: build/unix/mozconfig.gtk
> @@ +1,3 @@
> >  # To do try builds with Gtk+2, uncomment the following line, and remove
> >  # everything after that.
> >  #ac_add_options --enable-default-toolkit=cairo-gtk2
> 
> Please update this comment:
> To do builds with GTK2, set USE_GTK2, or words to that effect.
> 
> @@ +1,5 @@
> >  # To do try builds with Gtk+2, uncomment the following line, and remove
> >  # everything after that.
> >  #ac_add_options --enable-default-toolkit=cairo-gtk2
> >  
> > +if [ -z "${USE_GTK2}" ]; then
> 
> Please add comment above:
> SM still uses GTK2, or words to that effect.

With adding SM still uses GTK2,  I'd also need to have that reflected in
m-b's build/unix/mozconfig.gtk, right?

Comment 34

[Jorg K (CEST = GMT+2)]

Hi Edmund, I'm really not a build guy. I looked at the part affecting TB in attachment 8827312 [details] [diff] [review] and it was fine apart from improving some comments.

I don't know what the uplift situation is, but you can assume a blanket r+ and a+ for any uplifts you want to do with this change.

Now, as for attachment 8829065 [details] [diff] [review], is this the M-C/M-B file
comm-central/mozilla/build/unix/mozconfig.gtk
or the C-C/C-B file
comm-central/build/unix/mozconfig.gtk ?

If I'm not mistaken, they are both the same so far. Again, I'm not a build guy to know what's required, and again, if it were the C-B file, my blanket r+/a+ would apply ;-)

Comment 35

[Edmund Wong (:ewong)]

(In reply to Jorg K (GMT+1) from comment #34)
> Hi Edmund, I'm really not a build guy. I looked at the part affecting TB in
> attachment 8827312 [details] [diff] [review] and it was fine apart from
> improving some comments.
> 
> I don't know what the uplift situation is, but you can assume a blanket r+
> and a+ for any uplifts you want to do with this change.
> 
> Now, as for attachment 8829065 [details] [diff] [review], is this the
> M-C/M-B file
> comm-central/mozilla/build/unix/mozconfig.gtk
> or the C-C/C-B file
> comm-central/build/unix/mozconfig.gtk ?

That is the m-b file.

> 
> If I'm not mistaken, they are both the same so far. Again, I'm not a build
> guy to know what's required, and again, if it were the C-B file, my blanket
> r+/a+ would apply ;-)

They are the same because (iirc) aleth and a few others have worked on to
keep the c-c version synchronized with m-*.  While I've seen the synchronizing
from m-* to c-*,  I don't think I've seen any c-* -> m-* which is why 
I'm asking glandium a review.

Comment 36

[Mike Hommey [:glandium]]

Comment on attachment 8829065 [details] [diff] [review]
[mozconfigs] (m-b) do GTK2 build when env.var USE_GTK2 is set.

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

I'd rather avoid doing something like this. Can't you make an exception for mozconfig.gtk in the script that ensures the files are the same as in m-c?

Comment 37

[Edmund Wong (:ewong)]

Attachment: [mozconfigs] use GTK2 if USE_GTK2 is set. (v5)

With comment #36, I believe this is the next best thing; though I don't
know the policies of adding to check-sync-exceptions.

Comment 38

[Jorg K (CEST = GMT+2)]

Comment on attachment 8829767 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set. (v5)

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

Anything is fine be me as long as you don't break TB ;-)

::: build/check-sync-exceptions
@@ +4,5 @@
>  pymake
>  client.py-args
>  client.py-l10n-args
>  configobj.py
> +mozconfig.gtk

Can you enlighten me one what this does?

Comment 39

[Edmund Wong (:ewong)]

(In reply to Jorg K (GMT+1) from comment #38)
> Comment on attachment 8829767 [details] [diff] [review]
> [mozconfigs] use GTK2 if USE_GTK2 is set. (v5)
> 
> Review of attachment 8829767 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Anything is fine be me as long as you don't break TB ;-)
> 
> ::: build/check-sync-exceptions
> @@ +4,5 @@
> >  pymake
> >  client.py-args
> >  client.py-l10n-args
> >  configobj.py
> > +mozconfig.gtk
> 
> Can you enlighten me one what this does?

Assuming you mean this file, the aforementioned files are ignored
when doing the check-sync during one of the build steps.  If these files
aren't mentioned, any changes in mozilla-*/build  will need
to be ported/synced with comm-*/build.  Otherwise, the tree
goes red.

Comment 40

[Jorg K (CEST = GMT+2)]

Ah, I learn something new every day ;-) Thanks.

Comment 41

[Mike Hommey [:glandium]]

Comment on attachment 8829767 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set. (v5)

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

::: build/unix/mozconfig.gtk
@@ -1,3 @@
> -# To do try builds with Gtk+2, uncomment the following line, and remove
> -# everything after that.
> -#ac_add_options --enable-default-toolkit=cairo-gtk2

You could uncomment this like, remove everything else in the file, and not have to modify any other file except check-sync-exceptions.

Comment 42

[Mike Hommey [:glandium]]

s/like/line/

Comment 43

[Jorg K (CEST = GMT+2)]

r+ from me for that approach, too ;-)

Comment 44

[Philip Chee]

Comment on attachment 8829767 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set. (v5)

rs=me.