Fix #23, don't error if ASSETS_ROOT already existsdouble_create_assets

3 files changed, 45 insertions, 2 deletions

diff --git a/django_pyscss/scss.py b/django_pyscss/scss.py
index b4c5981..5aea4ac 100644
--- a/django_pyscss/scss.py
+++ b/django_pyscss/scss.py
@@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals
 
 import os
 from itertools import product
+import errno
 
 from django.contrib.staticfiles.storage import staticfiles_storage
 from django.conf import settings
@@ -24,6 +25,18 @@ config.ASSETS_ROOT = os.path.join(settings.STATIC_ROOT, 'scss', 'assets')
 config.ASSETS_URL = staticfiles_storage.url('scss/assets/')
 
 
+def idempotent_makedirs(path, *args, **kwargs):
+    """
+    os.makedirs throws an error if the directory already existed. This function
+    does not. See https://github.com/fusionbox/django-pyscss/issues/23
+    """
+    try:
+        os.makedirs(path, *args, **kwargs)
+    except OSError as e:
+        if e.errno != errno.EEXIST:
+            raise
+
+
 class DjangoScss(Scss):
     """
     A subclass of the Scss compiler that uses the storages API for accessing
@@ -157,8 +170,7 @@ class DjangoScss(Scss):
         Overwritten to call _find_source_file instead of
         SourceFile.from_filename.  Also added the relative_to option.
         """
-        if not os.path.exists(config.ASSETS_ROOT):
-            os.makedirs(config.ASSETS_ROOT)
+        idempotent_makedirs(config.ASSETS_ROOT)
         if super_selector:
             self.super_selector = super_selector + ' '
         self.reset()
diff --git a/setup.py b/setup.py
index 6cf31c7..b6881bd 100644
--- a/setup.py
+++ b/setup.py
@@ -2,6 +2,7 @@
 from setuptools import setup, find_packages
 import subprocess
 import os
+import sys
 
 __doc__ = "Makes it easier to use PySCSS in Django."
 
@@ -20,6 +21,8 @@ tests_require = [
     'django-discover-runner',
 ]
 
+if sys.version_info < (3, 3):
+    tests_require.append('mock')
 
 version = (1, 0, 0, 'alpha')
 
diff --git a/tests/test_scss.py b/tests/test_scss.py
index 78cd46e..6864be1 100644
--- a/tests/test_scss.py
+++ b/tests/test_scss.py
@@ -1,9 +1,12 @@
 import os
+import mock
+import tempfile
 
 from django.test import TestCase
 from django.test.utils import override_settings
 from django.conf import settings
 
+from scss import config
 from django_pyscss.scss import DjangoScss
 
 from tests.utils import clean_css, CollectStaticTestCase
@@ -141,3 +144,28 @@ class AssetsTest(CompilerTestMixin, TestCase):
         # pyScss puts a cachebuster query string on the end of the URLs, lets
         # just check that it made the file that we expected.
         self.assertIn('KUZdBAnPCdlG5qfocw9GYw.png', actual)
+
+    def test_mkdir_race_condition(self):
+        """
+        There is a race condition when different instances of DjangoScss are
+        instantiated in different threads.
+
+        See https://github.com/fusionbox/django-pyscss/issues/23
+
+        We simulate the race condition by mocking the return of os.path.exists.
+        """
+        old_assets_root = config.ASSETS_ROOT
+        try:
+            new_assets_root = tempfile.mkdtemp()
+            config.ASSETS_ROOT = new_assets_root
+
+            def return_false_once(*args, **kwargs):
+                patch.stop()
+                return False
+
+            patch = mock.patch('os.path.exists', new=return_false_once)
+            patch.start()
+            self.compiler.compile(scss_string=".test { color: red; }")
+        finally:
+            config.ASSETS_ROOT = old_assets_root
+            os.rmdir(new_assets_root)