Bug 1077151 - Always use expandlibs descriptors when they exist. r=mshal

Currently, when there is both an expandlibs descriptor and an actual static
library, expandlibs picks the static library. This has the side effect that
if there are object files in the static library that aren't directly used,
they're dropped when linking, even when they export symbols that would be
exported in the final linked binary.

In most cases in the code base, files are not dropped that way. The most
notable counter-example is xpcomglue, where actually not dropping files
leads to link failure because of missing symbols those files reference
(yes, that would tend to say the glue is broken in some way).

On the opposite side, there is mozglue, which does have both a descriptor
and a static library (the latter being necessary for the SDK), and that
linking as a static library drops files that shouldn't be dropped (like
jemalloc). We're currently relying on -Wl,--whole-archive for those files
not to be dropped, but that won't really be possible without much hassle
in a world where mozglue dependencies live in moz.build land.

Switching expandlibs to use descriptors when they exist, even when there
is a static library (so, the opposite of the current behavior) allows to
drop -Wl,--whole-archive and prepare for a better future. However, as
mentioned, xpcomglue does still require to be linked through the static
library, so we need to make it a static library only.

To achieve that, we make NO_EXPAND_LIBS now actually mean no expandlibs
and use that to build the various different xpcomglues.

config/expandlibs.py

@@ -113,14 +113,15 @@ def _expand(self, arg):
         (root, ext) = os.path.splitext(arg)
         if ext != conf.LIB_SUFFIX or not os.path.basename(root).startswith(conf.LIB_PREFIX):
             return [relativize(arg)]
-        if len(conf.IMPORT_LIB_SUFFIX):
-            dll = root + conf.IMPORT_LIB_SUFFIX
-        else:
+        if conf.LIB_PREFIX:
             dll = root.replace(conf.LIB_PREFIX, conf.DLL_PREFIX, 1) + conf.DLL_SUFFIX
+        else:
+            dll = root + conf.DLL_SUFFIX
         if os.path.exists(dll):
-            return [relativize(dll)]
-        if os.path.exists(arg):
-            return [relativize(arg)]
+            if conf.IMPORT_LIB_SUFFIX:
+                return [relativize(root + conf.IMPORT_LIB_SUFFIX)]
+            else:
+                return [relativize(dll)]
         return self._expand_desc(arg)
 
     def _expand_desc(self, arg):
@@ -136,7 +137,7 @@ def _expand_desc(self, arg):
             for lib in desc['LIBS']:
                 objs += self._expand(lib)
             return objs
-        return [arg]
+        return [relativize(arg)]
 
 if __name__ == '__main__':
     print " ".join(ExpandArgs(sys.argv[1:]))

config/rules.mk

@@ -140,7 +140,7 @@ ifdef REAL_LIBRARY
 ifdef FORCE_SHARED_LIB
 # ... except when we really want one
 ifdef NO_EXPAND_LIBS
-LIBRARY			:= $(REAL_LIBRARY) $(REAL_LIBRARY).$(LIBS_DESC_SUFFIX)
+LIBRARY			:= $(REAL_LIBRARY)
 else
 LIBRARY			:= $(REAL_LIBRARY).$(LIBS_DESC_SUFFIX)
 endif
@@ -149,9 +149,13 @@ else
 ifeq (,$(SDK_LIBRARY)$(DIST_INSTALL)$(NO_EXPAND_LIBS))
 LIBRARY			:= $(REAL_LIBRARY).$(LIBS_DESC_SUFFIX)
 else
+ifdef NO_EXPAND_LIBS
+LIBRARY			:= $(REAL_LIBRARY)
+else
 LIBRARY			:= $(REAL_LIBRARY) $(REAL_LIBRARY).$(LIBS_DESC_SUFFIX)
 endif
 endif
+endif
 endif # REAL_LIBRARY
 endif # LIBRARY
 
@@ -776,7 +780,8 @@ endif
 
 $(filter %.$(LIB_SUFFIX),$(LIBRARY)): $(OBJS) $(STATIC_LIBS_DEPS) $(filter %.$(LIB_SUFFIX),$(EXTRA_LIBS)) $(EXTRA_DEPS) $(GLOBAL_DEPS)
 	$(REPORT_BUILD)
-	$(RM) $(LIBRARY)
+# Always remove both library and library descriptor
+	$(RM) $(REAL_LIBRARY) $(REAL_LIBRARY).$(LIBS_DESC_SUFFIX)
 	$(EXPAND_AR) $(AR_FLAGS) $(OBJS) $(STATIC_LIBS) $(filter %.$(LIB_SUFFIX),$(EXTRA_LIBS))
 
 $(filter-out %.$(LIB_SUFFIX),$(LIBRARY)): $(filter %.$(LIB_SUFFIX),$(LIBRARY)) $(OBJS) $(STATIC_LIBS_DEPS) $(filter %.$(LIB_SUFFIX),$(EXTRA_LIBS)) $(EXTRA_DEPS) $(GLOBAL_DEPS)

config/tests/unit-expandlibs.py

@@ -186,15 +186,15 @@ def test_expand(self):
         args = ExpandArgs(['foo', '-bar'] + self.arg_files + [self.tmpfile('liby', Lib('y'))])
         self.assertRelEqual(args, ['foo', '-bar'] + self.files + self.liby_files + self.libx_files) 
 
-        # When a library exists at the same time as a descriptor, we just use
-        # the library
+        # When a library exists at the same time as a descriptor, we still use
+        # the descriptor.
         self.touch([self.tmpfile('libx', Lib('x'))])
         args = ExpandArgs(['foo', '-bar'] + self.arg_files + [self.tmpfile('liby', Lib('y'))])
-        self.assertRelEqual(args, ['foo', '-bar'] + self.files + self.liby_files + [self.tmpfile('libx', Lib('x'))]) 
+        self.assertRelEqual(args, ['foo', '-bar'] + self.files + self.liby_files + self.libx_files)
 
         self.touch([self.tmpfile('liby', Lib('y'))])
         args = ExpandArgs(['foo', '-bar'] + self.arg_files + [self.tmpfile('liby', Lib('y'))])
-        self.assertRelEqual(args, ['foo', '-bar'] + self.files + [self.tmpfile('liby', Lib('y'))])
+        self.assertRelEqual(args, ['foo', '-bar'] + self.files + self.liby_files + self.libx_files)
 
 class TestExpandArgsMore(TestExpandInit):
     def test_makelist(self):
@@ -278,14 +278,15 @@ def check_output(args, **kargs):
         self.touch([self.tmpfile('liby', Lib('y'))])
         for iteration in (1, 2):
             with ExpandArgsMore(['foo', '-bar'] + self.arg_files + [self.tmpfile('liby', Lib('y'))]) as args:
-                self.assertRelEqual(args, ['foo', '-bar'] + self.files + [self.tmpfile('liby', Lib('y'))])
+                files = self.files + self.liby_files + self.libx_files
+
+                self.assertRelEqual(args, ['foo', '-bar'] + files)
 
                 extracted = {}
                 # ExpandArgsMore also has an extra method extracting static libraries
                 # when possible
                 args.extract()
 
-                files = self.files + self.liby_files + self.libx_files
                 # With AR_EXTRACT, it uses the descriptors when there are, and
                 # actually
                 # extracts the remaining libraries

configure.in

@@ -6995,13 +6995,12 @@ elif test "${OS_TARGET}" = "WINNT" -o "${OS_TARGET}" = "Darwin"; then
   MOZ_GLUE_LDFLAGS='$(call EXPAND_LIBNAME_PATH,mozglue,$(LIBXUL_DIST)/lib)'
 else
   dnl On other Unix systems, we only want to link executables against mozglue
-  MOZ_GLUE_PROGRAM_LDFLAGS='$(MKSHLIB_FORCE_ALL) $(call EXPAND_LIBNAME_PATH,mozglue,$(LIBXUL_DIST)/lib)'
+  MOZ_GLUE_PROGRAM_LDFLAGS='$(call EXPAND_LIBNAME_PATH,mozglue,$(LIBXUL_DIST)/lib)'
   dnl On other Unix systems, where mozglue is a static library, jemalloc is
   dnl separated for the SDK, so we need to add it here.
   if test "$MOZ_MEMORY" = 1 -o \( "$LIBXUL_SDK" -a -f "$LIBXUL_SDK/lib/${LIB_PREFIX}memory.${LIB_SUFFIX}" \); then
     MOZ_GLUE_PROGRAM_LDFLAGS="$MOZ_GLUE_PROGRAM_LDFLAGS "'$(call EXPAND_LIBNAME_PATH,memory,$(LIBXUL_DIST)/lib)'
   fi
-  MOZ_GLUE_PROGRAM_LDFLAGS="$MOZ_GLUE_PROGRAM_LDFLAGS "'$(MKSHLIB_UNFORCE_ALL)'
   if test -n "$GNU_CC"; then
     dnl And we need mozglue symbols to be exported.
     MOZ_GLUE_PROGRAM_LDFLAGS="$MOZ_GLUE_PROGRAM_LDFLAGS -rdynamic"

xpcom/glue/Makefile.in

@@ -4,3 +4,5 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIST_INSTALL	= 1
+# Force to build a static library only
+NO_EXPAND_LIBS = 1

xpcom/glue/nomozalloc/Makefile.in

@@ -4,3 +4,5 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIST_INSTALL	= 1
+# Force to build a static library only
+NO_EXPAND_LIBS = 1

xpcom/glue/standalone/Makefile.in

@@ -4,3 +4,5 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIST_INSTALL	= 1
+# Force to build a static library only
+NO_EXPAND_LIBS = 1

xpcom/glue/standalone/staticruntime/Makefile.in

@@ -3,3 +3,5 @@
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIST_INSTALL	= 1
+# Force to build a static library only
+NO_EXPAND_LIBS = 1

xpcom/glue/staticruntime/Makefile.in

@@ -4,3 +4,5 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIST_INSTALL	= 1
+# Force to build a static library only
+NO_EXPAND_LIBS = 1