Propagate use_relative_paths into recursedeps

Currently, if a DEPS file sets use_relative_paths, but *also* sets recursedeps, then the recursed-upon DEPS files still get checked out relative to the .gclient root. This change makes it so that recursed-upon DEPS files check their dependencies out relative to where their parent wants them to be, if that parent sets use_relative_paths=True. R=maruel@chromium.org Review-Url: https://codereview.chromium.org/2324513003
2026-01-11 18:51:29 +00:00 · 2016-09-08 10:02:16 -07:00
parent 73baf85348
commit dce6ddcd21
2 changed files with 90 additions and 37 deletions
--- a/gclient.py
+++ b/gclient.py
@@ -214,7 +214,7 @@ class DependencySettings(GClientKeywords):
  """Immutable configuration settings."""
  def __init__(
      self, parent, url, safesync_url, managed, custom_deps, custom_vars,
-      custom_hooks, deps_file, should_process):
+      custom_hooks, deps_file, should_process, relative):
    GClientKeywords.__init__(self)

    # These are not mutable:
@@ -230,6 +230,11 @@ class DependencySettings(GClientKeywords):
    # recursion limit and controls gclient's behavior so it does not misbehave.
    self._managed = managed
    self._should_process = should_process
+    # If this is a recursed-upon sub-dependency, and the parent has
+    # use_relative_paths set, then this dependency should check out its own
+    # dependencies relative to that parent's path for this, rather than
+    # relative to the .gclient file.
+    self._relative = relative
    # This is a mutable value which has the list of 'target_os' OSes listed in
    # the current deps file.
    self.local_target_os = None
@@ -325,11 +330,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
  """Object that represents a dependency checkout."""

  def __init__(self, parent, name, url, safesync_url, managed, custom_deps,
-               custom_vars, custom_hooks, deps_file, should_process):
+               custom_vars, custom_hooks, deps_file, should_process,
+               relative):
    gclient_utils.WorkItem.__init__(self, name)
    DependencySettings.__init__(
        self, parent, url, safesync_url, managed, custom_deps, custom_vars,
-        custom_hooks, deps_file, should_process)
+        custom_hooks, deps_file, should_process, relative)

    # This is in both .gclient and DEPS files:
    self._deps_hooks = []
@@ -686,25 +692,36 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
    # the dictionary using paths relative to the directory containing
    # the DEPS file.  Also update recursedeps if use_relative_paths is
    # enabled.
+    # If the deps file doesn't set use_relative_paths, but the parent did
+    # (and therefore set self.relative on this Dependency object), then we
+    # want to modify the deps and recursedeps by prepending the parent
+    # directory of this dependency.
    use_relative_paths = local_scope.get('use_relative_paths', False)
+    rel_prefix = None
    if use_relative_paths:
+      rel_prefix = self.name
+    elif self._relative:
+      rel_prefix = os.path.dirname(self.name)
+    if rel_prefix:
      logging.warning('use_relative_paths enabled.')
      rel_deps = {}
      for d, url in deps.items():
        # normpath is required to allow DEPS to use .. in their
        # dependency local path.
-        rel_deps[os.path.normpath(os.path.join(self.name, d))] = url
-      logging.warning('Updating deps by prepending %s.', self.name)
+        rel_deps[os.path.normpath(os.path.join(rel_prefix, d))] = url
+      logging.warning('Updating deps by prepending %s.', rel_prefix)
      deps = rel_deps

      # Update recursedeps if it's set.
      if self.recursedeps is not None:
-        logging.warning('Updating recursedeps by prepending %s.', self.name)
+        logging.warning('Updating recursedeps by prepending %s.', rel_prefix)
        rel_deps = {}
        for depname, options in self.recursedeps.iteritems():
-          rel_deps[os.path.normpath(os.path.join(self.name, depname))] = options
+          rel_deps[
+              os.path.normpath(os.path.join(rel_prefix, depname))] = options
        self.recursedeps = rel_deps

+
    if 'allowed_hosts' in local_scope:
      try:
        self._allowed_hosts = frozenset(local_scope.get('allowed_hosts'))
@@ -728,7 +745,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
          deps_file = ent['deps_file']
      deps_to_add.append(Dependency(
          self, name, url, None, None, None, self.custom_vars, None,
-          deps_file, should_process))
+          deps_file, should_process, use_relative_paths))
    deps_to_add.sort(key=lambda x: x.name)

    # override named sets of hooks by the custom hooks
@@ -1217,7 +1234,7 @@ solutions = [
    # are processed.
    self._recursion_limit = 2
    Dependency.__init__(self, None, None, None, None, True, None, None, None,
-                        'unused', True)
+                        'unused', True, None)
    self._options = options
    if options.deps_os:
      enforced_os = options.deps_os.split(',')
@@ -1308,7 +1325,8 @@ want to set 'managed': False in .gclient.
            s.get('custom_vars', {}),
            s.get('custom_hooks', []),
            s.get('deps_file', 'DEPS'),
-            True))
+            True,
+            None))
      except KeyError:
        raise gclient_utils.Error('Invalid .gclient file. Solution is '
                                  'incomplete: %s' % s)
--- a/tests/gclient_test.py
+++ b/tests/gclient_test.py
@@ -220,7 +220,7 @@ class GclientTest(trial_dir.TestCase):
    # auto-fixed.
    d = gclient.Dependency(
        None, 'name', 'proto://host/path/@revision', None, None, None, None,
-        None, '', True)
+        None, '', True, False)
    self.assertEquals('proto://host/path@revision', d.url)

  def testStr(self):
@@ -228,28 +228,28 @@ class GclientTest(trial_dir.TestCase):
    options, _ = parser.parse_args([])
    obj = gclient.GClient('foo', options)
    obj.add_dependencies_and_close(
-        [
-          gclient.Dependency(
-            obj, 'foo', 'url', None, None, None, None, None, 'DEPS', True),
-          gclient.Dependency(
-            obj, 'bar', 'url', None, None, None, None, None, 'DEPS', True),
-        ],
-        [])
+      [
+        gclient.Dependency(
+          obj, 'foo', 'url', None, None, None, None, None, 'DEPS', True, False),
+        gclient.Dependency(
+          obj, 'bar', 'url', None, None, None, None, None, 'DEPS', True, False),
+      ],
+      [])
    obj.dependencies[0].add_dependencies_and_close(
-        [
-          gclient.Dependency(
-            obj.dependencies[0], 'foo/dir1', 'url', None, None, None, None,
-            None, 'DEPS', True),
-          gclient.Dependency(
-            obj.dependencies[0], 'foo/dir2',
-            gclient.GClientKeywords.FromImpl('bar'), None, None, None, None,
-            None, 'DEPS', True),
-          gclient.Dependency(
-            obj.dependencies[0], 'foo/dir3',
-            gclient.GClientKeywords.FileImpl('url'), None, None, None, None,
-            None, 'DEPS', True),
-        ],
-        [])
+      [
+        gclient.Dependency(
+          obj.dependencies[0], 'foo/dir1', 'url', None, None, None, None,
+          None, 'DEPS', True, False),
+        gclient.Dependency(
+          obj.dependencies[0], 'foo/dir2',
+          gclient.GClientKeywords.FromImpl('bar'), None, None, None, None,
+          None, 'DEPS', True, False),
+        gclient.Dependency(
+          obj.dependencies[0], 'foo/dir3',
+          gclient.GClientKeywords.FileImpl('url'), None, None, None, None,
+          None, 'DEPS', True, False),
+      ],
+      [])
    # Make sure __str__() works fine.
    # pylint: disable=W0212
    obj.dependencies[0]._file_list.append('foo')
@@ -567,7 +567,7 @@ class GclientTest(trial_dir.TestCase):
    """Verifies expected behavior of LateOverride."""
    url = "git@github.com:dart-lang/spark.git"
    d = gclient.Dependency(None, 'name', 'url',
-                           None, None, None, None, None, '', True)
+                           None, None, None, None, None, '', True, False)
    late_url = d.LateOverride(url)
    self.assertEquals(url, late_url)

@@ -739,7 +739,7 @@ class GclientTest(trial_dir.TestCase):
        '}\n'
        'recursedeps = ["bar"]')
    write(
-        os.path.join('bar', 'DEPS'),
+        os.path.join('foo/bar', 'DEPS'),
        'deps = {\n'
        '  "baz": "/baz",\n'
        '}')
@@ -756,9 +756,44 @@ class GclientTest(trial_dir.TestCase):
        [
          ('foo', 'svn://example.com/foo'),
          ('foo/bar', 'svn://example.com/foo/bar'),
-          # TODO(agable): Figure out why baz isn't included here. The
-          # recursedeps = ["bar"] in foo's DEPS means that we should be
-          # fetching the entries in bar's DEPS file, which includes baz.
+          ('foo/baz', 'svn://example.com/foo/bar/baz'),
+        ],
+        self._get_processed())
+
+  def testRelativeRecursion(self):
+    """Verifies that nested use_relative_paths is always respected."""
+    write(
+        '.gclient',
+        'solutions = [\n'
+        '  { "name": "foo", "url": "svn://example.com/foo" },\n'
+          ']')
+    write(
+        os.path.join('foo', 'DEPS'),
+        'use_relative_paths = True\n'
+        'deps = {\n'
+        '  "bar": "/bar",\n'
+        '}\n'
+        'recursedeps = ["bar"]')
+    write(
+        os.path.join('foo/bar', 'DEPS'),
+        'use_relative_paths = True\n'
+        'deps = {\n'
+        '  "baz": "/baz",\n'
+        '}')
+    write(
+        os.path.join('baz', 'DEPS'),
+        'deps = {\n'
+        '  "fizz": "/fizz",\n'
+        '}')
+
+    options, _ = gclient.OptionParser().parse_args([])
+    obj = gclient.GClient.LoadCurrentConfig(options)
+    obj.RunOnDeps('None', [])
+    self.assertEquals(
+        [
+          ('foo', 'svn://example.com/foo'),
+          ('foo/bar', 'svn://example.com/foo/bar'),
+          ('foo/bar/baz', 'svn://example.com/foo/bar/baz'),
        ],
        self._get_processed())