diff options
| -rw-r--r-- | lib/spack/llnl/util/filesystem.py | 45 | ||||
| -rw-r--r-- | lib/spack/spack/config.py | 6 | ||||
| -rw-r--r-- | lib/spack/spack/hooks/sbang.py | 57 |
3 files changed, 65 insertions, 43 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index aece52f843..d122280e99 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -1000,16 +1000,45 @@ def hash_directory(directory, ignore=[]): return md5_hash.hexdigest() +def _try_unlink(path): + try: + os.unlink(path) + except (IOError, OSError): + # But if that fails, that's OK. + pass + + @contextmanager @system_path_filter -def write_tmp_and_move(filename): - """Write to a temporary file, then move into place.""" - dirname = os.path.dirname(filename) - basename = os.path.basename(filename) - tmp = os.path.join(dirname, ".%s.tmp" % basename) - with open(tmp, "w") as f: - yield f - shutil.move(tmp, filename) +def write_tmp_and_move(path, mode="w"): + """Write to a temporary file in the same directory, then move into place.""" + # Rely on NamedTemporaryFile to give a unique file without races + # in the directory of the target file. + file = tempfile.NamedTemporaryFile( + prefix="." + os.path.basename(path), + suffix=".tmp", + dir=os.path.dirname(path), + mode=mode, + delete=False, # we delete it ourselves + ) + tmp_path = file.name + + try: + yield file + except BaseException: + # On any failure, try to remove the temporary file. + _try_unlink(tmp_path) + raise + finally: + # Always close the file decriptor + file.close() + + # Atomically move into existence. + try: + os.rename(tmp_path, path) + except (IOError, OSError): + _try_unlink(tmp_path) + raise @contextmanager diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index e9a3b1d956..f35310986e 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -45,7 +45,7 @@ from six import iteritems import llnl.util.lang import llnl.util.tty as tty -from llnl.util.filesystem import mkdirp, rename +from llnl.util.filesystem import mkdirp, write_tmp_and_move import spack.compilers import spack.paths @@ -287,10 +287,8 @@ class SingleFileScope(ConfigScope): parent = os.path.dirname(self.path) mkdirp(parent) - tmp = os.path.join(parent, ".%s.tmp" % os.path.basename(self.path)) - with open(tmp, "w") as f: + with write_tmp_and_move(self.path) as f: syaml.dump_config(data_to_write, stream=f, default_flow_style=False) - rename(tmp, self.path) except (yaml.YAMLError, IOError) as e: raise ConfigFileError("Error writing to config file: '%s'" % str(e)) diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py index 5d2de2b865..e22c2929c2 100644 --- a/lib/spack/spack/hooks/sbang.py +++ b/lib/spack/spack/hooks/sbang.py @@ -186,44 +186,39 @@ def install_sbang(): ``sbang`` here ensures that users can access the script and that ``sbang`` itself is in a short path. """ - # copy in a new version of sbang if it differs from what's in spack sbang_path = sbang_install_path() - if os.path.exists(sbang_path) and filecmp.cmp(spack.paths.sbang_script, sbang_path): - return - # make $install_tree/bin - sbang_bin_dir = os.path.dirname(sbang_path) - fs.mkdirp(sbang_bin_dir) + all = spack.spec.Spec("all") + group_name = spack.package_prefs.get_package_group(all) + config_mode = spack.package_prefs.get_package_dir_permissions(all) + group_id = grp.getgrnam(group_name).gr_gid if group_name else None - # get permissions for bin dir from configuration files - group_name = spack.package_prefs.get_package_group(spack.spec.Spec("all")) - config_mode = spack.package_prefs.get_package_dir_permissions(spack.spec.Spec("all")) + # First setup the bin dir correctly. + sbang_bin_dir = os.path.dirname(sbang_path) + if not os.path.isdir(sbang_bin_dir): + fs.mkdirp(sbang_bin_dir) - if group_name: - os.chmod(sbang_bin_dir, config_mode) # Use package directory permissions + # Set group and ownership like we do on package directories + if group_id: + os.chown(sbang_bin_dir, os.stat(sbang_bin_dir).st_uid, group_id) + os.chmod(sbang_bin_dir, config_mode) else: fs.set_install_permissions(sbang_bin_dir) - # set group on sbang_bin_dir if not already set (only if set in configuration) - # TODO: after we drop python2 support, use shutil.chown to avoid gid lookups that - # can fail for remote groups - if group_name and os.stat(sbang_bin_dir).st_gid != grp.getgrnam(group_name).gr_gid: - os.chown(sbang_bin_dir, os.stat(sbang_bin_dir).st_uid, grp.getgrnam(group_name).gr_gid) - - # copy over the fresh copy of `sbang` - sbang_tmp_path = os.path.join( - os.path.dirname(sbang_path), - ".%s.tmp" % os.path.basename(sbang_path), - ) - shutil.copy(spack.paths.sbang_script, sbang_tmp_path) - - # set permissions on `sbang` (including group if set in configuration) - os.chmod(sbang_tmp_path, config_mode) - if group_name: - os.chown(sbang_tmp_path, os.stat(sbang_tmp_path).st_uid, grp.getgrnam(group_name).gr_gid) - - # Finally, move the new `sbang` into place atomically - os.rename(sbang_tmp_path, sbang_path) + # Then check if we need to install sbang itself. + try: + already_installed = filecmp.cmp(spack.paths.sbang_script, sbang_path) + except (IOError, OSError): + already_installed = False + + if not already_installed: + with fs.write_tmp_and_move(sbang_path) as f: + shutil.copy(spack.paths.sbang_script, f.name) + + # Set permissions on `sbang` (including group if set in configuration) + os.chmod(sbang_path, config_mode) + if group_id: + os.chown(sbang_path, os.stat(sbang_path).st_uid, group_id) def post_install(spec): |