1381211 - Clean up SeaMonkeys mozconfig files

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, enhancement)

Description

[Frank-Rainer Grahl (:frg)]

Title says it all

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: 1381211-suite-mozconfigs.patch

I looked at the mozconfigs for the api key patch and found some inconsistencies. 

First I think we could also remove release-10n in all configurations. The cleaned up files are identical to l10n-mozconfig. I didn't do it because I am not sure what breaks in automation if I do. 

Second, is the OSX hack still required?

> # Big Hack that unsets CC / CXX so that mozconfig.common doesn't get
> # mixed up with host/target CPUs when trying to work out how to do the
> # universal build. When we redo the build system (bug 648979) this will
> # go away.
> if test -e "$topsrcdir/suite/config/version.txt"; then
>   unset CC
>   unset CXX
> fi

We no longer do OSX universal builds.

I added the with-official-branding necessary for the suite patch in bug 1354845 here and will remove it from the patch there. It is only needed for the release configs and not the l10n ones as I thought first.

I also added a win64 release config for day x.

This can be further cleaned up There is much overlapping in them which will never individually change.

Comment 2

[Ian Neal]

Comment on attachment 8886769 [details] [diff] [review]
1381211-suite-mozconfigs.patch

r=me assuming the conflicts with Bug 1354845 are removed.

Comment 3

[Edmund Wong (:ewong)]

Comment on attachment 8886769 [details] [diff] [review]
1381211-suite-mozconfigs.patch

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

no.. don't remove the release-l10n.    They are needed for release automation, whereas l10n-mozconfigs are for the 'normal'
l10n stuff.  Should they be the same?  I don't think so as we're probably using options on the normal l10n which we don't
use in the release environment.

All in all, looks good.  Just not particularly sure about the disable-compile-environment part.

I'm giving a 'reserved' r+.  (the l10n are broken... so they can't get anymore broken that his...) :)

::: suite/config/mozconfigs/linux32/nightly
@@ -5,5 @@
>  ac_add_options --disable-webrender
>  
>  ac_add_options --enable-application=suite
>  ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL}
> -ac_add_options --disable-elf-hack

we don't need to do this? is it because we've moved on
to a different gcc (compared to when this was inserted to the mozconfig when we were still on the old linux slaves)?

::: suite/config/mozconfigs/linux64/nightly
@@ -5,5 @@
>  ac_add_options --disable-webrender
>  
>  ac_add_options --enable-application=suite
>  ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL}
> -ac_add_options --disable-elf-hack

we don't need to disable this anymore?

::: suite/config/mozconfigs/macosx64/l10n-mozconfig
@@ -6,5 @@
> -  unset CC
> -  unset CXX
> -fi
> -
> -. $topsrcdir/build/macosx/mozconfig.common

I believe we still need this line.

@@ +7,5 @@
>  ac_add_options --enable-calendar
>  
> +# Disabling is needed, see bug 1345422 comment #58.
> +ac_add_options --disable-compile-environment
> +

I'm not convinced this is the right thing to do, despite TB doing it.  However, since our l10n nightlies are screwed up, we can't test this until after 2.48 and after we fix this repack mess.

::: suite/config/mozconfigs/macosx64/release-l10n
@@ -1,1 @@
> -. "$topsrcdir/build/mozconfig.common"

we need this for automation.

::: suite/config/mozconfigs/win32/l10n-mozconfig
@@ +1,1 @@
>  . "$topsrcdir/build/mozconfig.common"

I suspect we'll also need "$topsrcdir/build/mozconfig.win-common" here as well.

::: suite/config/mozconfigs/win32/release-l10n
@@ +null,0 @@


Please keep this line.

Comment 4

[Frank-Rainer Grahl (:frg)]

Thanks will look at it over the weekend.

Do you think the "Big Hack that unsets CC / CXX" is still needed for OSX?

Should I add bug 1380814 too?

https://hg.mozilla.org/mozilla-central/rev/18aeda1e84fb

Might affect us in the future.

Comment 5

[Frank-Rainer Grahl (:frg)]

> > -ac_add_options --disable-elf-hack
> we don't need to do this? is it because we've moved on
> ..

As far as I see it no longer needed. Both enable-elf-hack and enable-profiling are in the Fx nightly configs via includes. I think it is worth a try and we might be able to enable this in a followup bug. Success is only a backout away :)

See also bug 892355. I think the profiler has been fixed.

Comment 6

[Frank-Rainer Grahl (:frg)]

Attachment: 1381211-suite-mozconfigs-V2.patch

V2 comments from ewong addressed. r+ from IanN carried forward. reserved r+ from ewong too.

> ::: suite/config/mozconfigs/macosx64/release-l10n
> @@ -1,1 @@
> > -. "$topsrcdir/build/mozconfig.common"

mozconfig.common is pulled in via local-mozconfig.common which is pulled in via .$topsrcdir/build/macosx/mozconfig.common if not cross-compiled. 

> Do you think the "Big Hack that unsets CC / CXX" is still needed for OSX?
Answered this to myself here. The current debug config does not contain the hack and builds are produced. So no longer needed in the configs.

If something breaks I will either fix it or back the patch out if this can't be done easy.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/abd557868538
Clean up suite mozconfig files and align them with TB and Fx. r=IanN,ewong

Comment 8

[Frank-Rainer Grahl (:frg)]

I hope this stays closed :)

https://hg.mozilla.org/comm-central/rev/abd55786853812e0f1aed0c286c9bc432d06da2a

Comment 10

[Tristan Miller]

Possibly related: Bug 1619776