Simplify ensuredir() with Python3 stdlib features

- Simplify ensuredir() to equivalent os.makedir(name, exist_ok=True)
- Do not check if a directory exists before calling
  ensuredir() (ensuredir() already handles it)
- Add exist_ok argument to path.makedirs() to follow same pattern
- Drop unnecessary .exists() check immediately before .isdir()
- Add tests for ensuredir

sphinx/cmd/quickstart.py

@@ -109,7 +109,7 @@ class ValidationError(Exception):
 def is_path(x):
     # type: (unicode) -> unicode
     x = path.expanduser(x)
-    if path.exists(x) and not path.isdir(x):
+    if not path.isdir(x):
         raise ValidationError(__("Please enter a valid path name."))
     return x
 
@@ -405,8 +405,7 @@ def generate(d, overwrite=True, silent=False, templatedir=None):
                 'version', 'release', 'master'):
         d[key + '_str'] = d[key].replace('\\', '\\\\').replace("'", "\\'")
 
-    if not path.isdir(d['path']):
-        ensuredir(d['path'])
+    ensuredir(d['path'])
 
     srcdir = d['sep'] and path.join(d['path'], 'source') or d['path']
 

sphinx/testing/path.py

@@ -202,12 +202,12 @@ class path(text_type):
         """
         return os.path.lexists(self)
 
-    def makedirs(self, mode=0o777):
-        # type: (int) -> None
+    def makedirs(self, mode=0o777, exist_ok=False):
+        # type: (int, bool) -> None
         """
         Recursively create directories.
         """
-        os.makedirs(self, mode)
+        os.makedirs(self, mode, exist_ok=exist_ok)  # type: ignore
 
     def joinpath(self, *args):
         # type: (Any) -> path

sphinx/testing/util.py

@@ -116,11 +116,9 @@ class SphinxTestApp(application.Sphinx):
         builddir = srcdir / '_build'
         confdir = srcdir
         outdir = builddir.joinpath(buildername)
-        if not outdir.isdir():
-            outdir.makedirs()
+        outdir.makedirs(exist_ok=True)
         doctreedir = builddir.joinpath('doctrees')
-        if not doctreedir.isdir():
-            doctreedir.makedirs()
+        doctreedir.makedirs(exist_ok=True)
         if confoverrides is None:
             confoverrides = {}
         warningiserror = False

sphinx/util/__init__.py

@@ -218,8 +218,7 @@ def copy_static_entry(source, targetdir, builder, context={},
     if path.isfile(source):
         copy_asset_file(source, targetdir, context, builder.templates)
     elif path.isdir(source):
-        if not path.isdir(targetdir):
-            os.mkdir(targetdir)
+        ensuredir(targetdir)
         for entry in os.listdir(source):
             if entry.startswith('.'):
                 continue

sphinx/util/fileutil.py

@@ -39,7 +39,7 @@ def copy_asset_file(source, destination, context=None, renderer=None):
     if not os.path.exists(source):
         return
 
-    if os.path.exists(destination) and os.path.isdir(destination):
+    if os.path.isdir(destination):
         # Use source filename if destination points a directory
         destination = os.path.join(destination, os.path.basename(source))
 

sphinx/util/osutil.py

@@ -82,13 +82,7 @@ def relative_uri(base, to):
 def ensuredir(path):
     # type: (unicode) -> None
     """Ensure that a path exists."""
-    try:
-        os.makedirs(path)
-    except OSError:
-        # If the path is already an existing directory (not a file!),
-        # that is OK.
-        if not os.path.isdir(path):
-            raise
+    os.makedirs(path, exist_ok=True)  # type: ignore
 
 
 def walk(top, topdown=True, followlinks=False):

tests/test_util.py

@@ -9,12 +9,15 @@
     :license: BSD, see LICENSE for details.
 """
 
+import os
+import tempfile
+
 import pytest
 from mock import patch
 
 from sphinx.testing.util import strip_escseq
 from sphinx.util import (
-    display_chunk, encode_uri, parselinenos, status_iterator, xmlname_checker
+    display_chunk, encode_uri, ensuredir, parselinenos, status_iterator, xmlname_checker
 )
 from sphinx.util import logging
 
@@ -34,6 +37,20 @@ def test_encode_uri():
     assert expected, encode_uri(uri)
 
 
+def test_ensuredir():
+    with tempfile.TemporaryDirectory() as tmp_path:
+        # Does not raise an exception for an existing directory.
+        ensuredir(tmp_path)
+
+        path = os.path.join(tmp_path, 'a', 'b', 'c')
+        ensuredir(path)
+        assert os.path.isdir(path)
+
+    with tempfile.NamedTemporaryFile() as tmp:
+        with pytest.raises(OSError):
+            ensuredir(tmp.name)
+
+
 def test_display_chunk():
     assert display_chunk('hello') == 'hello'
     assert display_chunk(['hello']) == 'hello'