Commit 6c48a304 authored by maruel@chromium.org's avatar maruel@chromium.org

Fix a concurrency issue happening with deep dependencies when the intermediary

directory doesn't exist, on fresh checkout and --jobs >1.

It happens in the case where there is 3 dependencies:
a
a/b/c/d
a/b/c/e

'a' is processed first. Then 'a/b/c/d' and 'a/b/c/e' are fired simultaneously.
If both are not present, both svn checkout tries to mkdir b and b/c and a race
condition occurs between their call for isdir() and mkdir().

This works around the issue by creating the intermediary directories ourself
before calling out svn and managing the error ourself.

TBR=dpranke@chromium.org
BUG=
TEST=fresh checkout shouldn't be flaky


Review URL: http://codereview.chromium.org/8359018

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@106636 0039d316-1c4b-4281-b951-d872f2087c98
parent 97335088
...@@ -213,6 +213,7 @@ class GitWrapper(SCMWrapper): ...@@ -213,6 +213,7 @@ class GitWrapper(SCMWrapper):
rev_type = "hash" rev_type = "hash"
if not os.path.exists(self.checkout_path): if not os.path.exists(self.checkout_path):
gclient_utils.safe_makedirs(os.path.dirname(self.checkout_path))
self._Clone(revision, url, options) self._Clone(revision, url, options)
files = self._Capture(['ls-files']).splitlines() files = self._Capture(['ls-files']).splitlines()
file_list.extend([os.path.join(self.checkout_path, f) for f in files]) file_list.extend([os.path.join(self.checkout_path, f) for f in files])
...@@ -508,7 +509,7 @@ class GitWrapper(SCMWrapper): ...@@ -508,7 +509,7 @@ class GitWrapper(SCMWrapper):
# create it, so we need to do it manually. # create it, so we need to do it manually.
parent_dir = os.path.dirname(self.checkout_path) parent_dir = os.path.dirname(self.checkout_path)
if not os.path.exists(parent_dir): if not os.path.exists(parent_dir):
os.makedirs(parent_dir) gclient_utils.safe_makedirs(parent_dir)
percent_re = re.compile('.* ([0-9]{1,2})% .*') percent_re = re.compile('.* ([0-9]{1,2})% .*')
def _GitFilter(line): def _GitFilter(line):
...@@ -775,6 +776,7 @@ class SVNWrapper(SCMWrapper): ...@@ -775,6 +776,7 @@ class SVNWrapper(SCMWrapper):
rev_str = '' rev_str = ''
if not os.path.exists(self.checkout_path): if not os.path.exists(self.checkout_path):
gclient_utils.safe_makedirs(os.path.dirname(self.checkout_path))
# We need to checkout. # We need to checkout.
command = ['checkout', url, self.checkout_path] command = ['checkout', url, self.checkout_path]
command = self._AddAdditionalUpdateFlags(command, options, revision) command = self._AddAdditionalUpdateFlags(command, options, revision)
...@@ -906,7 +908,7 @@ class SVNWrapper(SCMWrapper): ...@@ -906,7 +908,7 @@ class SVNWrapper(SCMWrapper):
# information is not stored next to the file, so we will have to # information is not stored next to the file, so we will have to
# re-export the file every time we sync. # re-export the file every time we sync.
if not os.path.exists(self.checkout_path): if not os.path.exists(self.checkout_path):
os.makedirs(self.checkout_path) gclient_utils.safe_makedirs(self.checkout_path)
command = ["export", os.path.join(self.url, filename), command = ["export", os.path.join(self.url, filename),
os.path.join(self.checkout_path, filename)] os.path.join(self.checkout_path, filename)]
command = self._AddAdditionalUpdateFlags(command, options, command = self._AddAdditionalUpdateFlags(command, options,
......
...@@ -171,6 +171,26 @@ def rmtree(path): ...@@ -171,6 +171,26 @@ def rmtree(path):
RemoveDirectory = rmtree RemoveDirectory = rmtree
def safe_makedirs(tree):
"""Creates the directory in a safe manner.
Because multiple threads can create these directories concurently, trap the
exception and pass on.
"""
count = 0
while not os.path.exists(tree):
count += 1
try:
os.makedirs(tree)
except OSError, e:
# 17 POSIX, 183 Windows
if e.errno not in (17, 183):
raise
if count > 40:
# Give up.
raise
def CheckCallAndFilterAndHeader(args, always=False, **kwargs): def CheckCallAndFilterAndHeader(args, always=False, **kwargs):
"""Adds 'header' support to CheckCallAndFilter. """Adds 'header' support to CheckCallAndFilter.
......
...@@ -142,6 +142,10 @@ class SVNWrapperTestCase(BaseTestCase): ...@@ -142,6 +142,10 @@ class SVNWrapperTestCase(BaseTestCase):
gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False) gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False)
# Checkout. # Checkout.
gclient_scm.os.path.exists(self.base_path).AndReturn(False) gclient_scm.os.path.exists(self.base_path).AndReturn(False)
parent = gclient_scm.os.path.dirname(self.base_path)
gclient_scm.os.path.exists(parent).AndReturn(False)
gclient_scm.os.makedirs(parent)
gclient_scm.os.path.exists(parent).AndReturn(True)
files_list = self.mox.CreateMockAnything() files_list = self.mox.CreateMockAnything()
gclient_scm.scm.SVN.RunAndGetFileList( gclient_scm.scm.SVN.RunAndGetFileList(
options.verbose, options.verbose,
...@@ -167,6 +171,10 @@ class SVNWrapperTestCase(BaseTestCase): ...@@ -167,6 +171,10 @@ class SVNWrapperTestCase(BaseTestCase):
gclient_scm.os.path.exists(join(self.base_path, '.git')).AndReturn(False) gclient_scm.os.path.exists(join(self.base_path, '.git')).AndReturn(False)
gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False) gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False)
gclient_scm.os.path.exists(self.base_path).AndReturn(False) gclient_scm.os.path.exists(self.base_path).AndReturn(False)
parent = gclient_scm.os.path.dirname(self.base_path)
gclient_scm.os.path.exists(parent).AndReturn(False)
gclient_scm.os.makedirs(parent)
gclient_scm.os.path.exists(parent).AndReturn(True)
files_list = self.mox.CreateMockAnything() files_list = self.mox.CreateMockAnything()
gclient_scm.scm.SVN.Capture(['--version']).AndReturn('svn, version 1.6') gclient_scm.scm.SVN.Capture(['--version']).AndReturn('svn, version 1.6')
gclient_scm.scm.SVN.RunAndGetFileList( gclient_scm.scm.SVN.RunAndGetFileList(
...@@ -255,6 +263,10 @@ class SVNWrapperTestCase(BaseTestCase): ...@@ -255,6 +263,10 @@ class SVNWrapperTestCase(BaseTestCase):
gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False) gclient_scm.os.path.exists(join(self.base_path, '.hg')).AndReturn(False)
# Checkout. # Checkout.
gclient_scm.os.path.exists(self.base_path).AndReturn(False) gclient_scm.os.path.exists(self.base_path).AndReturn(False)
parent = gclient_scm.os.path.dirname(self.base_path)
gclient_scm.os.path.exists(parent).AndReturn(False)
gclient_scm.os.makedirs(parent)
gclient_scm.os.path.exists(parent).AndReturn(True)
files_list = self.mox.CreateMockAnything() files_list = self.mox.CreateMockAnything()
gclient_scm.scm.SVN.Capture(['--version'] gclient_scm.scm.SVN.Capture(['--version']
).AndReturn('svn, version 1.5.1 (r32289)') ).AndReturn('svn, version 1.5.1 (r32289)')
......
...@@ -36,7 +36,7 @@ class GclientUtilsUnittest(GclientUtilBase): ...@@ -36,7 +36,7 @@ class GclientUtilsUnittest(GclientUtilBase):
'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision', 'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision',
'SyntaxErrorToError', 'WorkItem', 'SyntaxErrorToError', 'WorkItem',
'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree', 'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree',
'stat', 'subprocess2', 'sys','threading', 'time', 'safe_makedirs', 'stat', 'subprocess2', 'sys','threading', 'time',
] ]
# If this test fails, you should add the relevant test. # If this test fails, you should add the relevant test.
self.compareMembers(gclient_utils, members) self.compareMembers(gclient_utils, members)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment