Fix GAUDI-1105: Race condition when creating installation directories

I wrote a test case, but it's a bugger to reproduce as the race condition has to happen between lines 223 and 225. However, tweaking the original and patched `install.py` scripts to manually create the race (by doing `makedirs()` twice) I can show the patch works fine.

See merge request !46

GaudiPolicy/CMakeLists.txt

@@ -36,3 +36,8 @@ endif()
 gaudi_add_test(GaudiTesting.nose
                COMMAND nosetests --with-doctest
                  ${CMAKE_CURRENT_SOURCE_DIR}/python/GaudiTesting)
+
+gaudi_add_test(GAUDI-1055
+               COMMAND ./test_install.py
+               WORKING_DIRECTORY
+               ${CMAKE_CURRENT_SOURCE_DIR}/tests/GAUDI-1105)

GaudiPolicy/scripts/install.py

@@ -214,15 +214,15 @@ def update(src,dest,old_dest = None, syml = False, logdir = realpath(".")):
     realdest = normpath(join(logdir, dest))
     dest_path = split(realdest)[0]
     realsrc = normpath(join(dest_path,src))
-    # The modification time is compared only with the precision of the second
-    # to avoid a bug in Python 2.5 + Win32 (Fixed in Python 2.5.1).
-    # See:
-    #   http://bugs.python.org/issue1671965
-    #   http://bugs.python.org/issue1565150
-    if (not exists(realdest)) or (int(getmtime(realsrc)) > int(getmtime(realdest))):
-        if not isdir(dest_path):
-            print "Create dir '%s'"%(dest_path)
+    # To avoid race conditions on makedirs(), use EAFP (see GAUDI-1105)
+    if (not exists(realdest)) or (getmtime(realsrc) > getmtime(realdest)):
+        try:
             makedirs(dest_path)
+            print "Created dir '{0}'".format(dest_path)
+        except OSError as e:
+            # OSerror no. 17 is "file exists" - harmless as long as the directory is there
+            if not(e.errno == 17 and isdir(dest_path)):
+                raise
         # the destination file is missing or older than the source
         if syml and sys.platform != "win32" :
             if exists(realdest):
@@ -240,11 +240,7 @@ def update(src,dest,old_dest = None, syml = False, logdir = realpath(".")):
                 shutil.copy2(realsrc, realdest) # do the copy (cp -p src dest)
             else:
                 shutil.copy(realsrc, realdest) # do the copy (cp src dest)
-            
-    #if old_dest != dest: # the file was installed somewhere else
-    #    # remove the old destination
-    #    if old_dest is not None:
-    #        remove(old_dest,logdir)
+
 
 def install(sources, destination, logfile, exclusions = [],
             destname = None, syml = False, logdir = realpath(".")):

GaudiPolicy/tests/GAUDI-1105/test_install.py

+#! /usr/bin/env python
+
+## @Package test_install.py
+#  @brief Unittests for install.py
+
+import os
+import os.path
+import subprocess
+import unittest
+import uuid
+
+import logging
+msg = logging.getLogger(__name__)
+
+# Unittests for this module
+class installTests(unittest.TestCase):
+
+    def setUp(self):
+        self.fileToInstall = "testModule-" + str(uuid.uuid4()) + ".py"
+        with open(self.fileToInstall, "w") as fh:
+            fh.write('''#!/usr/bin/env python\npass\n''')
+
+        self.targetDir = "testModuleDir-" + str(uuid.uuid4())
+
+    def tearDown(self):
+        for path in (os.path.join(self.targetDir, self.fileToInstall), self.fileToInstall, "install.log"):
+            try:
+                os.unlink(path)
+            except OSError:
+                pass
+        try:
+            os.rmdir(self.targetDir)
+        except OSError:
+            pass
+    
+    def test_installToExisitingPath(self):
+        cmd = ["install.py", self.fileToInstall, self.targetDir]
+        rc = subprocess.call(cmd)
+        self.assertEqual(rc, 0)
+
+if __name__ == '__main__':
+    unittest.main()
+