diff options
author | Peter Scheibel <scheibel1@llnl.gov> | 2023-07-05 00:54:04 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-05 09:54:04 +0200 |
commit | f365386447c39ae21cccfe7b844f844c9d59e99c (patch) | |
tree | 70d485177594c50a12d707f498ee514938c13bf4 | |
parent | a90200528fc8fefbd1d04b62f64db1e925596b8f (diff) | |
download | spack-f365386447c39ae21cccfe7b844f844c9d59e99c.tar.gz spack-f365386447c39ae21cccfe7b844f844c9d59e99c.tar.bz2 spack-f365386447c39ae21cccfe7b844f844c9d59e99c.tar.xz spack-f365386447c39ae21cccfe7b844f844c9d59e99c.zip |
Installations: don't set group permissions when they match what is desired (#38036)
* When installing a package Spack will attempt to set group permissions on
the install prefix even when the configuration does not specify a group.
Co-authored-by: David Gomez <dvdgomez@users.noreply.github.com>
-rw-r--r-- | lib/spack/llnl/util/filesystem.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/test/llnl/util/filesystem.py | 34 |
2 files changed, 36 insertions, 0 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 2b4cbe2424..1a9f6835aa 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -610,6 +610,8 @@ def chgrp(path, group, follow_symlinks=True): gid = grp.getgrnam(group).gr_gid else: gid = group + if os.stat(path).st_gid == gid: + return if follow_symlinks: os.chown(path, -1, gid) else: diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index 018137f406..9e22f96c65 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -502,6 +502,40 @@ def test_filter_files_with_different_encodings(regex, replacement, filename, tmp assert replacement in f.read() +@pytest.mark.skipif(sys.platform == "win32", reason="chgrp isn't used on Windows") +def test_chgrp_dont_set_group_if_already_set(tmpdir, monkeypatch): + with fs.working_dir(tmpdir): + os.mkdir("test-dir_chgrp_dont_set_group_if_already_set") + + def _fail(*args, **kwargs): + raise Exception("chrgrp should not be called") + + class FakeStat(object): + def __init__(self, gid): + self.st_gid = gid + + original_stat = os.stat + + def _stat(*args, **kwargs): + path = args[0] + if path == "test-dir_chgrp_dont_set_group_if_already_set": + return FakeStat(gid=1001) + else: + # Monkeypatching stat can interfere with post-test cleanup, so for + # paths that aren't part of the test, we want the original behavior + # of stat + return original_stat(*args, **kwargs) + + monkeypatch.setattr(os, "chown", _fail) + monkeypatch.setattr(os, "lchown", _fail) + monkeypatch.setattr(os, "stat", _stat) + + with fs.working_dir(tmpdir): + with pytest.raises(Exception): + fs.chgrp("test-dir_chgrp_dont_set_group_if_already_set", 1002) + fs.chgrp("test-dir_chgrp_dont_set_group_if_already_set", 1001) + + def test_filter_files_multiple(tmpdir): # All files given as input to this test must satisfy the pre-requisite # that the 'replacement' string is not present in the file initially and |