Clean up gclient MergeDependencies

gclient.py

Index: gclient.py
diff --git a/gclient.py b/gclient.py
index e2491cef1d39b1a80e6c6fcb3ae25fab676696bc..6c3072fe6119c60d5cf2ebd733a27bf6c531f75f 100755
--- a/gclient.py
+++ b/gclient.py
@@ -104,7 +104,7 @@ from third_party import colorama
 
 
 class GClientKeywords(object):
-  class FromImpl(object):
+  class FromImpl(gclient_utils.BaseRecord):
     """Used to implement the From() syntax."""
 
     def __init__(self, module_name, sub_target_name=None):
@@ -120,7 +120,7 @@ class GClientKeywords(object):
       return 'From(%s, %s)' % (repr(self.module_name),
                                repr(self.sub_target_name))
 
-  class FileImpl(object):
+  class FileImpl(gclient_utils.BaseRecord):
     """Used to implement the File('') syntax which lets you sync a single file
     from a SVN repo."""
 
@@ -143,7 +143,7 @@ class GClientKeywords(object):
         return rev_tokens[1]
       return None
 
-  class VarImpl(object):
+  class VarImpl(gclient_utils.BaseRecord):
     def __init__(self, custom_vars, local_scope):
       self._custom_vars = custom_vars
       self._local_scope = local_scope
@@ -472,39 +472,28 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     with information from deps_os (the deps_os section of the DEPS
     file) that matches the list of target os."""
     os_overrides = {}
-    for the_target_os in target_os_list:
+    # Sort target_os_list to ensure consistent conflict resolution.
+    for the_target_os in sorted(target_os_list):
       the_target_os_deps = deps_os.get(the_target_os, {})
       for os_dep_key, os_dep_value in the_target_os_deps.iteritems():
         overrides = os_overrides.setdefault(os_dep_key, [])
         overrides.append((the_target_os, os_dep_value))
 
-    # If any os didn't specify a value (we have fewer value entries
-    # than in the os list), then it wants to use the default value.
-    for os_dep_key, os_dep_value in os_overrides.iteritems():
-      if len(os_dep_value) != len(target_os_list):
-        # Record the default value too so that we don't accidently
-        # set it to None or miss a conflicting DEPS.
-        if os_dep_key in deps:
-          os_dep_value.append(('default', deps[os_dep_key]))
-
     target_os_deps = {}
-    for os_dep_key, os_dep_value in os_overrides.iteritems():
-      # os_dep_value is a list of (os, value) pairs.
-      possible_values = set(x[1] for x in os_dep_value if x[1] is not None)
-      if not possible_values:
-        target_os_deps[os_dep_key] = None
-      else:
-        if len(possible_values) > 1:
-          # It would be possible to abort here but it would be
-          # unfortunate if we end up preventing any kind of checkout.
-          logging.error('Conflicting dependencies for %s: %s. (target_os=%s)',
-                        os_dep_key, os_dep_value, target_os_list)
-        # Sorting to get the same result every time in case of conflicts.
-        target_os_deps[os_dep_key] = sorted(possible_values)[0]
-
-    new_deps = deps.copy()
-    new_deps.update(target_os_deps)
-    return new_deps
+    for os_dep_key, os_dep_values in os_overrides.iteritems():
+      if len(os_dep_values) < len(target_os_list):
+        # A targeted os is using the default val for this dep.
+        # Preferentially use this value when resolving conflicts.
+        os_dep_values.append(('default', deps.get(os_dep_key)))
+      all_vals = [val for _, val in os_dep_values if val is not None]
+      if len(set(all_vals)) > 1:
+        # It would be possible to abort here but it would be
+        # unfortunate if we end up preventing any kind of checkout.
+        logging.error('Conflicting dependencies for %s: %s. (target_os=%s)',
+                      os_dep_key, os_dep_values, target_os_list)
+      # Last non-None dep val; else None
+      target_os_deps[os_dep_key] = all_vals[-1] if all_vals else None
+    return dict(deps, **target_os_deps)
 
   def ParseDepsFile(self):
     """Parses the DEPS file for this dependency."""