I also added ANDROID_NDK_VERSION, ANDROID_NDK_VERSION_32BIT and ANDROID_SDK_VERSION such that these can be included in a local mozconfig and any changes will be picked up.

Is it intended that developers include the "common" mozconfig in their local mozconfigs? If so then changes to it should be severely restricted as they can have unintended consequences and/or break stuff. See for example https://bugzilla.mozilla.org/show_bug.cgi?id=892355#c10 and subsequent comments. If not then I should stop including that common mozconfig in my local one.

Comment on attachment 777081 [details] [diff] [review] patch Review of attachment 777081 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the following fix ::: mobile/android/config/mozconfigs/android-armv6/nightly @@ -22,5 @@ > - > -export JAVA_HOME=/tools/jdk6 > -export MOZILLA_OFFICIAL=1 > -export MOZ_TELEMETRY_REPORTING=1 > -export MOZ_PKG_SPECIAL=armv6 MOZ_PKG_SPECIAL should not be removed.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1) > Is it intended that developers include the "common" mozconfig in their local > mozconfigs? If so then changes to it should be severely restricted as they > can have unintended consequences and/or break stuff. See for example > https://bugzilla.mozilla.org/show_bug.cgi?id=892355#c10 and subsequent > comments. If not then I should stop including that common mozconfig in my > local one. In-tree mozconfigs are not for developer consumption.

Comment on attachment 777081 [details] [diff] [review] patch Review of attachment 777081 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the following fix ::: mobile/android/config/mozconfigs/android-armv6/nightly @@ -22,5 @@ > - > -export JAVA_HOME=/tools/jdk6 > -export MOZILLA_OFFICIAL=1 > -export MOZ_TELEMETRY_REPORTING=1 > -export MOZ_PKG_SPECIAL=armv6 MOZ_PKG_SPECIAL should not be removed. ::: mobile/android/config/mozconfigs/common @@ +32,5 @@ > +fi > + > +ac_add_options --with-android-gnu-compiler-version=4.7 > +ac_add_options --with-android-version=9 > +ac_add_options --with-system-zlib Note: none of these three are actually required. The build system will use these values on its own.

Comment on attachment 777081 [details] [diff] [review] patch Review of attachment 777081 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/config/mozconfigs/common @@ +35,5 @@ > +ac_add_options --with-android-version=9 > +ac_add_options --with-system-zlib > +ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL} > +ac_add_options --enable-profiling > +ac_add_options --disable-elf-hack # --enable-elf-hack conflicts with --enable-profiling Isn't that in conflict with the --enable-elf-hack above this chunk?

This patch appears to have regressed a startup resident memory usage improvement that glandium landed not too long ago. The improvement (~4.5MB) came from this range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=285c7cb20fc2&tochange=f4a2508ab57c And the regression came from this range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=843d948e138b&tochange=8e672b39183d The most likely culprit as far as I can see is that bug 892355 enabled elf-hack on armv6 builds (which is what AWSY runs on) and then this bug disabled it again (on all android builds). See also comment 5. I don't know what the intention here was - to keep or disable elf-hacking and on which builds?

This enabled profiling on armv6 builds, which needs elf-hack disabled. Eventually, both won't conflict (bug 788974), but that currently doesn't work. This only affects nightlies, not release builds.

(In reply to Mike Hommey [:glandium] from comment #8) > This enabled profiling on armv6 builds, which needs elf-hack disabled. > Eventually, both won't conflict (bug 788974), but that currently doesn't > work. This only affects nightlies, not release builds. ah, huh, I spoke too quickly... it does :(

So the following shouldn't be in the common file: ac_add_options --enable-profiling ac_add_options --disable-elf-hack # --enable-elf-hack conflicts with --enable-profiling STRIP_FLAGS="--strip-debug"

And ac_add_options --with-branding=mobile/android/branding/nightly

See comments 5-11. Looks like this needs a follow-up patch.

Comment on attachment 779921 [details] [diff] [review] mozconfig_follow_up.patch Review of attachment 779921 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/config/mozconfigs/android-armv6/nightly @@ +8,5 @@ > +ac_add_options --with-branding=mobile/android/branding/nightly > + > +# This will overwrite the default of stripping everything and keep the symbol table. > +# This is useful for profiling with eideticker. See bug 788680 > +STRIP_FLAGS="--strip-debug" Why --strip-debug here if profiling is not enabled in this mozconfig? Should this be moved to the x86 mozconfig instead? ::: mobile/android/config/mozconfigs/common @@ +45,1 @@ > . "$topsrcdir/mobile/android/config/mozconfigs/common.override" According to the documentation in common.override, this include should be at the bottom of the individual mozconfigs so as to override anything in them, rather than at the bottom of the common config which will not have the same effect.

Comment on attachment 779921 [details] [diff] [review] mozconfig_follow_up.patch Review of attachment 779921 [details] [diff] [review]: ----------------------------------------------------------------- I agree with everything kats said.

Comment on attachment 780026 [details] [diff] [review] mozconfig_follow_up.patch Review of attachment 780026 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Sorry not to have cought these on the original patch review.

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/f18854a1b9cb alongside bug 894313 because one or both of them broke Android. https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=2571b6bbc93a