Fix #139: Don't create the cache directory if --no-cache is passed

nml/generic.py

@@ -486,10 +486,9 @@ def find_file(filepath):
 
 def set_cache_root_dir(dir):
     global cache_root_dir
-    cache_root_dir = os.path.abspath(dir)
-    os.makedirs(cache_root_dir, exist_ok=True)
+    cache_root_dir = None if dir is None else os.path.abspath(dir)
 
-def get_cache_file(sources, extension):
+def _cache_file_path(sources, extension):
     """
     Compose a filename for a cache file.
 
@@ -512,10 +511,27 @@ def get_cache_file(sources, extension):
                 # Make sure that the path does not leave the cache dir
                 path = os.path.normpath(path).replace(os.path.pardir, "__")
                 path = os.path.join(cache_root_dir, path)
-                os.makedirs(path, exist_ok=True)
                 result = os.path.join(path, name)
             else:
                 # In case of multiple soure files, ignore the path component for all but the first
                 result += "_" + name
 
     return result + extension
+
+def open_cache_file(sources, extension, mode):
+    if cache_root_dir is None:
+        raise FileNotFoundError("No cache directory")
+
+    if not any(sources):
+        raise FileNotFoundError("Can't create cache file with no sources")
+
+    path = _cache_file_path(sources, extension)
+
+    try:
+        if 'w' in mode:
+            os.makedirs(os.path.dirname(path), exist_ok=True)
+        return open(path, mode)
+    except OSError:
+        if 'w' in mode:
+            print_warning("Can't create cache file {}. Check permissions, or use --cache-dir or --no-cache.".format(path))
+        raise

nml/main.py

@@ -77,7 +77,7 @@ def parse_cli(argv):
     opts, args = opt_parser.parse_args(argv)
 
     generic.set_verbosity(0 if opts.quiet else opts.verbosity)
-    generic.set_cache_root_dir(opts.cache_dir)
+    generic.set_cache_root_dir(None if opts.no_cache else opts.cache_dir)
     spritecache.keep_orphaned = opts.keep_orphaned
 
     opts.outputfile_given = (opts.grf_filename or opts.nfo_filename or opts.nml_filename or opts.dep_filename or opts.outputs)
@@ -167,15 +167,15 @@ def main(argv):
             generic.print_error("Unknown output format {}".format(outext))
             sys.exit(2)
 
-    ret = nml(input, input_filename, opts.debug, outputs, opts.start_sprite_num, opts.compress, opts.crop, not opts.no_cache, opts.forced_palette, opts.md5_filename, opts.rebuild_parser, opts.debug_parser)
+    ret = nml(input, input_filename, opts.debug, outputs, opts.start_sprite_num, opts.compress, opts.crop, opts.forced_palette, opts.md5_filename, opts.rebuild_parser, opts.debug_parser)
 
     input.close()
     sys.exit(ret)
 
 def filename_output_from_input(name, ext):
     return os.path.splitext(name)[0] + ext
 
-def nml(inputfile, input_filename, output_debug, outputfiles, start_sprite_num, compress_grf, crop_sprites, enable_cache, forced_palette, md5_filename, rebuild_parser, debug_parser):
+def nml(inputfile, input_filename, output_debug, outputfiles, start_sprite_num, compress_grf, crop_sprites, forced_palette, md5_filename, rebuild_parser, debug_parser):
     """
     Compile an NML file.
 
@@ -197,9 +197,6 @@ def nml(inputfile, input_filename, output_debug, outputfiles, start_sprite_num,
     @param crop_sprites: Enable sprite cropping.
     @type  crop_sprites: C{bool}
 
-    @param enable_cache: Enable sprite cache.
-    @type  enable_cache: C{bool}
-
     @param forced_palette: Palette to use for the file.
     @type  forced_palette: C{str}
 
@@ -358,7 +355,7 @@ def nml(inputfile, input_filename, output_debug, outputfiles, start_sprite_num,
         outputfile.palette = used_palette # used by RecolourSpriteAction
         if isinstance(outputfile, output_grf.OutputGRF):
             if encoder is None:
-                encoder = spriteencoder.SpriteEncoder(compress_grf, crop_sprites, enable_cache, used_palette)
+                encoder = spriteencoder.SpriteEncoder(compress_grf, crop_sprites, used_palette)
             outputfile.encoder = encoder
 
     generic.clear_progress()

nml/spritecache.py

@@ -22,11 +22,8 @@ class SpriteCache:
     """
     Cache for compressed sprites.
 
-    @ivar cache_filename: Filename of cache data file.
-    @type cache_filename: C{str}
-
-    @ivar cache_index_filename: Filename of cache index file.
-    @type cache_index_filename: C{str}
+    @ivar sources: Tuple of paths to files the cache belongs to or depends on.
+    @type sources: C{tuple} of (C{str} or C{None})
 
     @ivar cache_time: Date of cache files. The cache is invalid, if the source image files are newer.
     @type cache_time: C{int}
@@ -77,9 +74,8 @@ class SpriteCache:
         meta-information or padding. Offsets and sizes for the various sprites
         are in the cacheindex file.
     """
-    def __init__(self, filename):
-        self.cache_filename = filename + ".cache"
-        self.cache_index_filename = filename + ".cacheindex"
+    def __init__(self, sources=()):
+        self.sources = sources
         self.cache_time = 0
         self.cached_sprites = {}
 
@@ -137,17 +133,21 @@ def read_cache(self):
         """
         Read the *.grf.cache[index] files.
         """
-        if not (os.access(self.cache_filename, os.R_OK) and os.access(self.cache_index_filename, os.R_OK)):
-            # Cache files don't exist
+
+        try:
+            with generic.open_cache_file(self.sources, ".cache", 'rb') as cache_file:
+                index_file = generic.open_cache_file(self.sources, ".cacheindex", 'r')
+
+                cache_data = array.array('B')
+                cache_size = os.fstat(cache_file.fileno()).st_size
+                cache_data.fromfile(cache_file, cache_size)
+                assert cache_size == len(cache_data)
+                self.cache_time = os.path.getmtime(cache_file.name)
+        except OSError:
+            # Cache files don't exist (or otherwise aren't readable)
             return
 
-        index_file = open(self.cache_index_filename, 'r')
-        cache_file = open(self.cache_filename, 'rb')
-        cache_data = array.array('B')
-        cache_size = os.fstat(cache_file.fileno()).st_size
-        cache_data.fromfile(cache_file, cache_size)
-        assert cache_size == len(cache_data)
-        self.cache_time = os.path.getmtime(self.cache_filename)
+        # cache_file is closed by `with` block, but index_file is still open.
 
         source_mtime = dict()
 
@@ -238,16 +238,19 @@ def read_cache(self):
                 if is_valid:
                     self.cached_sprites[key] = value
         except:
-            generic.print_warning(self.cache_index_filename + " contains invalid data, ignoring. Please remove the file and file a bug report if this warning keeps appearing")
+            generic.print_warning(index_file.name + " contains invalid data, ignoring. Please remove the file and file a bug report if this warning keeps appearing")
             self.cached_sprites = {} # Clear cache
 
         index_file.close()
-        cache_file.close()
 
     def write_cache(self):
         """
         Write the cache data to the .cache[index] files.
         """
+        if generic.cache_root_dir is None:
+            # Writing cache files will fail, so bail early.
+            return
+
         index_data = []
         sprite_data = array.array('B')
         offset = 0
@@ -292,9 +295,10 @@ def write_cache(self):
 
         index_output = json.JSONEncoder(sort_keys = True).encode(index_data)
 
-        index_file = open(self.cache_index_filename, 'w')
-        index_file.write(index_output)
-        index_file.close()
-        cache_file = open(self.cache_filename, 'wb')
-        sprite_data.tofile(cache_file)
-        cache_file.close()
+        try:
+            with generic.open_cache_file(self.sources, ".cache", 'wb') as cache_file, \
+                    generic.open_cache_file(self.sources, ".cacheindex", 'w') as index_file:
+                index_file.write(index_output)
+                sprite_data.tofile(cache_file)
+        except OSError:
+            return

nml/spriteencoder.py

@@ -65,21 +65,17 @@ class SpriteEncoder:
     @ivar crop_sprites: Crop sprites if possible.
     @type crop_sprites: C{bool}
 
-    @ivar enable_cache: Read/write cache from/to disk.
-    @type enable_cache: C{bool}
-
     @ivar palette: Palette for encoding, see L{palette.palette_name}.
     @type palette: C{str}
 
     @ivar cached_image_files: Currently opened source image files.
     @type cached_image_files: C{dict} mapping C{str} to C{Image}
     """
-    def __init__(self, compress_grf, crop_sprites, enable_cache, palette):
+    def __init__(self, compress_grf, crop_sprites, palette):
         self.compress_grf = compress_grf
         self.crop_sprites = crop_sprites
-        self.enable_cache = enable_cache
         self.palette = palette
-        self.sprite_cache = spritecache.SpriteCache("")
+        self.sprite_cache = spritecache.SpriteCache()
         self.cached_image_files = {}
 
     def open(self, sprite_files):
@@ -105,9 +101,8 @@ def open(self, sprite_files):
 
             source_name = "_".join(src for src in sources if src is not None)
 
-            local_cache = spritecache.SpriteCache(generic.get_cache_file(sources, ""))
-            if self.enable_cache:
-                local_cache.read_cache()
+            local_cache = spritecache.SpriteCache(sources)
+            local_cache.read_cache()
 
             for sprite_info in sprite_list:
                 count_sprites += 1
@@ -140,7 +135,7 @@ def open(self, sprite_files):
             num_orphaned += local_cache.count_orphaned()
 
             # Only write cache if compression is enabled. Uncompressed data is not worth to be cached.
-            if self.enable_cache and self.compress_grf:
+            if self.compress_grf:
                 local_cache.write_cache()
 
             # Transfer data to global cache for later usage