From d11960ecfbb39c6b324d37fe0e51b0950a92dd6d Mon Sep 17 00:00:00 2001 From: Andrew Chin Date: Mon, 10 Feb 2014 21:37:31 -0500 Subject: [PATCH] Try to track the capabilities of our outputdir filesystem. For example, don't chmod if the filesystem dosen't support chmod, and don't rename over files if that's not supported (this functionality was already in place). Should fix #1061 Related to #1055 (we could add a mtime capability flag) --- overviewer_core/assetmanager.py | 14 +++-- overviewer_core/files.py | 99 +++++++++++++++++++++------------ overviewer_core/tileset.py | 9 ++- 3 files changed, 78 insertions(+), 44 deletions(-) diff --git a/overviewer_core/assetmanager.py b/overviewer_core/assetmanager.py index 96d8bb2..dd05b0e 100644 --- a/overviewer_core/assetmanager.py +++ b/overviewer_core/assetmanager.py @@ -25,7 +25,7 @@ from PIL import Image import world import util -from files import FileReplacer, mirror_dir +from files import FileReplacer, mirror_dir, get_fs_caps class AssetManager(object): """\ @@ -44,6 +44,8 @@ directory. self.custom_assets_dir = custom_assets_dir self.renders = dict() + self.fs_caps = get_fs_caps(self.outputdir) + # look for overviewerConfig in self.outputdir try: with open(os.path.join(self.outputdir, "overviewerConfig.js")) as c: @@ -148,7 +150,7 @@ directory. # write out config jsondump = json.dumps(dump, indent=4) - with FileReplacer(os.path.join(self.outputdir, "overviewerConfig.js")) as tmpfile: + with FileReplacer(os.path.join(self.outputdir, "overviewerConfig.js"), capabilities=self.fs_caps) as tmpfile: with codecs.open(tmpfile, 'w', encoding='UTF-8') as f: f.write("var overviewerConfig = " + jsondump + ";\n") @@ -162,12 +164,12 @@ directory. global_assets = os.path.join(util.get_program_path(), "overviewer_core", "data", "web_assets") if not os.path.isdir(global_assets): global_assets = os.path.join(util.get_program_path(), "web_assets") - mirror_dir(global_assets, self.outputdir) + mirror_dir(global_assets, self.outputdir, capabilities=self.fs_caps) if self.custom_assets_dir: # Could have done something fancy here rather than just overwriting # the global files, but apparently this what we used to do pre-rewrite. - mirror_dir(self.custom_assets_dir, self.outputdir) + mirror_dir(self.custom_assets_dir, self.outputdir, capabilities=self.fs_caps) # write a dummy baseMarkers.js if none exists if not os.path.exists(os.path.join(self.outputdir, "baseMarkers.js")): @@ -179,7 +181,7 @@ directory. js_src = os.path.join(util.get_program_path(), "overviewer_core", "data", "js_src") if not os.path.isdir(js_src): js_src = os.path.join(util.get_program_path(), "js_src") - with FileReplacer(os.path.join(self.outputdir, "overviewer.js")) as tmpfile: + with FileReplacer(os.path.join(self.outputdir, "overviewer.js"), capabilities=self.fs_caps) as tmpfile: with open(tmpfile, "w") as fout: # first copy in js_src/overviewer.js with open(os.path.join(js_src, "overviewer.js"), 'r') as f: @@ -199,6 +201,6 @@ directory. versionstr = "%s (%s)" % (util.findGitVersion(), util.findGitHash()[:7]) index = index.replace("{version}", versionstr) - with FileReplacer(indexpath) as indexpath: + with FileReplacer(indexpath, capabilities=self.fs_caps) as indexpath: with codecs.open(indexpath, 'w', encoding='UTF-8') as output: output.write(index) diff --git a/overviewer_core/files.py b/overviewer_core/files.py index dc680bb..ca6d8a6 100644 --- a/overviewer_core/files.py +++ b/overviewer_core/files.py @@ -20,9 +20,50 @@ import shutil import logging import stat +default_caps = {"chmod_works": True, "rename_works": True} + +def get_fs_caps(dir_to_test): + return {"chmod_works": does_chmod_work(dir_to_test), + "rename_works": does_rename_work(dir_to_test) + } + +def does_chmod_work(dir_to_test): + "Detects if chmod works in a given directory" + # a CIFS mounted FS is the only thing known to reliably not provide chmod + + if not os.path.isdir(dir_to_test): + return True + + f1 = tempfile.NamedTemporaryFile(dir=dir_to_test) + try: + f1_stat = os.stat(f1.name) + os.chmod(f1.name, f1_stat.st_mode | stat.S_IRUSR) + chmod_works = True + logging.debug("Detected that chmods work in %r" % dir_to_test) + except OSError: + chmod_works = False + logging.debug("Detected that chmods do NOT work in %r" % dir_to_test) + return chmod_works + +def does_rename_work(dir_to_test): + with tempfile.NamedTemporaryFile(dir=dir_to_test) as f1: + with tempfile.NamedTemporaryFile(dir=dir_to_test) as f2: + try: + os.rename(f1.name,f2.name) + except OSError: + renameworks = False + logging.debug("Detected that overwriting renames do NOT work in %r" % dir_to_test) + else: + renameworks = True + logging.debug("Detected that overwriting renames work in %r" % dir_to_test) + # re-make this file so it can be deleted without error + open(f1.name, 'w').close() + return renameworks + ## useful recursive copy, that ignores common OS cruft -def mirror_dir(src, dst, entities=None): +def mirror_dir(src, dst, entities=None, capabilities=default_caps): '''copies all of the entities from src to dst''' + chmod_works = capabilities.get("chmod_works") if not os.path.exists(dst): os.mkdir(dst) if entities and type(entities) != list: raise Exception("Expected a list, got a %r instead" % type(entities)) @@ -38,10 +79,13 @@ def mirror_dir(src, dst, entities=None): continue if os.path.isdir(os.path.join(src,entry)): - mirror_dir(os.path.join(src, entry), os.path.join(dst, entry)) + mirror_dir(os.path.join(src, entry), os.path.join(dst, entry), capabilities=capabilities) elif os.path.isfile(os.path.join(src,entry)): try: - shutil.copy(os.path.join(src, entry), os.path.join(dst, entry)) + if chmod_works: + shutil.copy(os.path.join(src, entry), os.path.join(dst, entry)) + else: + shutil.copyfile(os.path.join(src, entry), os.path.join(dst, entry)) except IOError as outer: try: # maybe permission problems? @@ -51,24 +95,16 @@ def mirror_dir(src, dst, entities=None): os.chmod(os.path.join(dst, entry), dst_stat.st_mode | stat.S_IWUSR) except OSError: # we don't care if this fails pass - shutil.copy(os.path.join(src, entry), os.path.join(dst, entry)) - # if this stills throws an error, let it propagate up + # try again; if this stills throws an error, let it propagate up + if chmod_works: + shutil.copy(os.path.join(src, entry), os.path.join(dst, entry)) + else: + shutil.copyfile(os.path.join(src, entry), os.path.join(dst, entry)) # Define a context manager to handle atomic renaming or "just forget it write # straight to the file" depending on whether os.rename provides atomic # overwrites. # Detect whether os.rename will overwrite files -with tempfile.NamedTemporaryFile() as f1: - with tempfile.NamedTemporaryFile() as f2: - try: - os.rename(f1.name,f2.name) - except OSError: - renameworks = False - else: - renameworks = True - # re-make this file so it can be deleted without error - open(f1.name, 'w').close() -del tempfile,f1,f2 doc = """This class acts as a context manager for files that are to be written out overwriting an existing file. @@ -89,16 +125,20 @@ with FileReplacer("config") as configname: with open(configout, 'w') as configout: configout.write(newconfig) """ -if renameworks: - class FileReplacer(object): - __doc__ = doc - def __init__(self, destname): - self.destname = destname +class FileReplacer(object): + __doc__ = doc + def __init__(self, destname, capabilities=default_caps): + self.caps = capabilities + self.destname = destname + if self.caps.get("rename_works"): self.tmpname = destname + ".tmp" - def __enter__(self): + def __enter__(self): + if self.caps.get("rename_works"): # rename works here. Return a temporary filename return self.tmpname - def __exit__(self, exc_type, exc_val, exc_tb): + return self.destname + def __exit__(self, exc_type, exc_val, exc_tb): + if self.caps.get("rename_works"): if exc_type: # error try: @@ -109,18 +149,7 @@ if renameworks: "'%s'!", self.tmpname) else: # copy permission bits, if needed - if os.path.exists(self.destname): + if self.caps.get("chmod_works") and os.path.exists(self.destname): shutil.copymode(self.destname, self.tmpname) # atomic rename into place os.rename(self.tmpname, self.destname) -else: - class FileReplacer(object): - __doc__ = doc - def __init__(self, destname): - self.destname = destname - def __enter__(self): - return self.destname - def __exit__(self, exc_type, exc_val, exc_tb): - return -del renameworks - diff --git a/overviewer_core/tileset.py b/overviewer_core/tileset.py index 0f0e1c3..3e13129 100644 --- a/overviewer_core/tileset.py +++ b/overviewer_core/tileset.py @@ -31,7 +31,7 @@ from PIL import Image from .util import roundrobin from . import nbt -from .files import FileReplacer +from .files import FileReplacer, get_fs_caps from .optimizeimages import optimize_image import rendermodes import c_overviewer @@ -357,6 +357,9 @@ class TileSet(object): self.options['renderchecks'] = 2 os.mkdir(self.outputdir) + # must wait until outputdir exists + self.fs_caps = get_fs_caps(self.outputdir) + if self.options['renderchecks'] == 2: # Set forcerendertime so that upon an interruption the next render # will continue where we left off. @@ -902,7 +905,7 @@ class TileSet(object): logging.error("While attempting to delete corrupt image %s, an error was encountered. You will need to delete it yourself. Error was '%s'", path[1], e) # Save it - with FileReplacer(imgpath) as tmppath: + with FileReplacer(imgpath, capabilities=self.fs_caps) as tmppath: if imgformat == 'jpg': img.save(tmppath, "jpeg", quality=self.options['imgquality'], subsampling=0) else: # png @@ -1006,7 +1009,7 @@ class TileSet(object): #draw.text((96,96), "c,r: %s,%s" % (col, row), fill='red') # Save them - with FileReplacer(imgpath) as tmppath: + with FileReplacer(imgpath, capabilities=self.fs_caps) as tmppath: if self.imgextension == 'jpg': tileimg.save(tmppath, "jpeg", quality=self.options['imgquality'], subsampling=0) else: # png