grit complete CL

build/gyp_chromium

Index: build/gyp_chromium
diff --git a/build/gyp_chromium b/build/gyp_chromium
index 3cddef69f7ca42117c2e24bca1ff61644e3e6bfa..37762c9dd34877b67efce46b3d5d3c0a5ee1e9a5 100755
--- a/build/gyp_chromium
+++ b/build/gyp_chromium
@@ -7,6 +7,7 @@
 # This script is wrapper for Chromium that adds some support for how GYP
 # is invoked by Chromium beyond what can be done in the gclient hooks.
 
+import cProfile

[Mark Mentovai, 2011/05/16 16:42:54]

Unneeded?

 import glob
 import os
 import shlex
@@ -16,6 +17,10 @@ import sys
 script_dir = os.path.dirname(__file__)
 chrome_src = os.path.abspath(os.path.join(script_dir, os.pardir))
 
+# Insert grit into path so that gyp can find it. This speeds gyp up
+# considerably.
+sys.path.insert(0, os.path.join(chrome_src, 'tools', 'grit'))

[Mark Mentovai, 2011/05/16 16:42:54]

I find this hard to follow on a quick glance. You’re putting tools/grit into
sys.path after tools/gyp/pylib, but you’re doing the insertions backwards. I
think you should put this sys.path.insert after the existing one for
tools/gyp/pylib, and insert it at position 1. Nobody says you need to use 0 for
each sys.path.insert.

+
 sys.path.insert(0, os.path.join(chrome_src, 'tools', 'gyp', 'pylib'))
 import gyp
 
@@ -95,7 +100,7 @@ def additional_include_files(args=[]):
 
   return result
 
-if __name__ == '__main__':
+def main():
   args = sys.argv[1:]
 
   # Use the Psyco JIT if available.
@@ -165,3 +170,8 @@ if __name__ == '__main__':
 
   # Off we go...
   sys.exit(gyp.main(args))
+
+

[Mark Mentovai, 2011/05/16 16:42:54]

This file seems to use only one blank line between things.

+if __name__ == '__main__':
+  #cProfile.run('main()', 'profile')

[Mark Mentovai, 2011/05/16 16:42:54]

Unneeded?

+  main()

[Mark Mentovai, 2011/05/16 16:42:54]

sys.exit(main(sys.argv[1:])) instead, and adjust main accordingly?

Might make it easier in the future if someone wants to wrap gyp_chromium in the
same way you’re now trying to wrap grit_info.