Merge lp:~florian-rathgeber/instant/paths-module-fixes into lp:instant

[Florian Rathgeber]

A few small fixes for the paths utility module:

* Fix docstrings
* Add helper function for checking and creating directories that don't yet exist
* Call instant_error when directory creation fails

[Johan Hake (johan-hake)]

Thanks!

The if test for generating the cache dir might fail for clusters, for
jobs submitted to several nodes. For this purpose there is a function in
build.py called makedirs, which fails alright, but silently so.

If you do not mind can you use this function for the purpose of this
patch? It is also more logical to move makedirs to the paths.py module.
If you could fix that too it would be excellent!

Johan

On 03/17/2012 08:10 PM, Florian Rathgeber wrote:
> Florian Rathgeber has proposed merging
> lp:~florian-rathgeber/instant/paths-module-fixes into lp:instant.
>
> Requested reviews: Instant Core Team (instant-core)
>
> For more details, see:
> https://code.launchpad.net/~florian-rathgeber/instant/paths-module-fixes/+merge/98065
>
> A few small fixes for the paths utility module:
>
> * Fix docstrings * Add helper function for checking and creating
> directories that don't yet exist * Call instant_error when directory
> creation fails

[lp:~florian-rathgeber/instant/paths-module-fixes]

335. By Florian Rathgeber on 2012-03-19

Move makedirs helper function to paths module and use that instead of _check_or_create

336. By Florian Rathgeber on 2012-03-19

Use debug log level in paths module test

337. By Florian Rathgeber on 2012-03-19

Ignore empty string when reading cache/error dir from environment

[Florian Rathgeber (florian-rathgeber)]

I hadn't seen this function was already available, sorry. The changes I just pushed use it in the way you suggest.

[Johan Hake (johan-hake)]

Well, it was present in the "wrong" module so not your fault. Thanks again for the patch. It is now applied, tested and pushed.

=== modified file 'instant/build.py'
--- instant/build.py	2011-11-28 23:57:11 +0000
+++ instant/build.py	2012-03-19 19:59:16 +0000
@@ -35,16 +35,6 @@
         x = x.split()
     return strip_strings(x)
 
-def makedirs(path):
-    """
-    Creates a directory (tree). If directory already excists it does nothing.
-    """
-    try:
-        os.makedirs(path)
-    except os.error, e:
-        if e.errno != errno.EEXIST:
-            raise
-
 def copy_files(source, dest, files):
     """Copy a list of files from a source directory to a destination directory.
     This may seem a bit complicated, but a lot of this code is error checking."""

=== modified file 'instant/paths.py'
--- instant/paths.py	2011-06-18 05:32:06 +0000
+++ instant/paths.py	2012-03-19 19:59:16 +0000
@@ -3,6 +3,7 @@
 # Utilities for directory handling:
 
 import os
+import errno
 import shutil
 import tempfile
 import time
@@ -13,7 +14,7 @@
     """Return a temporary directory for the duration of this process.
     
     Multiple calls in the same process returns the same directory.
-    Remember to all delete_temp_dir() before exiting."""
+    Remember to call delete_temp_dir() before exiting."""
     global _tmp_dir
     if _tmp_dir is None:
         datestring = "%d-%d-%d-%02d-%02d" % time.localtime()[:5]
@@ -30,47 +31,53 @@
     _tmp_dir = None
 
 def get_instant_dir():
-    "Return a temporary directory for the duration of this process."
+    "Return the default instant directory, creating it if necessary."
     # os.path.expanduser works for Windows, Linux, and Mac
     # In Windows, $HOME is os.environ['HOMEDRIVE'] + os.environ['HOMEPATH']
     instant_dir = os.path.join(os.path.expanduser("~"), ".instant")
-    if not os.path.isdir(instant_dir):
-        instant_debug("Creating instant directory '%s'." % instant_dir)
-        os.mkdir(instant_dir)
+    makedirs(instant_dir)
     return instant_dir
 
 def get_default_cache_dir():
     "Return the default cache directory."
-    if "INSTANT_CACHE_DIR" in os.environ:
-        cache_dir = os.environ["INSTANT_CACHE_DIR"]
-    else:
+    cache_dir = os.environ.get("INSTANT_CACHE_DIR")
+    # Catches the cases where INSTANT_CACHE_DIR is not set or ''
+    if not cache_dir:
         cache_dir = os.path.join(get_instant_dir(), "cache")
-    if not os.path.isdir(cache_dir):
-        instant_debug("Creating cache directory '%s'." % cache_dir)
-        os.mkdir(cache_dir)
+    makedirs(cache_dir)
     return cache_dir
 
 def get_default_error_dir():
-    "Return the default cache directory."
-    if "INSTANT_ERROR_DIR" in os.environ:
-        cache_dir = os.environ["INSTANT_ERROR_DIR"]
-    else:
-        cache_dir = os.path.join(get_instant_dir(), "error")
-    if not os.path.isdir(cache_dir):
-        instant_debug("Creating cache directory '%s'." % cache_dir)
-        os.mkdir(cache_dir)
-    return cache_dir
+    "Return the default error directory."
+    error_dir = os.environ.get("INSTANT_ERROR_DIR")
+    # Catches the cases where INSTANT_ERROR_DIR is not set or ''
+    if not error_dir:
+        error_dir = os.path.join(get_instant_dir(), "error")
+    makedirs(error_dir)
+    return error_dir
 
 def validate_cache_dir(cache_dir):
     if cache_dir is None:
         return get_default_cache_dir()
     instant_assert(isinstance(cache_dir, str), "Expecting cache_dir to be a string.")
     cache_dir = os.path.abspath(cache_dir)
-    if not os.path.isdir(cache_dir):
-        os.mkdir(cache_dir)
+    makedirs(cache_dir)
     return cache_dir
 
+def makedirs(path):
+    """
+    Creates a directory (tree). If directory already excists it does nothing.
+    """
+    try:
+        os.makedirs(path)
+        instant_debug("In instant.makedirs: Creating directory %r" % path)
+    except os.error, e:
+        if e.errno != errno.EEXIST:
+            raise
+
 def _test():
+    from output import set_logging_level
+    set_logging_level("DEBUG")
     print "Temp dir:", get_temp_dir()
     print "Instant dir:", get_instant_dir()
     print "Default cache dir:", get_default_cache_dir()