Bug 1640578 - Remove --disable-install-strip from mac mozconfigs. r=froydnj

The need for --disable-install-strip in the mac mozconfigs comes from a
discrepancy in how stripping is handled between platforms. On Windows,
there is no stripping. On non-Mac unix, `strip` removes local symbols as
well as debug info and debug symbols. On Mac, it actually removes too
much, and one has to pass flags to remove both local symbols (`-x`) and
debug symbols (`-S`). Debug info is already in a separate file
(`.dSYM`).

For profiling reasons, we do ship e.g. nightlies with local symbols but
not debug info or symbols (or at least that's the intent). On Windows,
again, nothing to do. On non-Mac unix, we pass `--strip-debug` to
`strip` so that it keeps local symbols. That's where the discrepancy
comes in for Mac: the build system doesn't handle this at all, so the
mozconfigs contain --disable-install-strip to avoid stripping.

The build system should be doing what it's expected to be doing from the
start, without mozconfigs opting into anything.

AFAIK, we only really need the local symbols, so we can `strip -S` on
Mac when profiling is enabled, rather than `strip -x -S`. This also
significantly reduces the size of the installer for nightlies.

And while we're here, move the logic out of old-configure and into
python configure.

Differential Revision: https://phabricator.services.mozilla.com/D76789

browser/config/mozconfigs/macosx64/devedition

@@ -3,8 +3,6 @@
 # Add-on signing is not required for DevEdition
 MOZ_REQUIRE_SIGNING=0
 
-ac_add_options --disable-install-strip
-
 ac_add_options --enable-instruments
 
 # Cross-compiled builds fail when dtrace is enabled

browser/config/mozconfigs/macosx64/nightly

@@ -1,6 +1,5 @@
 . "$topsrcdir/browser/config/mozconfigs/macosx64/common-opt"
 
-ac_add_options --disable-install-strip
 ac_add_options --enable-instruments
 
 # Cross-compiled builds fail when dtrace is enabled

browser/config/mozconfigs/whitelist

@@ -13,7 +13,6 @@ for platform in all_platforms:
     ]
 
 whitelist['nightly']['macosx64'] += [
-    'ac_add_options --disable-install-strip',
     'ac_add_options --enable-instruments',
     'ac_add_options --enable-dtrace',
     'if test `uname -s` != Linux; then',

moz.configure

@@ -258,6 +258,10 @@ set_define('MOZ_DLL_PREFIX', depends(library_name_info.dll.prefix)(lambda s: '"%
 set_define('MOZ_DLL_SUFFIX', depends(library_name_info.dll.suffix)(lambda s: '"%s"' % s))
 set_config('WASM_OBJ_SUFFIX', 'wasm')
 
+# Make `profiling` available to this file even when js/moz.configure
+# doesn't end up included.
+profiling = dependable(False)
+
 include(include_project_configure)
 
 @depends('--help')
@@ -672,6 +676,31 @@ def pkg_skip_strip(install_strip, may_strip):
 set_config('PKG_SKIP_STRIP', True, when=pkg_skip_strip)
 
 
+@depends('--enable-strip', '--enable-install-strip', when=may_strip)
+def strip(strip, install_strip):
+    return strip or install_strip
+
+js_option(env='STRIP_FLAGS', nargs=1, when=strip, help='Flags for the strip command')
+
+@depends('STRIP_FLAGS', profiling, target, when=strip)
+def strip_flags(flags, profiling, target):
+    if flags:
+        return flags[0].split()
+    if profiling:
+        # Only strip debug info and symbols when profiling is enabled, keeping
+        # local symbols.
+        if target.kernel == 'Darwin':
+            return ['-S']
+        else:
+            return ['--strip-debug']
+    # Otherwise strip everything we can, which happens without flags on non-Darwin.
+    # On Darwin, it tries to strip things it can't, so we need to limit its scope.
+    elif target.kernel == 'Darwin':
+        return ['-x', '-S']
+
+set_config('STRIP_FLAGS', strip_flags)
+
+
 # Please do not add configure checks from here on.
 
 # Fallthrough to autoconf-based configure

old-configure.in

@@ -450,15 +450,6 @@ if test "$GNU_CXX"; then
     _DEFINES_CXXFLAGS="-DMOZILLA_CLIENT -include $_objdir/mozilla-config.h"
 fi
 
-# For profiling builds keep the symbol information
-if test "$MOZ_PROFILING" -a -z "$STRIP_FLAGS"; then
-    case "$OS_TARGET" in
-    Linux|DragonFly|FreeBSD|NetBSD|OpenBSD)
-        STRIP_FLAGS="--strip-debug"
-        ;;
-    esac
-fi
-
 dnl ========================================================
 dnl = Enable DMD
 dnl ========================================================
@@ -524,7 +515,6 @@ case "$target" in
     MOZ_OPTIMIZE_FLAGS="-O3"
     CXXFLAGS="$CXXFLAGS -stdlib=libc++"
     DSO_LDOPTS=''
-    STRIP_FLAGS="$STRIP_FLAGS -x -S"
     # The ExceptionHandling framework is needed for Objective-C exception
     # logging code in nsObjCExceptions.h. Currently we only use that in debug
     # builds.
@@ -2670,7 +2660,6 @@ AC_SUBST(MOZ_UPDATER)
 AC_SUBST(MOZ_ANDROID_APPLICATION_CLASS)
 AC_SUBST(MOZ_ANDROID_BROWSER_INTENT_CLASS)
 AC_SUBST(MOZ_EXCLUDE_HYPHENATION_DICTIONARIES)
-AC_SUBST(STRIP_FLAGS)
 AC_SUBST(INCREMENTAL_LINKER)
 
 AC_SUBST_LIST(MOZ_FIX_LINK_PATHS)

python/mozbuild/mozpack/executables.py

@@ -95,7 +95,7 @@ def strip(path):
     '''
     from buildconfig import substs
     strip = substs['STRIP']
-    flags = substs['STRIP_FLAGS'].split() if 'STRIP_FLAGS' in substs else []
+    flags = substs.get('STRIP_FLAGS', [])
     cmd = [strip] + flags + [path]
     if subprocess.call(cmd) != 0:
         errors.fatal('Error executing ' + ' '.join(cmd))