Handle conflicting os deps better in gclient.

gclient.py

Index: gclient.py
diff --git a/gclient.py b/gclient.py
index 407b9ad4e99f1efd3f31270f3695e18f458b1fcb..2c130b79fb6d45001db27a57c55fdd8d88abf7c1 100755
--- a/gclient.py
+++ b/gclient.py
@@ -490,17 +490,40 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # override or extend the values defined by the 'deps' member.
     target_os_deps = {}
     if 'deps_os' in local_scope:
-      for deps_os_key in self.target_os:
-        os_deps = local_scope['deps_os'].get(deps_os_key, {})
-        if len(self.target_os) > 1:
-          # Ignore any conflict when including deps for more than one
-          # platform, so we collect the broadest set of dependencies
-          # available. We may end up with the wrong revision of something for
-          # our platform, but this is the best we can do.
-          target_os_deps.update(
-              [x for x in os_deps.items() if not x[0] in target_os_deps])
-        else:
-          target_os_deps.update(os_deps)
+      if len(self.target_os) > 0:

[iannucci, 2013/09/16 22:54:40]

what values can target_os have? can it be None?

Would it work to just do `if 'deps_os' in local_scope and self.target_os:`?

+        the_first_os = self.target_os[0]
+        # |set_by_os| tracks what deps are overridden and by whom.
+        set_by_os = dict.fromkeys(deps.keys(), the_first_os)
+        for os_dep_key in deps.keys():
+          set_by_os[os_dep_key] = the_first_os

[iannucci, 2013/09/16 22:54:40]

Hm... Isn't this loop doing exactly the same thing as fromkeys?

+
+        os_deps = local_scope['deps_os'].get(the_first_os, {})
+        for os_dep_key, os_dep_value in os_deps.iteritems():
+          target_os_deps[os_dep_key] = os_dep_value
+          set_by_os[os_dep_key] = the_first_os
+
+        for the_target_os in self.target_os[1:]:
+          # Ignore (but warn) for any conflict when including deps for
+          # more than one platform, so we collect the broadest set of
+          # dependencies available. We may end up with the wrong
+          # revision of something for our platform, but this is the
+          # best we can do.
+          os_deps = local_scope['deps_os'].get(the_target_os, {})
+          for os_dep_key, os_dep_value in os_deps.iteritems():
+            prev_value = target_os_deps.get(os_dep_key, deps.get(os_dep_key))
+            if os_dep_key in set_by_os and prev_value is not None:
+              # Some platforms override with None just to create a
+              # smaller checkout. No reason to warn about that.
+              if os_dep_value is not None and os_dep_value != prev_value:
+                print('Conflicting dependencies for %s. %s wants %s '

[iannucci, 2013/09/16 22:54:40]

this should be logging.warning, probably

+                      'while %s wants %s. '
+                      '%s will be used since %s was listed before %s.' %
+                      (os_dep_key, the_target_os, os_dep_value,
+                       set_by_os[os_dep_key], prev_value,
+                       prev_value, set_by_os[os_dep_key], the_target_os))
+            else:
+              target_os_deps[os_dep_key] = os_dep_value
+              set_by_os[os_dep_key] = the_target_os
 
     # deps_os overrides paths from deps
     deps.update(target_os_deps)