summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHarmen Stoppels <harmenstoppels@gmail.com>2022-11-09 10:31:27 +0100
committerGitHub <noreply@github.com>2022-11-09 09:31:27 +0000
commitd1d668a9d57e984a225eeef153d93cb315e03dfb (patch)
tree7269eced52c38ac05c6a4ccfb9b4ed3baa552165
parent284c3a3fd8a4213a2cc6bf4aef85d672369f4b5b (diff)
downloadspack-d1d668a9d57e984a225eeef153d93cb315e03dfb.tar.gz
spack-d1d668a9d57e984a225eeef153d93cb315e03dfb.tar.bz2
spack-d1d668a9d57e984a225eeef153d93cb315e03dfb.tar.xz
spack-d1d668a9d57e984a225eeef153d93cb315e03dfb.zip
Revert "fix racy sbang (#33549)" (#33778)
This reverts commit 4d28a6466188ea9fe3b55a4c3d7690dd66e0dc8f.
-rw-r--r--lib/spack/llnl/util/filesystem.py45
-rw-r--r--lib/spack/spack/config.py6
-rw-r--r--lib/spack/spack/hooks/sbang.py57
3 files changed, 43 insertions, 65 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index d122280e99..aece52f843 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -1000,45 +1000,16 @@ 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(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
+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)
@contextmanager
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index f35310986e..e9a3b1d956 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, write_tmp_and_move
+from llnl.util.filesystem import mkdirp, rename
import spack.compilers
import spack.paths
@@ -287,8 +287,10 @@ class SingleFileScope(ConfigScope):
parent = os.path.dirname(self.path)
mkdirp(parent)
- with write_tmp_and_move(self.path) as f:
+ tmp = os.path.join(parent, ".%s.tmp" % os.path.basename(self.path))
+ with open(tmp, "w") 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 e22c2929c2..5d2de2b865 100644
--- a/lib/spack/spack/hooks/sbang.py
+++ b/lib/spack/spack/hooks/sbang.py
@@ -186,39 +186,44 @@ 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
- 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
-
- # First setup the bin dir correctly.
+ # make $install_tree/bin
sbang_bin_dir = os.path.dirname(sbang_path)
- if not os.path.isdir(sbang_bin_dir):
- fs.mkdirp(sbang_bin_dir)
+ fs.mkdirp(sbang_bin_dir)
+
+ # 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"))
- # 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)
+ if group_name:
+ os.chmod(sbang_bin_dir, config_mode) # Use package directory permissions
else:
fs.set_install_permissions(sbang_bin_dir)
- # 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)
+ # 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)
def post_install(spec):