Closed Bug 1328886 Opened 3 years ago Closed 2 years ago

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

Categories

(SeaMonkey :: Release Engineering, defect)

SeaMonkey 2.48 Branch
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ewong, Assigned: ewong)

References

Details

Attachments

(3 files, 3 obsolete files)

With the possibility of doing a beta release next, 
we need to get a lot of the 'bad' mozconfig stuff
out of the way.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #8824083 - Flags: review?(philip.chee)
Comment on attachment 8824083 [details] [diff] [review]
[mozconfigs] patch

I assume this applies to comm-beta? r=me a=me
Attachment #8824083 - Flags: review?(philip.chee)
Attachment #8824083 - Flags: review+
Attachment #8824083 - Flags: approval-comm-beta+
Should c-b be without gtk3?
Flags: needinfo?(frgrahl)
Flags: needinfo?(antoine.mechelynck)
Personally I would say yes unless someone has time to check out gtk3. Should be decided in tomorrows status meeting.
Flags: needinfo?(frgrahl)
(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 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
Attachment #8824083 - Flags: approval-comm-aurora?
This is similar to the c-b mozconfig patch minus the macosx-universal/release-l10n.  (this is going to be fixed in bug 1329379)
Attachment #8825657 - Flags: review?(philip.chee)
Attachment #8825657 - Flags: review?(philip.chee) → review+
https://hg.mozilla.org/comm-central/rev/433bc33a6ed30e0f8fee6a817b6d82b756463dd7
Bug 1328886 - Remove no_tooltool=1 from release mozconfigs. r+a=RattyAway
Attachment #8824083 - Flags: approval-comm-aurora?
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
Attachment #8825657 - Flags: approval-comm-aurora?
(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?
Flags: needinfo?(bugspam.Callek)
I think we should really try to have gtk3 enabled. Do we know of anything specific that breaks with gtk3 in SM?
> 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.
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).
gtk2 Support has been removed in Bug 1278282 for Gecko 53. SeaMonkey 2.50 will need to enable gtk3
(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.
Ok, so the main question is what to do with the 2.49 releases if and when moving to the ESR branch.
(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.
>>
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.
(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).
(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.
Flags: needinfo?(antoine.mechelynck)
Ok.  Given everyone's comments, 2.48 will be the last beta/release to have GTK2.
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.
Attachment #8827312 - Flags: review?(rkent)
Attachment #8827312 - Flags: review?(philip.chee)
(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?
(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 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?
Attachment #8827312 - Flags: review?(philip.chee) → review+
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.
Attachment #8827312 - Flags: review?(rkent) → review?(jorgk)
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.
Attachment #8827312 - Flags: review?(jorgk) → review+
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 ;-)
(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?
Attachment #8829063 - Flags: review?(mh+mozilla)
Attachment #8829063 - Attachment is obsolete: true
Attachment #8829063 - Flags: review?(mh+mozilla)
Attachment #8829065 - Flags: review?(mh+mozilla)
(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?
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 ;-)
(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 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?
Attachment #8829065 - Flags: review?(mh+mozilla)
With comment #36, I believe this is the next best thing; though I don't
know the policies of adding to check-sync-exceptions.
Attachment #8827312 - Attachment is obsolete: true
Attachment #8829065 - Attachment is obsolete: true
Attachment #8829767 - Flags: review?(philip.chee)
Attachment #8829767 - Flags: review?(jorgk)
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?
Attachment #8829767 - Flags: review?(jorgk) → review+
(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.
Ah, I learn something new every day ;-) Thanks.
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.
s/like/line/
r+ from me for that approach, too ;-)
Comment on attachment 8829767 [details] [diff] [review]
[mozconfigs] use GTK2 if USE_GTK2 is set. (v5)

rs=me.
Attachment #8829767 - Flags: review?(philip.chee) → review+
Blocks: SM2.48b1
Depends on: 1338958
Blocks: SM2.48
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Flags: needinfo?(bugspam.Callek)
Attachment #8825657 - Flags: approval-comm-aurora?
You need to log in before you can comment on or make changes to this bug.