Teach gyp about configurable filter commands, add "pymod_do_main" filter

pylib/gyp/input.py

 #!/usr/bin/python
 
 # Copyright (c) 2011 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.
 
 from compiler.ast import Const
 from compiler.ast import Dict
 from compiler.ast import Discard
 from compiler.ast import List
 from compiler.ast import Module
 from compiler.ast import Node
 from compiler.ast import Stmt
 import compiler
 import copy
 import gyp.common
 import optparse
 import os.path
 import re
 import shlex
 import subprocess
 import sys
 
+# Try to import grit.
+try:
+  import grit_info  # TODO: make optional

[Mark Mentovai, 2011/05/16 16:51:34]

What does “TODO: make optional” mean when this is already inside a try?

+  grit_available = True
+except Exception, e:
+  grit_available = False

[tony, 2011/05/16 16:24:05]

The python idiom for this is on Exception to just set grit_info to None and
check for grit_info below.

+
 
 # A list of types that are treated as linkable.
 linkable_types = ['executable', 'shared_library', 'loadable_module']
 
 # A list of sections that contain links to other targets.
 dependency_sections = ['dependencies', 'export_dependent_settings']
 
 # base_path_sections is a list of sections defined by GYP that contain
 # pathnames.  The generators can provide more keys, the two lists are merged
 # into path_sections, but you should call IsPathSection instead of using either
@@ skipping 593 matching lines @@
         # is invoked it produces different output by design. When the need
         # arises, the syntax should be extended to support no caching off a
         # command's output so it is run every time.
         cache_key = str(contents)
         cached_value = cached_command_results.get(cache_key, None)
         if cached_value is None:
           gyp.DebugOutput(gyp.DEBUG_VARIABLES,
                           "Executing command '%s' in directory '%s'" %
                           (contents,build_file_dir))
 
-          # Fix up command with platform specific workarounds.
-          contents = FixupPlatformCommand(contents)
-          p = subprocess.Popen(contents, shell=use_shell,
-                               stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE,
-                               stdin=subprocess.PIPE,
-                               cwd=build_file_dir)
+          replacement = ''
 
-          (p_stdout, p_stderr) = p.communicate('')
+          parsed_contents = contents.split()

[Mark Mentovai, 2011/05/16 16:51:34]

Can you refactor this whole block into its own function that handles
subprocess.Popen, p.communicate, and p.wait, or their equivalents when doing
things internally?

[Mark Mentovai, 2011/05/16 16:51:34]

shlex instead?

+          if (grit_available and
+              parsed_contents[0] == 'python' and
+              parsed_contents[1].endswith('grit_info.py')):

[tony, 2011/05/16 16:24:05]

Will parsed_contents always have at least 2 valeus?

[bradn, 2011/05/16 16:35:50]

Maybe raise an exception with a more specific error.

[Mark Mentovai, 2011/05/16 16:51:34]

It certainly seems like you can tighten up the safety of this check.

+            # Invoke grit_info directly, without spawning a subprocess.  Do
+            # this in |build_file_dir|.
+            oldwd = os.getcwd()  # Python doesn't like os.open('.'): no fchdir.
+            os.chdir(build_file_dir)
+            replacement = str(grit_info.DoMain(contents.split()[2:]).rstrip())

[tony, 2011/05/16 16:24:05]

The extra str() seems unnecessary (doesn't rstrip() always returns a string?).

[Mark Mentovai, 2011/05/16 16:51:34]

You already have parsed_contents, why are you going back to contents again?

+            os.chdir(oldwd)
+            assert replacement != None
+          else:
+            # Fix up command with platform specific workarounds.
+            contents = FixupPlatformCommand(contents)
+            p = subprocess.Popen(contents, shell=use_shell,
+                                 stdout=subprocess.PIPE,
+                                 stderr=subprocess.PIPE,
+                                 stdin=subprocess.PIPE,
+                                 cwd=build_file_dir)
 
-          if p.wait() != 0 or p_stderr:
-            sys.stderr.write(p_stderr)
-            # Simulate check_call behavior, since check_call only exists
-            # in python 2.5 and later.
-            raise Exception("Call to '%s' returned exit status %d." %
-                            (contents, p.returncode))
-          replacement = p_stdout.rstrip()
+            (p_stdout, p_stderr) = p.communicate('')
+
+            if p.wait() != 0 or p_stderr:
+              sys.stderr.write(p_stderr)
+              # Simulate check_call behavior, since check_call only exists
+              # in python 2.5 and later.
+              raise Exception("Call to '%s' returned exit status %d." %
+                              (contents, p.returncode))
+            replacement = p_stdout.rstrip()
 
           cached_command_results[cache_key] = replacement
         else:
           gyp.DebugOutput(gyp.DEBUG_VARIABLES,
                           "Had cache value for command '%s' in directory '%s'" %
                           (contents,build_file_dir))
           replacement = cached_value
 
       else:
         if not contents in variables:
@@ skipping 1577 matching lines @@
     ValidateRunAsInTarget(target, target_dict, build_file)
     ValidateActionsInTarget(target, target_dict, build_file)
 
   # Generators might not expect ints.  Turn them into strs.
   TurnIntIntoStrInDict(data)
 
   # TODO(mark): Return |data| for now because the generator needs a list of
   # build files that came in.  In the future, maybe it should just accept
   # a list, and not the whole data dict.
   return [flat_list, targets, data]