zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix racy sbang (#33549)

Browse Source 
Spack currently creates a temporary sbang that is moved "atomically" in place,
but this temporary causes races when multiple processes start installing sbang.

Let's just stick to an idempotent approach. Notice that we only re-install sbang
if Spack updates it (since we do file compare), and sbang was only touched
18 times in the past 6 years, whereas we hit the sbang tempfile issue
frequently with parallel install on a fresh spack instance in CI.

Also fixes a bug where permissions weren't updated if config changed but
the latest version of the sbang file was already installed.
This commit is contained in:
Harmen Stoppels
2022-11-08 20:05:05 +01:00
committed by GitHub
parent 4a5e68816b
commit 4d28a64661
3 changed files with 63 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
lib/spack/spack/config.py
View File

@@ -45,7 +45,7 @@
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 @@ def _write_section(self, section):
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))

53
lib/spack/spack/hooks/sbang.py
View File

@@ -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
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.
sbang_bin_dir = os.path.dirname(sbang_path)
fs.mkdirp(sbang_bin_dir)
if not os.path.isdir(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"))
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)
# 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
# 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)
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_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)
# 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):