[NaCl SDK] Fix create_nmf handling of missing libraries.

native_client_sdk/src/tools/create_nmf.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import errno
 import hashlib
 import json
 import optparse
 import os
@@ skipping 172 matching lines @@
     path: Full path to this file on the build system
     arch: Architecture of this file (e.g., x86-32)
     url: Relative path to file in the staged web directory.
         Used for specifying the "url" attribute in the nmf file.'''
 
   def __init__(self, name, path, url, arch=None):
     self.name = name
     self.path = path
     self.url = url
     self.arch = arch
-    if arch is None:
+    if not arch:
       self.arch = ParseElfHeader(path)[0]
 
   def __repr__(self):
     return '<ArchFile %s>' % self.path
 
   def __str__(self):
     '''Return the file path when invoked with the str() function'''
     return self.path
 
 
@@ skipping 24 matching lines @@
     self.objdump = objdump
     self.main_files = main_files or []
     self.extra_files = extra_files or []
     self.lib_path = lib_path or []
     self.manifest = None
     self.needed = {}
     self.lib_prefix = lib_prefix or []
     self.remap = remap or {}
     self.pnacl = main_files and main_files[0].endswith('pexe')
 
-  def GleanFromObjdump(self, files):
+    for filename in self.main_files:
+      if not os.path.exists(filename):
+        raise Error('Input file not found: %s' % filename)
+      if not os.path.isfile(filename):
+        raise Error('Input is not a file: %s' % filename)
+
+  def GleanFromObjdump(self, files, arch):
     '''Get architecture and dependency information for given files
 
     Args:
-      files: A dict with key=filename and value=list or set of archs.  E.g.:
-          { '/path/to/my.nexe': ['x86-32']
-            '/path/to/lib64/libmy.so': ['x86-64'],
-            '/path/to/mydata.so': ['x86-32', 'x86-64'],
-            '/path/to/my.data': None }  # Indicates all architectures
+      files: A list of files to examine.
+          [ '/path/to/my.nexe',
+            '/path/to/lib64/libmy.so',
+            '/path/to/mydata.so',
+            '/path/to/my.data' ]
+      arch: The architecure we are looking for, or None to accept any
+            architecture.
 
     Returns: A tuple with the following members:
       input_info: A dict with key=filename and value=ArchFile of input files.
           Includes the input files as well, with arch filled in if absent.
           Example: { '/path/to/my.nexe': ArchFile(my.nexe),
                      '/path/to/libfoo.so': ArchFile(libfoo.so) }
       needed: A set of strings formatted as "arch/name".  Example:
           set(['x86-32/libc.so', 'x86-64/libgcc.so'])
     '''
     if not self.objdump:
       self.objdump = FindObjdumpExecutable()
       if not self.objdump:
         raise Error('No objdump executable found (see --help for more info)')
-    DebugPrint('GleanFromObjdump(%s)' % ([self.objdump, '-p'] + files.keys()))
-    proc = subprocess.Popen([self.objdump, '-p'] + files.keys(),
-                            stdout=subprocess.PIPE,
+
+    full_paths = set()
+    for filename in files:
+      if os.path.exists(filename):
+        full_paths.add(filename)
+      else:
+        for path in self.FindLibsInPath(filename):
+          full_paths.add(path)
+
+    cmd = [self.objdump, '-p'] + list(full_paths)
+    DebugPrint('GleanFromObjdump[%s](%s)' % (arch, cmd))
+    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE, bufsize=-1)
+
     input_info = {}
+    found_basenames = {}

[binji, 2013/05/28 22:52:41]

= set(), you never use the value

[Sam Clegg, 2013/05/28 23:04:40]

Its used below, unless I'm misunderstanding.

     needed = set()
     output, err_output = proc.communicate()
     if proc.returncode:
       raise Error('%s\nStdError=%s\nobjdump failed with error code: %d' %
                   (output, err_output, proc.returncode))
 
     for line in output.splitlines(True):
       # Objdump should display the architecture first and then the dependencies
       # second for each file in the list.
       matched = FormatMatcher.match(line)
       if matched:
         filename = matched.group(1)
-        arch = OBJDUMP_ARCH_MAP[matched.group(2)]
-        if files[filename] is None or arch in files[filename]:
-          name = os.path.basename(filename)
-          input_info[filename] = ArchFile(
-              arch=arch,
-              name=name,
-              path=filename,
-              url='/'.join(self.lib_prefix + [ARCH_LOCATION[arch], name]))
+        file_arch = OBJDUMP_ARCH_MAP[matched.group(2)]
+        if arch and file_arch != arch:
+          continue
+        name = os.path.basename(filename)
+        found_basenames[name] = True
+        input_info[filename] = ArchFile(
+            arch=file_arch,
+            name=name,
+            path=filename,
+            url='/'.join(self.lib_prefix + [ARCH_LOCATION[file_arch], name]))
       matched = NeededMatcher.match(line)
       if matched:
-        if files[filename] is None or arch in files[filename]:
-          needed.add('/'.join([arch, matched.group(1)]))
+        if not arch or arch == file_arch:

[binji, 2013/05/28 22:52:41]

might be nicer to use
not(arch and arch != file_arch) to make the connection between this and the
above check clearer.

[binji, 2013/05/28 22:52:41]

now that I look at it closer, where is file_arch coming from here? It looks like
it is only set by line 296 above.

[Sam Clegg, 2013/05/28 23:04:40]

Just removed this condition as the continue above means it is alwasy true.

+          match = '/'.join([file_arch, matched.group(1)])
+          needed.add(match)
+          Trace("NEEDED: %s" % match)
+
+    for filename in files:
+      if os.path.basename(filename) not in found_basenames:
+        raise Error('Library not found [%s]: %s' % (arch, filename))

[binji, 2013/05/28 22:52:41]

seems like it would be useful to know which object needed this missing library.

[Sam Clegg, 2013/05/28 23:04:40]

Maybe.   That would require another re-factor since that information is not
available at this level.

+
     return input_info, needed
 
   def FindLibsInPath(self, name):
     '''Finds the set of libraries matching |name| within lib_path
 
     Args:
       name: name of library to find
 
     Returns:
       A list of system paths that match the given name within the lib_path'''
@@ skipping 17 matching lines @@
 
     if self.needed:
       return self.needed
 
     DebugPrint('GetNeeded(%s)' % self.main_files)
 
     dynamic = any(ParseElfHeader(f)[1] for f in self.main_files)
 
     if dynamic:
       examined = set()
-      all_files, unexamined = self.GleanFromObjdump(
-          dict([(f, None) for f in self.main_files]))
-      for name, arch_file in all_files.items():
-        arch_file.url = name
+      all_files, unexamined = self.GleanFromObjdump(self.main_files, None)
+      for arch_file in all_files.itervalues():
+        arch_file.url = arch_file.path
         if unexamined:
           unexamined.add('/'.join([arch_file.arch, RUNNABLE_LD]))
+
       while unexamined:
         files_to_examine = {}
+
+        # Take all the currently unexamined files and group them
+        # by architecture.
         for arch_name in unexamined:
           arch, name = arch_name.split('/')
-          for path in self.FindLibsInPath(name):
-            files_to_examine.setdefault(path, set()).add(arch)
-        new_files, needed = self.GleanFromObjdump(files_to_examine)
-        all_files.update(new_files)
+          files_to_examine.setdefault(arch, []).append(name)
+
+        # Call GleanFromObjdump() for each architecture.
+        needed = set()
+        for arch, files in files_to_examine.iteritems():
+          new_files, new_needed = self.GleanFromObjdump(files, arch)
+          all_files.update(new_files)
+          needed |= new_needed
+
         examined |= unexamined
         unexamined = needed - examined
 
       # With the runnable-ld.so scheme we have today, the proper name of
       # the dynamic linker should be excluded from the list of files.
       ldso = [LD_NACL_MAP[arch] for arch in set(OBJDUMP_ARCH_MAP.values())]
-      for name, arch_map in all_files.items():
-        if arch_map.name in ldso:
+      for name, arch_file in all_files.items():
+        if arch_file.name in ldso:
           del all_files[name]
 
       self.needed = all_files
     else:
       for filename in self.main_files:
         url = os.path.split(filename)[1]
         archfile = ArchFile(name=os.path.basename(filename),
                             path=filename, url=url)
         self.needed[filename] = archfile
 
     return self.needed
 
   def StageDependencies(self, destination_dir):
     '''Copies over the dependencies into a given destination directory
 
     Each library will be put into a subdirectory that corresponds to the arch.
 
     Args:
       destination_dir: The destination directory for staging the dependencies
     '''
     nexe_root = os.path.dirname(os.path.abspath(self.main_files[0]))
     nexe_root = os.path.normcase(nexe_root)
 
     needed = self.GetNeeded()
-    for source, arch_file in needed.items():
+    for arch_file in needed.itervalues():
       urldest = arch_file.url
+      source = arch_file.path
 
       # for .nexe and .so files specified on the command line stage
       # them in paths relative to the .nexe (with the .nexe always
       # being staged at the root).
       if source in self.main_files:
         absdest = os.path.normcase(os.path.abspath(urldest))
         if absdest.startswith(nexe_root):
           urldest = os.path.relpath(urldest, nexe_root)
 
       destination = os.path.join(destination_dir, urldest)
@@ skipping 205 matching lines @@
 def main(argv):
   parser = optparse.OptionParser(
       usage='Usage: %prog [options] nexe [extra_libs...]')
   parser.add_option('-o', '--output', dest='output',
                     help='Write manifest file to FILE (default is stdout)',
                     metavar='FILE')
   parser.add_option('-D', '--objdump', dest='objdump',
                     help='Override the default "objdump" tool used to find '
                          'shared object dependencies',
                     metavar='TOOL')
-  parser.add_option('--no-default-libpath',
+  parser.add_option('--no-default-libpath', action='store_true',
                     help="Don't include the SDK default library paths")
   parser.add_option('--debug-libs', action='store_true',
                     help='Use debug library paths when constructing default '
                          'library path.')
   parser.add_option('-L', '--library-path', dest='lib_path',
                     action='append', default=[],
                     help='Add DIRECTORY to library search path',
                     metavar='DIRECTORY')
   parser.add_option('-P', '--path-prefix', dest='path_prefix', default='',
                     help='A path to prepend to shared libraries in the .nmf',
@@ skipping 18 matching lines @@
     Trace.verbose = True
   if options.debug_mode:
     DebugPrint.debug_mode = True
 
   if options.toolchain is not None:
     print 'warning: option -t/--toolchain is deprecated.'
 
   if len(args) < 1:
     raise Error('No nexe files specified.  See --help for more info')
 
-  for filename in args:
-    if not os.path.exists(filename):
-      raise Error('Input file not found: %s' % filename)
-    if not os.path.isfile(filename):
-      raise Error('Input is not a file: %s' % filename)
-
   canonicalized = ParseExtraFiles(options.extra_files, sys.stderr)
   if canonicalized is None:
     parser.error('Bad --extra-files (-x) argument syntax')
 
   remap = {}
   for ren in options.name:
     parts = ren.split(',')
     if len(parts) != 2:
       raise Error('Expecting --name=<orig_arch.so>,<new_name.so>')
     remap[parts[0]] = parts[1]
 
   if options.path_prefix:
     path_prefix = options.path_prefix.split('/')
   else:
     path_prefix = []
 
+  for libpath in options.lib_path:
+    if not os.path.exists(libpath):
+      raise Error('Specified library path does not exist: %s' % libpath)
+    if not os.path.isdir(libpath):
+      raise Error('Specified library is not a directory: %s' % libpath)
+
   if not options.no_default_libpath:
     # Add default libraries paths to the end of the search path.
     config = options.debug_libs and 'Debug' or 'Release'
     options.lib_path += GetDefaultLibPath(config)
 
   nmf = NmfUtils(objdump=options.objdump,
                  main_files=args,
                  lib_path=options.lib_path,
                  extra_files=canonicalized,
                  lib_prefix=path_prefix,
                  remap=remap)
 
   nmf.GetManifest()
-  if options.output is None:
+  if not options.output:
     sys.stdout.write(nmf.GetJson())
   else:
     with open(options.output, 'w') as output:
       output.write(nmf.GetJson())
 
   if options.stage_dependencies and not nmf.pnacl:
     Trace('Staging dependencies...')
     nmf.StageDependencies(options.stage_dependencies)
 
   return 0
 
 
 # Invoke this file directly for simple testing.
 if __name__ == '__main__':
   try:
     rtn = main(sys.argv[1:])
   except Error, e:
     sys.stderr.write('%s: %s\n' % (os.path.basename(__file__), e))
     rtn = 1
   except KeyboardInterrupt:
     sys.stderr.write('%s: interrupted\n' % os.path.basename(__file__))
     rtn = 1
   sys.exit(rtn)