# HG changeset patch # User Adam Simpkins # Date 2016-04-26 22:32:59 # Node ID 07be86828e795278abada785da3861b3c38c80e2 # Parent 94451300f3ec4a81135d3cf86ca3dce13e53837b util: fix race in makedirs() Update makedirs() to ignore EEXIST in case someone else has already created the directory in question. Previously the ensuredirs() function existed, and was nearly identical to makedirs() except that it fixed this race. Unfortunately ensuredirs() was only used in 3 places, and most code uses the racy makedirs() function. This fixes makedirs() to be non-racy, and replaces calls to ensuredirs() with makedirs(). In particular, mercurial.scmutil.origpath() used the racy makedirs() code, which could cause failures during "hg update" as it tried to create backup directories. This does slightly change the behavior of call sites using ensuredirs(): previously ensuredirs() would throw EEXIST if the path existed but was a regular file instead of a directory. It did this by explicitly checking os.path.isdir() after getting EEXIST. The makedirs() code did not do this and swallowed all EEXIST errors. I kept the makedirs() behavior, since it seemed preferable to avoid the extra stat call in the common case where this directory already exists. If the path does happen to be a file, the caller will almost certainly fail with an ENOTDIR error shortly afterwards anyway. I checked the 3 existing call sites of ensuredirs(), and this seems to be the case for them. diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -539,7 +539,7 @@ class vfs(abstractvfs): # to a directory. Let the posixfile() call below raise IOError. if basename: if atomictemp: - util.ensuredirs(dirname, self.createmode, notindexed) + util.makedirs(dirname, self.createmode, notindexed) return util.atomictempfile(f, mode, self.createmode) try: if 'w' in mode: @@ -556,7 +556,7 @@ class vfs(abstractvfs): if e.errno != errno.ENOENT: raise nlink = 0 - util.ensuredirs(dirname, self.createmode, notindexed) + util.makedirs(dirname, self.createmode, notindexed) if nlink > 0: if self._trustnlink is None: self._trustnlink = nlink > 1 or util.checknlink(f) @@ -583,7 +583,7 @@ class vfs(abstractvfs): except OSError: pass - util.ensuredirs(os.path.dirname(linkname), self.createmode) + util.makedirs(os.path.dirname(linkname), self.createmode) if self._cansymlink: try: diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -1420,7 +1420,12 @@ class atomictempfile(object): self.discard() def makedirs(name, mode=None, notindexed=False): - """recursive directory creation with parent mode inheritance""" + """recursive directory creation with parent mode inheritance + + Newly created directories are marked as "not to be indexed by + the content indexing service", if ``notindexed`` is specified + for "write" mode access. + """ try: makedir(name, notindexed) except OSError as err: @@ -1432,29 +1437,13 @@ def makedirs(name, mode=None, notindexed if parent == name: raise makedirs(parent, mode, notindexed) - makedir(name, notindexed) - if mode is not None: - os.chmod(name, mode) - -def ensuredirs(name, mode=None, notindexed=False): - """race-safe recursive directory creation - - Newly created directories are marked as "not to be indexed by - the content indexing service", if ``notindexed`` is specified - for "write" mode access. - """ - if os.path.isdir(name): - return - parent = os.path.dirname(os.path.abspath(name)) - if parent != name: - ensuredirs(parent, mode, notindexed) - try: - makedir(name, notindexed) - except OSError as err: - if err.errno == errno.EEXIST and os.path.isdir(name): - # someone else seems to have won a directory creation race - return - raise + try: + makedir(name, notindexed) + except OSError as err: + # Catch EEXIST to handle races + if err.errno == errno.EEXIST: + return + raise if mode is not None: os.chmod(name, mode)