refactor for review

This commit is contained in:
Gregory Becker 2023-03-28 17:29:15 -07:00
parent 6bad473e4e
commit 6f959badcb
4 changed files with 77 additions and 6 deletions

View File

@ -407,6 +407,7 @@ def __init__(
exclude=[], exclude=[],
link=default_view_link, link=default_view_link,
link_type="symlink", link_type="symlink",
update_method="auto",
): ):
self.base = base_path self.base = base_path
self.raw_root = root self.raw_root = root
@ -416,6 +417,7 @@ def __init__(
self.exclude = exclude self.exclude = exclude
self.link_type = view_func_parser(link_type) self.link_type = view_func_parser(link_type)
self.link = link self.link = link
self.update_method = update_method
def select_fn(self, spec): def select_fn(self, spec):
return any(spec.satisfies(s) for s in self.select) return any(spec.satisfies(s) for s in self.select)
@ -432,6 +434,7 @@ def __eq__(self, other):
self.exclude == other.exclude, self.exclude == other.exclude,
self.link == other.link, self.link == other.link,
self.link_type == other.link_type, self.link_type == other.link_type,
self.update_method == other.update_method,
] ]
) )
@ -452,6 +455,8 @@ def to_dict(self):
ret["link_type"] = inverse_view_func_parser(self.link_type) ret["link_type"] = inverse_view_func_parser(self.link_type)
if self.link != default_view_link: if self.link != default_view_link:
ret["link"] = self.link ret["link"] = self.link
if self.update_method != "auto":
ret["update_method"] = self.update_method
return ret return ret
@staticmethod @staticmethod
@ -464,6 +469,7 @@ def from_dict(base_path, d):
d.get("exclude", []), d.get("exclude", []),
d.get("link", default_view_link), d.get("link", default_view_link),
d.get("link_type", "symlink"), d.get("link_type", "symlink"),
d.get("update_method", "auto"),
) )
@property @property
@ -593,14 +599,49 @@ def specs_for_view(self, concretized_root_specs):
return specs return specs
def raise_if_invalid_exchange(self):
if not spack.util.atomic_update.renameat2():
msg = "This operating system does not support the 'exchange' atomic update method."
msg += f"\n If the view at {self.root} does not already exist on the filesystem,"
msg += "change its update_method to 'symlink' or 'auto'."
msg += f"\n If the view at {self.root} exists already, either remove it for a"
msg += " non-atomic update or run on a newer OS."
raise RuntimeError(msg)
def raise_if_symlink_before_exchange(self):
if os.path.islink(self.root):
msg = f"The view at {self.root} cannot be updated with the 'exchange' update method"
msg += " because it was originally constructed with the 'symlink' method."
msg += " Either change the update method to 'symlink' or remove the view for a"
msg += " non-atomic update"
raise RuntimeError(msg)
def raise_if_exchange_before_symlink(self):
if os.path.isdir(self.root) and not os.path.islink(self.root):
msg = f"The view at {self.root} cannot be updated with the 'symlink' update method"
msg += " because it was originally constructed with the 'exchange' method."
msg += " Either change the update method to 'exchange' or remove the view for a"
msg += " non-atomic update"
raise RuntimeError(msg)
def use_renameat2(self): def use_renameat2(self):
# If it's set explicitly, respect that
if self.update_method == "exchange":
self.raise_if_symlink_before_exchange()
self.raise_if_invalid_exchange()
return True
if self.update_method == "symlink":
self.raise_if_exchange_before_symlink()
return False
# If it's set to "auto", detect which it should be
if os.path.islink(self.root): if os.path.islink(self.root):
return False return False
elif os.path.isdir(self.root): elif os.path.isdir(self.root):
if not spack.util.atomic_update.renameat2: self.raise_if_invalid_exchange()
raise Exception return True
return bool(spack.util.atomic_update.renameat2) return bool(spack.util.atomic_update.renameat2())
def regenerate(self, concretized_root_specs): def regenerate(self, concretized_root_specs):
specs = self.specs_for_view(concretized_root_specs) specs = self.specs_for_view(concretized_root_specs)

View File

@ -111,6 +111,10 @@
"type": "array", "type": "array",
"items": {"type": "string"}, "items": {"type": "string"},
}, },
"update_method": {
"type": "string",
"pattern": "(symlink|exchange|auto)",
},
"projections": projections_scheme, "projections": projections_scheme,
}, },
} }

View File

@ -52,7 +52,7 @@
sep = os.sep sep = os.sep
if spack.util.atomic_update.renameat2: if spack.util.atomic_update.renameat2():
use_renameat2 = [True, False] use_renameat2 = [True, False]
else: else:
use_renameat2 = [False] use_renameat2 = [False]
@ -61,7 +61,7 @@
@pytest.fixture(params=use_renameat2) @pytest.fixture(params=use_renameat2)
def atomic_update_implementations(request, monkeypatch): def atomic_update_implementations(request, monkeypatch):
if request.param is False: if request.param is False:
monkeypatch.setattr(spack.util.atomic_update, "renameat2", None) monkeypatch.setattr(spack.util.atomic_update, "_renameat2", None)
yield yield
@ -2902,7 +2902,7 @@ def test_environment_view_target_already_exists(
the new view dir already exists. If so, it should not be the new view dir already exists. If so, it should not be
removed or modified.""" removed or modified."""
# Only works for symlinked atomic views # Only works for symlinked atomic views
monkeypatch.setattr(spack.util.atomic_update, "renameat2", None) monkeypatch.setattr(spack.util.atomic_update, "_renameat2", None)
# Create a new environment # Create a new environment
view = str(tmpdir.join("view")) view = str(tmpdir.join("view"))
@ -3295,3 +3295,28 @@ def test_environment_created_in_users_location(mutable_config, tmpdir):
assert dir_name in out assert dir_name in out
assert env_dir in ev.root(dir_name) assert env_dir in ev.root(dir_name)
assert os.path.isdir(os.path.join(env_dir, dir_name)) assert os.path.isdir(os.path.join(env_dir, dir_name))
@pytest.mark.parametrize("update_method", ["symlink", "exchange"])
def test_view_update_mismatch(update_method, tmpdir, install_mockery, mock_fetch):
root = str(tmpdir.join("root"))
if update_method == "symlink":
os.makedirs(root)
checker = "cannot be updated with the 'symlink' update method"
elif True in use_renameat2:
link = str(tmpdir.join("symlink"))
os.makedirs(link)
os.symlink(link, root)
checker = "cannot be updated with the 'exchange' update method"
else:
checker = "does not support the 'exchange' atomic update method"
view = ev.environment.ViewDescriptor(
base_path=str(tmpdir), root=root, update_method=update_method
)
spec = spack.spec.Spec("libelf").concretized()
install("libelf")
with pytest.raises(RuntimeError, match=checker):
view.regenerate([spec])

View File

@ -36,6 +36,7 @@ def set_renameat2():
def renameat2(): def renameat2():
global _renameat2
if _renameat2 is notset: if _renameat2 is notset:
_renameat2 = set_renameat2() _renameat2 = set_renameat2()
return _renameat2 return _renameat2