Add the ability to specify a target_os for gclient solutions

gclient.py

Index: gclient.py
===================================================================
--- gclient.py	(revision 132861)
+++ gclient.py	(working copy)
@@ -143,7 +143,7 @@
   """Immutable configuration settings."""
   def __init__(
       self, parent, url, safesync_url, managed, custom_deps, custom_vars,
-      deps_file, should_process):
+      deps_file, target_os, should_process):
     GClientKeywords.__init__(self)
 
     # These are not mutable:
@@ -151,6 +151,7 @@
     self._safesync_url = safesync_url
     self._deps_file = deps_file
     self._url = url
+    self._target_os = target_os

[John Grabowski, 2012/04/19 05:29:04]

move this to a spot "right above" should_process to be more consistent with the
convention (vars initialized in same-ish order as args)

[Peter Beverloo, 2012/04/19 06:11:03]

I disagree. There is a large comment block above _managed and _should_process
which does not apply to target_os. Since target_os is not mutable (only in the
.gclient file) I think this would be the most appropriate position.
Alternatively it could be around line 167.

     # 'managed' determines whether or not this dependency is synced/updated by
     # gclient after gclient checks it out initially.  The difference between
     # 'managed' and 'should_process' is that the user specifies 'managed' via
@@ -220,6 +221,10 @@
     return self._url
 
   @property
+  def target_os(self):
+    return self._target_os
+
+  @property
   def recursion_limit(self):
     """Returns > 0 if this dependency is not too recursed to be processed."""
     return max(self.parent.recursion_limit - 1, 0)
@@ -236,11 +241,11 @@
   """Object that represents a dependency checkout."""
 
   def __init__(self, parent, name, url, safesync_url, managed, custom_deps,
-               custom_vars, deps_file, should_process):
+               custom_vars, deps_file, target_os, should_process):
     gclient_utils.WorkItem.__init__(self, name)
     DependencySettings.__init__(
         self, parent, url, safesync_url, managed, custom_deps, custom_vars,
-        deps_file, should_process)
+        deps_file, target_os, should_process)
 
     # This is in both .gclient and DEPS files:
     self._deps_hooks = []
@@ -430,6 +435,9 @@
     # override or extend the values defined by the 'deps' member.
     if 'deps_os' in local_scope:
       enforced_os = self.root.enforced_os
+      if self.target_os and self.target_os not in enforced_os:
+        enforced_os = enforced_os + tuple([self.target_os])

[John Grabowski, 2012/04/19 05:29:04]

tuple() seems redundant.  How about just
  enforced_os = enforced_os + [self.target_os]
or, even simpler, 
  enforced_os.append(self.target_os)

[Peter Beverloo, 2012/04/19 06:11:03]

enforced_os is a tuple, which are not mutable (so have no append method).
Appending a list to a tuple throws an error as this is not allowerd either..

+
       for deps_os_key in enforced_os:
         os_deps = local_scope['deps_os'].get(deps_os_key, {})
         if len(enforced_os) > 1:
@@ -465,7 +473,7 @@
       should_process = self.recursion_limit and self.should_process
       deps_to_add.append(Dependency(
           self, name, url, None, None, None, None,
-          self.deps_file, should_process))
+          self.deps_file, self.target_os, should_process))
     deps_to_add.sort(key=lambda x: x.name)
     self.add_dependencies_and_close(deps_to_add, local_scope.get('hooks', []))
     logging.info('ParseDepsFile(%s) done' % self.name)
@@ -743,7 +751,8 @@
     out = []
     for i in ('name', 'url', 'parsed_url', 'safesync_url', 'custom_deps',
               'custom_vars', 'deps_hooks', 'file_list', 'should_process',
-              'processed', 'hooks_ran', 'deps_parsed', 'requirements'):
+              'target_os', 'processed', 'hooks_ran', 'deps_parsed',

[John Grabowski, 2012/04/19 05:29:04]

Somewhere you should document the idea behind target_os and how it is optional.
I don't know where, but somewhere.

[Peter Beverloo, 2012/04/19 06:11:03]

Agreed. I added this to the first comment in the file, similar to how "hooks"
has done this.

+              'requirements'):
       # First try the native property if it exists.
       if hasattr(self, '_' + i):
         value = getattr(self, '_' + i, False)
@@ -821,7 +830,7 @@
     # are processed.
     self._recursion_limit = 2
     Dependency.__init__(self, None, None, None, None, True, None, None,
-                        'unused', True)
+                        'unused', None, True)
     self._options = options
     if options.deps_os:
       enforced_os = options.deps_os.split(',')
@@ -852,6 +861,7 @@
             s.get('custom_deps', {}),
             s.get('custom_vars', {}),
             s.get('deps_file', 'DEPS'),
+            s.get('target_os', None),
             True))
       except KeyError:
         raise gclient_utils.Error('Invalid .gclient file. Solution is '