From fba76cf68dc9523192d833f633c8f6b9b1b77590 Mon Sep 17 00:00:00 2001 From: Mike Hendricks Date: Fri, 30 Aug 2024 17:45:17 -0700 Subject: [PATCH] Add omittable_distros support for configs --- README.md | 26 +++++++++ hab/parsers/config.py | 11 ++++ hab/parsers/flat_config.py | 4 +- hab/parsers/hab_base.py | 3 + hab/resolver.py | 9 ++- hab/solvers.py | 10 +++- tests/configs/omittable/omittable.json | 21 +++++++ tests/configs/omittable/omittable_child.json | 11 ++++ .../configs/omittable/omittable_inherit.json | 6 ++ tests/resolver_freeze_configs.json | 56 +++++++++++++++++++ tests/site_main_check.habcache | 42 ++++++++++++++ tests/test_parsing.py | 1 + tests/test_resolver.py | 3 + tests/test_solver.py | 41 ++++++++++++++ 14 files changed, 240 insertions(+), 4 deletions(-) create mode 100644 tests/configs/omittable/omittable.json create mode 100644 tests/configs/omittable/omittable_child.json create mode 100644 tests/configs/omittable/omittable_inherit.json diff --git a/README.md b/README.md index 1a2e985..8d6af7a 100644 --- a/README.md +++ b/README.md @@ -1285,6 +1285,32 @@ the `-r`/`--requirement` option. Users can see the optional distros in the dump output with verbosity level of 1 or higher. `hab dump - -v` +### Omittable Distros + +The `omittable_distros` key in [config](#config) definitions are used to specify distros +that are not required to use this hab configuration. This can be used to make it +so not all hosts need to have a dcc installed. For example a producer likely will +never need to open houdini but does need access to external tools. You would need +to install Houdini(or create a empty .hab.json distro) so hab doesn't raise an +`InvalidRequirementError` when it can't find Houdini. + +```json5 + "distros": [ + "houdini20.0==20.0.688", + "SideFXLabs20.0==20.0.506", + "python_tools" + ], + "omittable_distros": [ + "houdini20.0", + "SideFXLabs20.0" + ] +``` +This will make it so `houdini20.0` and `SideFXLabs20.0` will be loaded if found, +but if not they will be ignored. `python_tools` will always need to be installed. + +Note: `omittable_distros` is a list of distro names. It does not accept specifier +arguments like `==20.0.688`. + ### Platform specific code Hab works on windows, linux and osx(needs tested). To make it easier to handle diff --git a/hab/parsers/config.py b/hab/parsers/config.py index 6aad2ec..d376b49 100644 --- a/hab/parsers/config.py +++ b/hab/parsers/config.py @@ -129,8 +129,19 @@ def load(self, filename): data = super().load(filename) self._alias_mods = data.get("alias_mods", NotSet) self.inherits = data.get("inherits", NotSet) + if self.omittable_distros is NotSet: + self.omittable_distros = data.get("omittable_distros", NotSet) return data + @hab_property(verbosity=3, process_order=50) + def omittable_distros(self): + """A collection of distro names that are ignored if required by distros.""" + return self.frozen_data.get("omittable_distros", NotSet) + + @omittable_distros.setter + def omittable_distros(self, value): + self.frozen_data["omittable_distros"] = value + @hab_property(verbosity=1, group=0) def uri(self): # Mark uri as a HabProperty so it is included in _properties diff --git a/hab/parsers/flat_config.py b/hab/parsers/flat_config.py index 62495e5..2648c01 100644 --- a/hab/parsers/flat_config.py +++ b/hab/parsers/flat_config.py @@ -223,7 +223,9 @@ def versions(self): self._alias_mods = {} self.frozen_data["versions"] = versions - reqs = self.resolver.resolve_requirements(distros) + reqs = self.resolver.resolve_requirements( + distros, omittable=self.omittable_distros + ) for req in reqs.values(): version = self.resolver.find_distro(req) versions.append(version) diff --git a/hab/parsers/hab_base.py b/hab/parsers/hab_base.py index 40a1169..3a0d2fa 100644 --- a/hab/parsers/hab_base.py +++ b/hab/parsers/hab_base.py @@ -104,6 +104,9 @@ def _collect_values(self, node, props=None, default=False): for name, mod in mods.items(): self._alias_mods.setdefault(name, []).append(mod) continue + if not hasattr(node, attrname) and isinstance(node, self._placeholder): + # Skip properties that don't exist on the placeholder class + continue value = getattr(node, attrname) if value is NotSet: self._missing_values = True diff --git a/hab/resolver.py b/hab/resolver.py index 86f66ba..6ddfac5 100644 --- a/hab/resolver.py +++ b/hab/resolver.py @@ -334,18 +334,23 @@ def resolve(self, uri): context = self.closest_config(uri) return context.reduced(self, uri=uri) - def resolve_requirements(self, requirements): + def resolve_requirements(self, requirements, omittable=None): """Recursively solve the provided requirements into a final list of requirements. Args: requirements (list): The requirements to resolve. + omittable (list, optional): A list of distro names that are not required. + If a suitable distro can not be found, normally an `InvalidRequirementError` + is raised. If that distro name is in this list a warning is logged instead. Raises: MaxRedirectError: Redirect limit reached, unable to resolve the requested requirements. """ - solver = Solver(requirements, self, forced=self.forced_requirements) + solver = Solver( + requirements, self, forced=self.forced_requirements, omittable=omittable + ) return solver.resolve() def uri_validate(self, uri): diff --git a/hab/solvers.py b/hab/solvers.py index 7f0a995..ea19ad3 100644 --- a/hab/solvers.py +++ b/hab/solvers.py @@ -18,14 +18,18 @@ class Solver(object): forced (dict, optional): Forces this distro requirement replacing any resolved requirements. Using this may lead to configuring your environment incorrectly, use with caution. + omittable (list, optional): A list of distro names that are not required. + If a suitable distro can not be found, normally an `InvalidRequirementError` + is raised. If that distro name is in this list a warning is logged instead. Attributes: invalid (dict, optional): If a recursive requirement makes a already resolved version invalid, that version is added to this list as an exclusive exclude. """ - def __init__(self, requirements, resolver, forced=None): + def __init__(self, requirements, resolver, forced=None, omittable=None): self.forced = forced if forced else {} + self.omittable = omittable if omittable else [] self.invalid = {} self.max_redirects = 2 self.requirements = requirements @@ -126,6 +130,10 @@ def _resolve( logger.warning(f"Forced Requirement: {req}") reported.add(name) + if name in self.omittable and name not in self.resolver.distros: + logger.warning(f"Skipping missing omitted requirement: {req}") + continue + # Update the requirement to match all current requirements req = self.append_requirement(resolved, req) if name in self.invalid: diff --git a/tests/configs/omittable/omittable.json b/tests/configs/omittable/omittable.json new file mode 100644 index 0000000..0d151c5 --- /dev/null +++ b/tests/configs/omittable/omittable.json @@ -0,0 +1,21 @@ +{ + "name": "omittable", + "context": [], + "description": "Test that omittable_distros ignore missing configs.", + "inherits": false, + "distros": { + "maya2020": [ + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c" + ], + "missing_dcc": [ + "the_dcc_plugin_d", + "non-existent-distro" + ] + }, + "omittable_distros": [ + "missing_dcc", + "non-existent-distro" + ] +} diff --git a/tests/configs/omittable/omittable_child.json b/tests/configs/omittable/omittable_child.json new file mode 100644 index 0000000..6c106ad --- /dev/null +++ b/tests/configs/omittable/omittable_child.json @@ -0,0 +1,11 @@ +{ + "name": "child", + "context": ["omittable"], + "description": "This config should override omittable_distros on its parent.", + "inherits": true, + "omittable_distros": [ + "missing_dcc", + "maya2020", + "non-existent-distro" + ] +} diff --git a/tests/configs/omittable/omittable_inherit.json b/tests/configs/omittable/omittable_inherit.json new file mode 100644 index 0000000..a0714c1 --- /dev/null +++ b/tests/configs/omittable/omittable_inherit.json @@ -0,0 +1,6 @@ +{ + "name": "inherit", + "context": ["omittable"], + "description": "This config should inherit omittable_distros from its parent.", + "inherits": true +} diff --git a/tests/resolver_freeze_configs.json b/tests/resolver_freeze_configs.json index 870f526..fa71bfd 100644 --- a/tests/resolver_freeze_configs.json +++ b/tests/resolver_freeze_configs.json @@ -242,6 +242,62 @@ "name": "os", "uri": "not_set/os" }, + "omittable": { + "context": [], + "name": "omittable", + "omittable_distros": [ + "missing_dcc", + "non-existent-distro" + ], + "versions": [ + "the_dcc_plugin_a==1.1", + "the_dcc_plugin_e==1.1", + "the_dcc_plugin_d==1.1", + "the_dcc_plugin_b==1.1", + "the_dcc_plugin_c==1.1", + "maya2020==2020.1" + ], + "uri": "omittable" + }, + "omittable/child": { + "context": [ + "omittable" + ], + "name": "child", + "omittable_distros": [ + "missing_dcc", + "maya2020", + "non-existent-distro" + ], + "versions": [ + "the_dcc_plugin_a==1.1", + "the_dcc_plugin_e==1.1", + "the_dcc_plugin_d==1.1", + "the_dcc_plugin_b==1.1", + "the_dcc_plugin_c==1.1", + "maya2020==2020.1" + ], + "uri": "omittable/child" + }, + "omittable/inherit": { + "context": [ + "omittable" + ], + "name": "inherit", + "omittable_distros": [ + "missing_dcc", + "non-existent-distro" + ], + "versions": [ + "the_dcc_plugin_a==1.1", + "the_dcc_plugin_e==1.1", + "the_dcc_plugin_d==1.1", + "the_dcc_plugin_b==1.1", + "the_dcc_plugin_c==1.1", + "maya2020==2020.1" + ], + "uri": "omittable/inherit" + }, "optional": { "context": [], "name": "optional", diff --git a/tests/site_main_check.habcache b/tests/site_main_check.habcache index c77d935..631f419 100644 --- a/tests/site_main_check.habcache +++ b/tests/site_main_check.habcache @@ -366,6 +366,48 @@ }, "inherits": false }, + "{config-root}/configs/omittable/omittable.json": { + "name": "omittable", + "context": [], + "description": "Test that omittable_distros ignore missing configs.", + "inherits": false, + "distros": { + "maya2020": [ + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c" + ], + "missing_dcc": [ + "the_dcc_plugin_d", + "non-existent-distro" + ] + }, + "omittable_distros": [ + "missing_dcc", + "non-existent-distro" + ] + }, + "{config-root}/configs/omittable/omittable_child.json": { + "name": "child", + "context": [ + "omittable" + ], + "description": "This config should override omittable_distros on its parent.", + "inherits": true, + "omittable_distros": [ + "missing_dcc", + "maya2020", + "non-existent-distro" + ] + }, + "{config-root}/configs/omittable/omittable_inherit.json": { + "name": "inherit", + "context": [ + "omittable" + ], + "description": "This config should inherit omittable_distros from its parent.", + "inherits": true + }, "{config-root}/configs/optional/optional.json": { "name": "optional", "context": [], diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 90a2ce2..538c69b 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -309,6 +309,7 @@ def test_metaclass(): "min_verbosity", "inherits", "name", + "omittable_distros", "optional_distros", "uri", "variables", diff --git a/tests/test_resolver.py b/tests/test_resolver.py index a35150f..d97c1c0 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -133,6 +133,9 @@ class TestDumpForest: " not_set/no_distros", " not_set/no_env", " not_set/os", + "omittable", + " omittable/child", + " omittable/inherit", "optional", " optional/child", "place-holder", diff --git a/tests/test_solver.py b/tests/test_solver.py index 0ed5f86..a643b72 100644 --- a/tests/test_solver.py +++ b/tests/test_solver.py @@ -1,3 +1,4 @@ +import logging from collections import OrderedDict import pytest @@ -87,3 +88,43 @@ def test_solver_errors(uncached_resolver): solver.max_redirects = 0 with pytest.raises(MaxRedirectError, match="Redirect limit of 0 reached"): solver.resolve() + + +def test_omittable(caplog, uncached_resolver): + """Test the solver respects the `omittable` property. This will prevent raising + an error if a distro is required but is not found. + """ + # A set of requirements that includes distros that hab can't find + requirements = OrderedDict( + ( + ("the_dcc", Requirement("the_dcc")), + ("the_dcc_plugin_b", Requirement("the_dcc_plugin_b==0.9")), + ("missing_distro", Requirement("missing_distro")), + ("missing_distro_b", Requirement("missing_distro_b==1.0")), + ) + ) + # By default this should raise an InvalidRequirementError + solver = Solver(requirements, uncached_resolver) + with pytest.raises(InvalidRequirementError, match="requirement: missing_distro"): + solver.resolve() + + # However if that distro is marked as omittable, then a warning is logged + # and no exception is raised. + omittable = ["the_dcc_plugin_b", "missing_distro", "missing_distro_b"] + solver = Solver(requirements, uncached_resolver, omittable=omittable) + caplog.clear() + with caplog.at_level(logging.WARNING): + reqs = solver.resolve() + # This plugin can be found so it is not skipped + assert "the_dcc_plugin_b" not in caplog.text + # These plugins don't exist and will be skipped by the omittable setting. + assert "Skipping missing omitted requirement: missing_distro" in caplog.text + assert "Skipping missing omitted requirement: missing_distro_b==1.0" in caplog.text + check = [ + "the_dcc", + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_d", + "the_dcc_plugin_e", + ] + assert sorted(reqs.keys()) == check