Do not write gclient_gn_args_file too early.

Previously, gclient would attempt to write an args file after
a dependency was checked out, but before any sub-dependencies
had been checked out. If the args file path pointed at something
inside a sub-dependency, this wouldn't work, because the directory
might not yet exist. This most obviously happened for buildspec
clobber builds.

The fix is to wait until after the sub-dependencies have been
checked out to write the file.

R=phajdan.jr@chromium.org, mmoss@chromium.org
BUG=773933

Change-Id: I0cf4564204f7dabd9f843dc7904db7050fcc0d23
Reviewed-on: https://chromium-review.googlesource.com/714644Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>

gclient.py

@@ -893,8 +893,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     # Always parse the DEPS file.
     self.ParseDepsFile()
-    if self._gn_args_file and command == 'update':
-      self.WriteGNArgsFile()
     self._run_is_done(file_list or [], parsed_url)
     if command in ('update', 'revert') and not options.noprehooks:
       self.RunPreDepsHooks()
@@ -961,6 +959,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
         else:
           print('Skipped missing %s' % cwd, file=sys.stderr)
 
+  def HasGNArgsFile(self):
+    return self._gn_args_file is not None
+
   def WriteGNArgsFile(self):
     lines = ['# Generated from %r' % self.deps_file]
     variables = self.get_vars()
@@ -1006,6 +1007,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       result.extend(s.GetHooks(options))
     return result
 
+  def WriteGNArgsFilesRecursively(self, dependencies):
+    for dep in dependencies:
+      if dep.HasGNArgsFile():
+        dep.WriteGNArgsFile()
+      self.WriteGNArgsFilesRecursively(dep.dependencies)
+
   def RunHooksRecursively(self, options):
     assert self.hooks_ran == False
     self._hooks_ran = True
@@ -1478,8 +1485,11 @@ it or fix the checkout.
       print('Please fix your script, having invalid --revision flags will soon '
             'considered an error.', file=sys.stderr)
 
-    # Once all the dependencies have been processed, it's now safe to run the
-    # hooks.
+    # Once all the dependencies have been processed, it's now safe to write
+    # out any gn_args_files and run the hooks.
+    if command == 'update':
+      self.WriteGNArgsFilesRecursively(self.dependencies)
+
     if not self._options.nohooks:
       self.RunHooksRecursively(self._options)
 

testing_support/fake_repos.py

@@ -336,7 +336,9 @@ vars = {
   'str_var': 'abc',
   'cond_var': 'false_str_var and true_var',
 }
-gclient_gn_args_file = 'src/gclient.args'
+# Nest the args file in a sub-repo, to make sure we don't try to
+# write it before we've cloned everything.
+gclient_gn_args_file = 'src/repo2/gclient.args'
 gclient_gn_args = [
   'false_var',
   'false_str_var',
@@ -489,7 +491,7 @@ vars = {
   'cond_var': 'false_str_var and true_var',
 }
 
-gclient_gn_args_file = 'src/gclient.args'
+gclient_gn_args_file = 'src/repo2/gclient.args'
 gclient_gn_args = [
   'false_var',
   'false_str_var',

tests/gclient_smoketest.py

@@ -343,7 +343,7 @@ class GClientSmokeGIT(GClientSmokeBase):
                                 ('repo_3@1', 'src/repo2/repo3'),
                                 ('repo_4@2', 'src/repo4'))
     tree['src/git_hooked2'] = 'git_hooked2'
-    tree['src/gclient.args'] = '\n'.join([
+    tree['src/repo2/gclient.args'] = '\n'.join([
         '# Generated from \'DEPS\'',
         'false_var = false',
         'false_str_var = false',
@@ -364,7 +364,7 @@ class GClientSmokeGIT(GClientSmokeBase):
                                 ('repo_4@2', 'src/repo4'))
     tree['src/git_hooked1'] = 'git_hooked1'
     tree['src/git_hooked2'] = 'git_hooked2'
-    tree['src/gclient.args'] = '\n'.join([
+    tree['src/repo2/gclient.args'] = '\n'.join([
         '# Generated from \'DEPS\'',
         'false_var = false',
         'false_str_var = false',
@@ -406,7 +406,7 @@ class GClientSmokeGIT(GClientSmokeBase):
                                 ('repo_2@2', 'src/repo2'),
                                 ('repo_3@1', 'src/repo2/repo3'),
                                 ('repo_4@2', 'src/repo4'))
-    tree['src/gclient.args'] = '\n'.join([
+    tree['src/repo2/gclient.args'] = '\n'.join([
         '# Generated from \'DEPS\'',
         'false_var = false',
         'false_str_var = false',
@@ -451,7 +451,7 @@ class GClientSmokeGIT(GClientSmokeBase):
                                 ('repo_3@1', 'src/repo2/repo3'),
                                 ('repo_4@2', 'src/repo4'))
     tree['src/git_hooked2'] = 'git_hooked2'
-    tree['src/gclient.args'] = '\n'.join([
+    tree['src/repo2/gclient.args'] = '\n'.join([
         '# Generated from \'DEPS\'',
         'false_var = false',
         'false_str_var = false',
@@ -473,7 +473,7 @@ class GClientSmokeGIT(GClientSmokeBase):
                                 ('repo_4@2', 'src/repo4'))
     tree['src/git_hooked1'] = 'git_hooked1'
     tree['src/git_hooked2'] = 'git_hooked2'
-    tree['src/gclient.args'] = '\n'.join([
+    tree['src/repo2/gclient.args'] = '\n'.join([
         '# Generated from \'DEPS\'',
         'false_var = false',
         'false_str_var = false',
@@ -659,7 +659,7 @@ class GClientSmokeGIT(GClientSmokeBase):
 
     self.maxDiff = None
     self.assertEqual([
-        'gclient_gn_args_file = "src/gclient.args"',
+        'gclient_gn_args_file = "src/repo2/gclient.args"',
         'gclient_gn_args = [\'false_var\', \'false_str_var\', \'true_var\', '
             '\'true_str_var\', \'str_var\', \'cond_var\']',
         'allowed_hosts = [',
@@ -833,7 +833,7 @@ class GClientSmokeGIT(GClientSmokeBase):
       deps_contents = f.read()
 
     self.assertEqual([
-        'gclient_gn_args_file = "src/gclient.args"',
+        'gclient_gn_args_file = "src/repo2/gclient.args"',
         'gclient_gn_args = [\'false_var\', \'false_str_var\', \'true_var\', '
             '\'true_str_var\', \'str_var\', \'cond_var\']',
         'allowed_hosts = [',