Bug 1077151 - Always use expandlibs descriptors when they exist. r=mshal
authorMike Hommey <mh+mozilla@glandium.org>
Sat, 04 Oct 2014 10:33:00 +0900
changeset 208804 d959a6081ceacde13d41b8a4ee192c912c85ef02
parent 208803 d2005a8af9501712f7b5b06f13b9a1ab53dc3eb1
child 208805 4da1ac5c08a17e1e897c18c9f379296d20481708
push id9169
push userphilringnalda@gmail.com
push dateSun, 05 Oct 2014 16:53:36 +0000
treeherderfx-team@3d6d136be24f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmshal
bugs1077151
milestone35.0a1
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
config/rules.mk
config/tests/unit-expandlibs.py
configure.in
xpcom/glue/Makefile.in
xpcom/glue/nomozalloc/Makefile.in
xpcom/glue/standalone/Makefile.in
xpcom/glue/standalone/staticruntime/Makefile.in
xpcom/glue/staticruntime/Makefile.in
--- a/config/expandlibs.py
+++ b/config/expandlibs.py
@@ -108,35 +108,36 @@ class ExpandArgs(list):
         for arg in args:
             self += self._expand(arg)
 
     def _expand(self, arg):
         '''Internal function doing the actual work'''
         (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):
         '''Internal function taking care of lib descriptor expansion only'''
         desc = os.path.abspath(arg + conf.LIBS_DESC_SUFFIX)
         if os.path.exists(desc):
             if desc in self._descs:
                 return []
             self._descs.add(desc)
             with open(desc, 'r') as f:
                 desc = LibDescriptor(f.readlines())
             objs = [relativize(o) for o in desc['OBJS']]
             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:]))
--- a/config/rules.mk
+++ b/config/rules.mk
@@ -135,28 +135,32 @@ endif # ENABLE_TESTS
 #
 
 ifndef LIBRARY
 ifdef REAL_LIBRARY
 # Don't build actual static library if a shared library is also built
 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
 else
 # Only build actual library if it is installed in DIST/lib or SDK
 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
 
 ifndef HOST_LIBRARY
 ifdef HOST_LIBRARY_NAME
 HOST_LIBRARY		:= $(LIB_PREFIX)$(HOST_LIBRARY_NAME).$(LIB_SUFFIX)
 endif
 endif
@@ -771,17 +775,18 @@ endif
 
 ifdef DTRACE_PROBE_OBJ
 EXTRA_DEPS += $(DTRACE_PROBE_OBJ)
 OBJS += $(DTRACE_PROBE_OBJ)
 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)
 # When we only build a library descriptor, blow out any existing library
 	$(REPORT_BUILD)
 	$(if $(filter %.$(LIB_SUFFIX),$(LIBRARY)),,$(RM) $(REAL_LIBRARY))
 	$(EXPAND_LIBS_GEN) -o $@ $(OBJS) $(STATIC_LIBS) $(filter %.$(LIB_SUFFIX),$(EXTRA_LIBS))
 
--- a/config/tests/unit-expandlibs.py
+++ b/config/tests/unit-expandlibs.py
@@ -181,25 +181,25 @@ class TestExpandArgs(TestExpandInit):
     def test_expand(self):
         '''Test library expansion'''
         # Expanding arguments means libraries with a descriptor are expanded
         # with the descriptor content, and import libraries are used when
         # a library doesn't exist
         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):
         '''Test grouping object files in lists'''
         # ExpandArgsMore does the same as ExpandArgs
         with ExpandArgsMore(['foo', '-bar'] + self.arg_files + [self.tmpfile('liby', Lib('y'))]) as args:
             self.assertRelEqual(args, ['foo', '-bar'] + self.files + self.liby_files + self.libx_files) 
 
@@ -273,24 +273,25 @@ class TestExpandArgsMore(TestExpandInit)
             lib = os.path.splitext(os.path.basename(args[3]))[0]
             return '%s\n%s\n' % (Obj(lib), Obj(lib + '2'))
         subprocess.check_output = check_output
 
         # ExpandArgsMore does the same as ExpandArgs
         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
                 extracted_args = []
                 for f in files:
                     if f.endswith(config.LIB_SUFFIX):
                         base = os.path.splitext(os.path.basename(f))[0]
                         # On the first iteration, we test the behavior of
--- a/configure.in
+++ b/configure.in
@@ -6990,23 +6990,22 @@ fi
 if test "${OS_TARGET}" = "Android"; then
   dnl On Android, we use WRAP_LDFLAGS to link everything to mozglue
   :
 elif test "${OS_TARGET}" = "WINNT" -o "${OS_TARGET}" = "Darwin"; then
   dnl On Windows and OSX, we want to link all our binaries against mozglue
   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"
   fi
   if test "$MOZ_LINKER" = 1; then
     MOZ_GLUE_PROGRAM_LDFLAGS="$MOZ_GLUE_PROGRAM_LDFLAGS $MOZ_ZLIB_LIBS"
   fi
 fi
--- a/xpcom/glue/Makefile.in
+++ b/xpcom/glue/Makefile.in
@@ -1,6 +1,8 @@
 # vim:set ts=8 sw=8 sts=8 noet:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # 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
--- a/xpcom/glue/nomozalloc/Makefile.in
+++ b/xpcom/glue/nomozalloc/Makefile.in
@@ -1,6 +1,8 @@
 # vim:set ts=8 sw=8 sts=8 noet:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # 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
--- a/xpcom/glue/standalone/Makefile.in
+++ b/xpcom/glue/standalone/Makefile.in
@@ -1,6 +1,8 @@
 # vim:set ts=8 sw=8 sts=8 noet:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # 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
--- a/xpcom/glue/standalone/staticruntime/Makefile.in
+++ b/xpcom/glue/standalone/staticruntime/Makefile.in
@@ -1,5 +1,7 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this 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
--- a/xpcom/glue/staticruntime/Makefile.in
+++ b/xpcom/glue/staticruntime/Makefile.in
@@ -1,6 +1,8 @@
 # vim:set ts=8 sw=8 sts=8 noet:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # 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