From f365386447c39ae21cccfe7b844f844c9d59e99c Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Wed, 5 Jul 2023 00:54:04 -0700 Subject: 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 --- lib/spack/llnl/util/filesystem.py | 2 ++ lib/spack/spack/test/llnl/util/filesystem.py | 34 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) 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 -- cgit v1.2.3-60-g2f50