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

pylib/gyp/input.py

Index: pylib/gyp/input.py
===================================================================
--- pylib/gyp/input.py	(revision 917)
+++ pylib/gyp/input.py	(working copy)
@@ -21,7 +21,14 @@
 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']
 
@@ -634,24 +641,38 @@
                           "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,