From 86fe47d1a076563a664adaaf2ae9e4d8dbe86932 Mon Sep 17 00:00:00 2001
From: katthomas <katthomas@google.com>
Date: Mon, 31 Oct 2016 13:49:14 -0700
Subject: [PATCH] Make bot_update on win more resilient

Sometimes Windows has trouble deleting files. This can cause problems
when lockfiles are left in .git directories.

R=agable@google.com
BUG=659178

Review-Url: https://codereview.chromium.org/2454463002
---
 .../bot_update/resources/bot_update.py        | 31 +++++++++++++++++++
 tests/bot_update_coverage_test.py             | 21 +++++++++++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/recipe_modules/bot_update/resources/bot_update.py b/recipe_modules/bot_update/resources/bot_update.py
index aa7728947..3a5838cc6 100755
--- a/recipe_modules/bot_update/resources/bot_update.py
+++ b/recipe_modules/bot_update/resources/bot_update.py
@@ -469,6 +469,25 @@ def is_broken_repo_dir(repo_dir):
   return not path.exists(os.path.join(repo_dir, '.git', 'config'))
 
 
+def _maybe_break_locks(checkout_path):
+  """This removes all .lock files from this repo's .git directory.
+
+  In particular, this will cleanup index.lock files, as well as ref lock
+  files.
+  """
+  git_dir = os.path.join(checkout_path, '.git')
+  for dirpath, _, filenames in os.walk(git_dir):
+    for filename in filenames:
+      if filename.endswith('.lock'):
+        to_break = os.path.join(dirpath, filename)
+        print 'breaking lock: %s' % to_break
+        try:
+          os.remove(to_break)
+        except OSError as ex:
+          print 'FAILED to break lock: %s: %s' % (to_break, ex)
+          raise
+
+
 def git_checkout(solutions, revisions, shallow, refs, git_cache_dir):
   build_dir = os.getcwd()
   # Before we do anything, break all git_cache locks.
@@ -522,6 +541,18 @@ def git_checkout(solutions, revisions, shallow, refs, git_cache_dir):
           refspec = '%s:%s' % (ref, ref.lstrip('+'))
           git('fetch', 'origin', refspec, cwd=sln_dir)
 
+        # Windows sometimes has trouble deleting files.
+        # This can make git commands that rely on locks fail.
+        # Try a few times in case Windows has trouble again (and again).
+        if sys.platform.startswith('win'):
+          tries = 3
+          while tries:
+            try:
+              _maybe_break_locks(sln_dir)
+              break
+            except Exception:
+              tries -= 1
+
         revision = get_target_revision(name, url, revisions) or 'HEAD'
         force_revision(sln_dir, revision)
         done = True
diff --git a/tests/bot_update_coverage_test.py b/tests/bot_update_coverage_test.py
index 9b7715e4e..f808c633a 100755
--- a/tests/bot_update_coverage_test.py
+++ b/tests/bot_update_coverage_test.py
@@ -186,6 +186,10 @@ class BotUpdateUnittests(unittest.TestCase):
     delattr(bot_update, 'open')
     setattr(codecs, 'open', self.old_codecs_open)
 
+  def overrideSetupForWindows(self):
+    sys.platform = 'win'
+    self.call.expect(('gclient.bat', 'sync')).returns(self.gclient)
+
   def testBasic(self):
     bot_update.ensure_checkout(**self.params)
     return self.call.records
@@ -196,8 +200,7 @@ class BotUpdateUnittests(unittest.TestCase):
     return self.call.records
 
   def testBreakLocks(self):
-    sys.platform = 'win'
-    self.call.expect(('gclient.bat', 'sync')).returns(self.gclient)
+    self.overrideSetupForWindows()
     bot_update.ensure_checkout(**self.params)
     gclient_sync_cmd = None
     for record in self.call.records:
@@ -206,6 +209,20 @@ class BotUpdateUnittests(unittest.TestCase):
         gclient_sync_cmd = args
     self.assertTrue('--break_repo_locks' in gclient_sync_cmd)
 
+  def testGitCheckoutBreaksLocks(self):
+    self.overrideSetupForWindows()
+    path = '/b/build/slave/foo/build/.git'
+    lockfile = 'index.lock'
+    removed = []
+    old_os_walk = os.walk
+    old_os_remove = os.remove
+    setattr(os, 'walk', lambda _: [(path, None, [lockfile])])
+    setattr(os, 'remove', removed.append)
+    bot_update.ensure_checkout(**self.params)
+    setattr(os, 'walk', old_os_walk)
+    setattr(os, 'remove', old_os_remove)
+    self.assertTrue(os.path.join(path, lockfile) in removed)
+
 
 if __name__ == '__main__':
   unittest.main()