From c2ae21de9df158ee636c6d648447945c15de8315 Mon Sep 17 00:00:00 2001 From: Mike Hendricks Date: Fri, 4 Oct 2024 14:41:46 -0700 Subject: [PATCH] Add omittable_distros support for configs --- README.md | 26 +++ hab/parsers/config.py | 11 ++ hab/parsers/flat_config.py | 4 +- hab/resolver.py | 9 +- hab/solvers.py | 10 +- tests/site_omit/README.md | 10 ++ tests/site_omit/configs/default.json | 20 +++ .../site_omit/configs/omittable_defined.json | 25 +++ .../configs/omittable_inherited.json | 16 ++ .../site_omit/configs/omittable_invalid.json | 24 +++ tests/site_omit/site_omit.json | 16 ++ tests/site_omit/test_omittable.py | 169 ++++++++++++++++++ tests/test_parsing.py | 8 +- tests/test_solver.py | 41 +++++ 14 files changed, 384 insertions(+), 5 deletions(-) create mode 100644 tests/site_omit/README.md create mode 100644 tests/site_omit/configs/default.json create mode 100644 tests/site_omit/configs/omittable_defined.json create mode 100644 tests/site_omit/configs/omittable_inherited.json create mode 100644 tests/site_omit/configs/omittable_invalid.json create mode 100644 tests/site_omit/site_omit.json create mode 100644 tests/site_omit/test_omittable.py diff --git a/README.md b/README.md index 684bf13..a5c237c 100644 --- a/README.md +++ b/README.md @@ -1286,6 +1286,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 9f9c046..6fac9a1 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/resolver.py b/hab/resolver.py index 357d4c5..8d968fe 100644 --- a/hab/resolver.py +++ b/hab/resolver.py @@ -356,18 +356,23 @@ def resolve(self, uri, forced_requirements=None): if forced_requirements is not None: self.forced_requirements = current - 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/site_omit/README.md b/tests/site_omit/README.md new file mode 100644 index 0000000..0fb8dc9 --- /dev/null +++ b/tests/site_omit/README.md @@ -0,0 +1,10 @@ +This is a isolated site configuration specifically for testing omittable_distros. +This allows for custom configuration of the default config without affecting other tests. + +* The [default](configs/default.json) config defines `omittable_distros` that can be +inherited by other distros. It lists two distros that do not exist for this site. +* [omittable/defined](omittable/defined.json) defines `omittable_distros` replacing it with a modified +list of distros to omit for this URI and its children. +* [omittable/inherited](omittable/inherited.json) does not define the `omittable_distros` and inherits +from its parents which aren't defined so it finally inherits the value defined +in the `default` config. diff --git a/tests/site_omit/configs/default.json b/tests/site_omit/configs/default.json new file mode 100644 index 0000000..ecb6533 --- /dev/null +++ b/tests/site_omit/configs/default.json @@ -0,0 +1,20 @@ +{ + "name": "default", + "context": [], + "inherits": false, + "distros": { + "maya2020": [ + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c" + ], + "houdini19.5": [ + "the_dcc_plugin_d" + ] + }, + "omittable_distros": [ + "missing_dcc", + "non-existent-distro", + "the_dcc_plugin_b" + ] +} diff --git a/tests/site_omit/configs/omittable_defined.json b/tests/site_omit/configs/omittable_defined.json new file mode 100644 index 0000000..88a374a --- /dev/null +++ b/tests/site_omit/configs/omittable_defined.json @@ -0,0 +1,25 @@ +{ + "name": "defined", + "context": ["omittable"], + "description": "This config defines omittable_distros and will ignore the default.", + "inherits": true, + "distros": { + "maya2020": [ + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c" + ], + "houdini19.5": [ + "the_dcc_plugin_d" + ], + "missing_dcc": [ + "the_dcc_plugin_d", + "non-existent-distro" + ] + }, + "omittable_distros": [ + "houdini19.5", + "missing_dcc", + "non-existent-distro" + ] +} diff --git a/tests/site_omit/configs/omittable_inherited.json b/tests/site_omit/configs/omittable_inherited.json new file mode 100644 index 0000000..74a875e --- /dev/null +++ b/tests/site_omit/configs/omittable_inherited.json @@ -0,0 +1,16 @@ +{ + "name": "inherited", + "context": ["omittable"], + "description": "This config doesn't define omittable_distros and will inherit from default.", + "inherits": true, + "distros": { + "maya2020": [ + "the_dcc_plugin_a", + "the_dcc_plugin_b" + ], + "missing_dcc": [ + "the_dcc_plugin_d", + "non-existent-distro" + ] + } +} diff --git a/tests/site_omit/configs/omittable_invalid.json b/tests/site_omit/configs/omittable_invalid.json new file mode 100644 index 0000000..0b82a24 --- /dev/null +++ b/tests/site_omit/configs/omittable_invalid.json @@ -0,0 +1,24 @@ +{ + "name": "invalid", + "context": ["omittable"], + "description": "This config doesn't mark the distro missing_dcc as omittable_distros and will cause an InvalidRequirementError.", + "inherits": true, + "distros": { + "maya2020": [ + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c" + ], + "houdini19.5": [ + "the_dcc_plugin_d" + ], + "missing_dcc": [ + "the_dcc_plugin_d", + "non-existent-distro" + ] + }, + "omittable_distros": [ + "houdini19.5", + "non-existent-distro" + ] +} diff --git a/tests/site_omit/site_omit.json b/tests/site_omit/site_omit.json new file mode 100644 index 0000000..7745085 --- /dev/null +++ b/tests/site_omit/site_omit.json @@ -0,0 +1,16 @@ +{ + "append": { + "config_paths": [ + "{relative_root}/configs" + ], + "distro_paths": [ + "{relative_root}/../distros/*" + ] + }, + "set": { + "platforms": [ + "windows", + "linux" + ] + } +} diff --git a/tests/site_omit/test_omittable.py b/tests/site_omit/test_omittable.py new file mode 100644 index 0000000..8fbf8f4 --- /dev/null +++ b/tests/site_omit/test_omittable.py @@ -0,0 +1,169 @@ +import pytest + +from hab import Resolver, Site, errors + + +@pytest.fixture +def omit_resolver(config_root): + """Return a testing resolver with omittable_distros defined in default. + Does not use habcache files.""" + site = Site([config_root / "site_omit" / "site_omit.json"]) + return Resolver(site=site) + + +def test_defined(omit_resolver): + """The `omittable/defined` config defines both distros and omittable_distros. + Ensure the versions are resolved correctly and doesn't raise a + `InvalidRequirementError` exception. + """ + cfg = omit_resolver.resolve("omittable/defined") + assert cfg.omittable_distros == [ + "houdini19.5", + "missing_dcc", + "non-existent-distro", + ] + + # An omitted distro still shows up in the distros list + assert "missing_dcc" in cfg.distros + assert "non-existent-distro" in cfg.distros + + # Omitted distros that were found show up in versions + version_names = [v.distro_name for v in cfg.versions] + assert set(version_names) == set( + [ + "houdini19.5", + "maya2020", + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c", + "the_dcc_plugin_d", + "the_dcc_plugin_e", + ] + ) + + # If a distro is not found it won't be in versions and no errors are raised. + assert "missing_dcc" not in version_names + assert "non-existent-distro" not in version_names + + +def test_inherited(omit_resolver): + """The `omittable/inherited` config only defines distros but not + omittable_distros which are inherited from `default`. Ensure the versions + are resolved correctly and doesn't raise a `InvalidRequirementError` exception. + """ + # This config defines both distros and omittable_distros inside the file + cfg = omit_resolver.resolve("omittable/inherited") + assert cfg.omittable_distros == [ + "missing_dcc", + "non-existent-distro", + "the_dcc_plugin_b", + ] + + # An omitted distro still shows up in the distros list + assert "missing_dcc" in cfg.distros + assert "non-existent-distro" in cfg.distros + + # Omitted distros that were found show up in versions + version_names = [v.distro_name for v in cfg.versions] + assert set(version_names) == set( + [ + "maya2020", + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_d", + "the_dcc_plugin_e", + ] + ) + + # If a distro is not found it won't be in versions and no errors are raised. + assert "missing_dcc" not in version_names + assert "non-existent-distro" not in version_names + + +def test_forced(omit_resolver): + """Checks how omittable_distros handle forced_requirements.""" + + # Passing a missing non-omittable_distro as a forced_requirement should + # still raise an error + with pytest.raises( + errors.InvalidRequirementError, + match=r"Unable to find a distro for requirement: missing_dcc", + ): + omit_resolver.resolve("omittable/invalid", forced_requirements=["missing_dcc"]) + + # Valid forced_requirements are respected normally + cfg = omit_resolver.resolve( + "omittable/inherited", forced_requirements=["houdini19.5", "missing_dcc"] + ) + + # An omitted distro still shows up in the distros list + assert "missing_dcc" in cfg.distros + assert "non-existent-distro" in cfg.distros + + # But one added by forced_requiremnts don't show up there + assert "houdini19.5" not in cfg.distros + + # Forced and omitted distros that were found show up in versions + version_names = [v.distro_name for v in cfg.versions] + assert set(version_names) == set( + [ + "houdini19.5", + "maya2020", + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_d", + "the_dcc_plugin_e", + ] + ) + + # If a distro is not found it won't be in versions and no errors are raised. + assert "missing_dcc" not in version_names + assert "non-existent-distro" not in version_names + + +def test_pure_default(omit_resolver): + """The `omittable/undefined` config is not explicitly defined so all values + are inherited from `default`. Ensure the versions are resolved correctly + and doesn't raise a `InvalidRequirementError` exception. + """ + # This config and its parent are not defined, both distros and omittable_distros + # are only defined in the default configuration. + cfg = omit_resolver.resolve("omittable/undefined") + assert cfg.omittable_distros == [ + "missing_dcc", + "non-existent-distro", + "the_dcc_plugin_b", + ] + + # An omitted distro still shows up in the distros list + assert "missing_dcc" not in cfg.distros + assert "non-existent-distro" not in cfg.distros + + # Omitted distros that were found show up in versions + version_names = [v.distro_name for v in cfg.versions] + assert set(version_names) == set( + [ + "houdini19.5", + "maya2020", + "the_dcc_plugin_a", + "the_dcc_plugin_b", + "the_dcc_plugin_c", + "the_dcc_plugin_d", + "the_dcc_plugin_e", + ] + ) + + # If a distro is not found it won't be in versions and no errors are raised. + assert "missing_dcc" not in version_names + assert "non-existent-distro" not in version_names + + +def test_missing_required(omit_resolver): + """The `omittable/invalid` config requires the `missing_dcc` distro but does + not add it to `omittable_distros`, and will raise `InvalidRequirementError`. + """ + with pytest.raises( + errors.InvalidRequirementError, + match=r"Unable to find a distro for requirement: missing_dcc", + ): + omit_resolver.resolve("omittable/invalid") diff --git a/tests/test_parsing.py b/tests/test_parsing.py index d608a36..4d81378 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", @@ -426,7 +427,7 @@ def test_no_values(self, resolver, uri): """If noting sets aliases or distros, don't include them in the dump text.""" cfg = resolver.resolve(uri) - for verbosity in range(3): + for verbosity in range(4): result = cfg.dump(verbosity=verbosity, color=False) assert "aliases" not in result assert " distros:" not in result @@ -435,6 +436,11 @@ def test_no_values(self, resolver, uri): assert "optional_distros:" not in result else: assert "optional_distros:" in result + if verbosity < 3: + # This is shown for v3 or higher + assert "omittable_distros:" not in result + else: + assert "omittable_distros:" in result def test_environment(resolver): 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