ninja mac: Smarter dylib linking.

pylib/gyp/generator/ninja.py

Index: pylib/gyp/generator/ninja.py
===================================================================
--- pylib/gyp/generator/ninja.py	(revision 1425)
+++ pylib/gyp/generator/ninja.py	(working copy)
@@ -153,14 +153,19 @@
     """Return true if this is a target that can be linked against."""
     return self.type in ('static_library', 'shared_library')
 
-  def SharedLinkable(self):
-    """Return true if this is a shared library/module."""
+  def WriteToc(self, flavor):

[Ami GONE FROM CHROMIUM, 2012/07/01 03:51:25]

s/WriteToc/UsesToc/ ?

[Nico, 2012/07/01 16:33:22]

Done.

+    """Return if the target should produce a restat rule based on a TOC file."""

[Ami GONE FROM CHROMIUM, 2012/07/01 03:51:25]

s/if/true if/ to match the other method in this class (or use "whether" to keep
on a single line)

[Nico, 2012/07/01 16:33:22]

Done.

+    # For bundles, the .TOC should be produced for the binary, not for
+    # FinalOutput(). But the naive approach would put the TOC file into the
+    # bundle, so don't do this for bundles for now.
+    if flavor == 'win' or self.bundle:
+      return False
     return self.type in ('shared_library', 'loadable_module')
 
   def PreActionInput(self, flavor):
     """Return the path, if any, that should be used as a dependency of
     any dependent action step."""
-    if self.SharedLinkable() and flavor not in ['mac', 'win']:
+    if self.WriteToc(flavor):
       return self.FinalOutput() + '.TOC'
     return self.FinalOutput() or self.preaction_stamp
 
@@ -840,7 +845,7 @@
             extra_link_deps |= set(target.component_objs)
           elif self.flavor == 'win' and target.import_lib:
             extra_link_deps.add(target.import_lib)
-          elif target.SharedLinkable() and self.flavor not in ['mac', 'win']:
+          elif target.WriteToc(self.flavor):
             solibs.add(target.binary)
             implicit_deps.add(target.binary + '.TOC')
           else:
@@ -889,17 +894,18 @@
 
     if command in ('solink', 'solink_module'):
       extra_bindings.append(('soname', os.path.split(output)[1]))
-      extra_bindings.append(('lib', output))
+      extra_bindings.append(('lib',
+                            gyp.common.EncodePOSIXShellArgument(output)))
       if self.flavor == 'win':
         self.target.import_lib = output + '.lib'
         extra_bindings.append(('dll', output))
         extra_bindings.append(('implib', self.target.import_lib))
         output = [output, self.target.import_lib]
-      elif self.flavor != 'mac':
+      else:
         output = [output, output + '.TOC']
 
     if len(solibs):
-      extra_bindings.append(('solibs', ' '.join(solibs)))
+      extra_bindings.append(('solibs', gyp.common.EncodePOSIXShellList(solibs)))
 
     self.ninja.build(output, command, link_deps,
                      implicit=list(implicit_deps),
@@ -1476,23 +1482,47 @@
       command='rm -f $out && '
               './gyp-mac-tool filter-libtool libtool -static -o $out $in'
               '$postbuilds')
+
+    # Record the public interface of $lib in $lib.TOC. See the corresponding
+    # comment in the posix section above for details.
+    mtime_preserving_solink_base = (

[Ami GONE FROM CHROMIUM, 2012/07/01 03:51:25]

IWBN to dedup this with the posix version above.  I think the only diffs are the
conditional (otool) and extract_toc; might be worth unifying.  Or not.  Your
call.

[Nico, 2012/07/01 16:33:22]

It's likely I'll move this into mac-tool soon (once I looked at the todo in line
1506), so I'd like to keep this duplicated for now.

+        'if [ ! -e $lib -o ! -e ${lib}.TOC ] || '
+             # Always force dependent libraries to relink if this library

[Ami GONE FROM CHROMIUM, 2012/07/01 03:51:25]

s/libraries/targets/

[Nico, 2012/07/01 16:33:22]

Done.

+             # reexports something. Handling this correctly would require
+             # recursive TOC dumping but this is rare in practice, so punt.
+             'otool -l $lib | grep -q LC_REEXPORT_DYLIB ; then '
+          '%(solink)s && %(extract_toc)s > ${lib}.TOC; '
+        'else '
+          '%(solink)s && %(extract_toc)s > ${lib}.tmp && '
+          'if ! cmp -s ${lib}.tmp ${lib}.TOC; then '
+            'mv ${lib}.tmp ${lib}.TOC ; '
+          'fi; '
+        'fi'
+        % { 'solink': '$ld -shared $ldflags -o $lib %(suffix)s',
+            'extract_toc':
+              '{ otool -l $lib | grep LC_ID_DYLIB -A 2; '

[Mark Mentovai, 2012/07/01 03:50:42]

This should be -A 5. You want to catch the version numbers too.

Don’t worry about catching the “timestamp” in there. It’s always the same. It’s
not really a timestamp (any more).

[Nico, 2012/07/01 16:33:22]

Done.

+              'nm -gP $lib |cut -f1-2 -d\' \' |grep \'[ATDBCSI]$$\'; true; }'})

[Mark Mentovai, 2012/07/01 03:50:42]

2. Why not grep -v U$? Seems less fragile in case a new section is introduced in
the future.

[Mark Mentovai, 2012/07/01 03:50:42]

1. Does this work for Objective-C too? (There ought to be a test to cover that.
Add or remove an Objective-C function in a dylib and make sure that a dependent
is relinked.)

[Nico, 2012/07/01 16:33:22]

Done. This is a leftover from me using -UP earlier, where I also needed to
filter out [atdbcsi-].

[Nico, 2012/07/01 16:33:22]

As far as I understand, Objective-C functions always have private visibility and
the linker doesn't do anything with them -- Objective-C function linking is done
by the runtime at program start (or dylib load). This means this won't show
functions (class and metaclass symbols are public and do show up), but as far as
I understand it's not necessary either.

Sample:
Nicos-MacBook-Pro:src thakis$ cat test.mm
#import <Foundation/Foundation.h>
@interface A : NSObject - (void)f; @end
@implementation A - (void)f {} @end
Nicos-MacBook-Pro:src thakis$ clang -c test.mm
Nicos-MacBook-Pro:src thakis$ clang -shared -o libtest.dylib test.o -framework
Foundation
Nicos-MacBook-Pro:src thakis$ nm libtest.dylib 
0000000000000f50 t -[A f]
00000000000010e8 S  _OBJC_METACLASS_$_A
                 U _OBJC_CLASS_$_NSObject
00000000000010c0 S _OBJC_METACLASS_$_A
                 U _OBJC_METACLASS_$_NSObject
                 U __objc_empty_cache
                 U __objc_empty_vtable
                 U dyld_stub_binder

(Note that -[A f] is t, but  _OBJC_METACLASS_$_A is S)

+
     # TODO(thakis): The solink_module rule is likely wrong. Xcode seems to pass
     # -bundle -single_module here (for osmesa.so).
     master_ninja.rule(
       'solink',
-      description='SOLINK $out, POSTBUILDS',
-      command=('$ld -shared $ldflags -o $out '
-               '$in $libs$postbuilds'))
+      description='SOLINK $lib, POSTBUILDS',
+      restat=True,
+      command=(mtime_preserving_solink_base % {
+          'suffix': '$in $solibs $libs$postbuilds'}))
     master_ninja.rule(
       'solink_module',
-      description='SOLINK(module) $out, POSTBUILDS',
-      command=('$ld -shared $ldflags -o $out '
-               '$in $libs$postbuilds'))
+      description='SOLINK(module) $lib, POSTBUILDS',
+      restat=True,
+      command=(mtime_preserving_solink_base % {
+          'suffix': '$in $solibs $libs$postbuilds'}))
+
     master_ninja.rule(
       'link',
       description='LINK $out, POSTBUILDS',
       command=('$ld $ldflags -o $out '
-               '$in $libs$postbuilds'))
+               '$in $solibs $libs$postbuilds'))
     master_ninja.rule(
       'infoplist',
       description='INFOPLIST $out',