From 6e854e6c0368730c0641b3ddaa44f50de8d3849d Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Wed, 2 Sep 2020 01:30:21 -0400 Subject: [PATCH 1/9] Adds Conditional factories for rezconfig. Allow to specify any value in rezconfig based on platform or any other key. Tests and documentation included. --- src/rez/config.py | 7 +- src/rez/exceptions.py | 12 +++ src/rez/rezconfig.py | 10 ++ src/rez/tests/data/config/test_conditional.py | 27 ++++++ src/rez/tests/test_config.py | 71 ++++++++++++++- src/rez/utils/data_utils.py | 91 ++++++++++++++++++- wiki/pages/_Configuring-Rez.md | 25 +++++ 7 files changed, 239 insertions(+), 4 deletions(-) create mode 100644 src/rez/tests/data/config/test_conditional.py diff --git a/src/rez/config.py b/src/rez/config.py index 185b53e6e..502bd8c64 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -877,6 +877,7 @@ def _replace_config(other): @lru_cache() def _load_config_py(filepath): + from rez.utils.data_utils import Conditional, PlatformDependent, ArchDependent, OsDependent reserved = dict( # Standard Python module variables # Made available from within the module, @@ -886,7 +887,11 @@ def _load_config_py(filepath): rez_version=__version__, ModifyList=ModifyList, - DelayLoad=DelayLoad + DelayLoad=DelayLoad, + Conditional=Conditional, + PlatformDependent=PlatformDependent, + ArchDependent=ArchDependent, + OsDependent=OsDependent ) g = reserved.copy() diff --git a/src/rez/exceptions.py b/src/rez/exceptions.py index a70415aec..44c0cb4a9 100644 --- a/src/rez/exceptions.py +++ b/src/rez/exceptions.py @@ -33,6 +33,18 @@ class ConfigurationError(RezError): pass +class ConditionalConfigurationError(ConfigurationError): + """ + A misconfiguration error due to a missing key in a Conditional. + """ + def __init__(self, missing_key, *args, **kwargs): + super(ConditionalConfigurationError, self).__init__( + "'{}' not found in Conditional options".format(missing_key), + *args, + **kwargs + ) + + class ResolveError(RezError): """A resolve-related error.""" pass diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 50520c499..4c7ee9d07 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -35,6 +35,16 @@ (based on Python's os.path.sep). So for Linux paths, / should be used. On Windows \ (unescaped) should be used. +Platform dependent configurations can be achieved via the PlatformDependent +factory. Read the documentation for details. For example: + default_shell = PlatformDependent( + { + "linux": "bash", + "windows": "powershell", + }, + default="zsh" + ) + Note: The comments in this file are extracted and turned into Wiki content. Pay attention to the comment formatting and follow the existing style closely. """ diff --git a/src/rez/tests/data/config/test_conditional.py b/src/rez/tests/data/config/test_conditional.py new file mode 100644 index 000000000..f4e068cb0 --- /dev/null +++ b/src/rez/tests/data/config/test_conditional.py @@ -0,0 +1,27 @@ +from rez.utils.platform_ import platform_ + +# Test fallback of Conditional +release_hooks = Conditional( + { + "Something": False, + }, + key="3", + default=["foo"] +) + +prune_failed_graph = ArchDependent({ + platform_._arch(): False, +}) + +# Fallback of OsDependent +warn_all = OsDependent({}, default=False) + +plugins = PlatformDependent({ + platform_.name: { + "release_hook": { + "emailer": { + "recipients": ["joe@here.com"] + } + } + } +}) diff --git a/src/rez/tests/test_config.py b/src/rez/tests/test_config.py index 2e8f86136..0f4b65d31 100644 --- a/src/rez/tests/test_config.py +++ b/src/rez/tests/test_config.py @@ -6,7 +6,8 @@ from rez.exceptions import ConfigurationError from rez.config import Config, get_module_root_config, _replace_config from rez.system import system -from rez.utils.data_utils import RO_AttrDictWrapper +from rez.utils.data_utils import RO_AttrDictWrapper, PlatformDependent, \ + ArchDependent, OsDependent from rez.packages import get_developer_package import os import os.path @@ -59,6 +60,74 @@ def _test_overrides(self, c): self._test_basic(c) + def test_conditional_overrides(self): + """Test configuration with platform conditionals""" + + from rez.utils.platform_ import platform_ + platform_name = platform_.name + platform_arch = platform_.arch + platform_os = platform_.os + + c = Config([self.root_config_file], locked=True) + c.validate_data() + + # Schema validation still works? + c.override("debug_none", PlatformDependent({ + platform_name: "Wrong Type", + })) + with self.assertRaises(ConfigurationError): + c.validate_data() + + # Missing valid key or fallback + with self.assertRaises(ConfigurationError): + c.override("debug_none", PlatformDependent({ + "__not__valid__platform__": True, + })) + + c.override("debug_none", PlatformDependent({ + platform_name: True, + })) + + c.override("build_directory", PlatformDependent({ + platform_name: "floober" + })) + + # Usage of fallback key + c.override("plugins.release_vcs.tag_name", PlatformDependent( + { "something else": "Not sure", }, + default="bah" + )) + + # Arch variant + c.override("plugins.release_hook.emailer.sender", ArchDependent({ + platform_arch: "joe.bloggs", + } + )) + + # Os variant + c.override("implicit_packages", OsDependent({ + platform_os: ["a list", "of values"], + } + )) + + c.validate_data() + + self.assertEqual(c.debug_none, True) + self.assertEqual(c.build_directory, "floober") + self.assertEqual(c.plugins.release_vcs.tag_name, "bah") + self.assertEqual(c.plugins.release_hook.emailer.sender, "joe.bloggs") + self.assertListEqual(c.implicit_packages, ["a list", "of values"]) + + def test_conditional_in_file(self): + """Test package config overrides.""" + conf = os.path.join(self.config_path, "test_conditional.py") + c = Config([self.root_config_file, conf]) + + self.assertListEqual(c.plugins.release_hook.emailer.recipients, ["joe@here.com"]) + self.assertEqual(c.release_hooks, ["foo"]) + self.assertEqual(c.prune_failed_graph, False) + self.assertEqual(c.warn_all, True) + def test_1(self): """Test just the root config file.""" diff --git a/src/rez/utils/data_utils.py b/src/rez/utils/data_utils.py index 3adf806aa..3e025f67e 100644 --- a/src/rez/utils/data_utils.py +++ b/src/rez/utils/data_utils.py @@ -1,12 +1,13 @@ """ Utilities related to managing data types. """ +from rez.exceptions import ConditionalConfigurationError import os.path from rez.vendor.schema.schema import Schema, Optional -from rez.exceptions import RexError -from threading import Lock from rez.vendor.six import six +from inspect import isclass +from threading import Lock if six.PY2: from collections import MutableMapping @@ -645,6 +646,92 @@ def getter(self): return cached_property(getter, name=attribute) +class Conditional(object): + """ + Factory class to that constructs a value of given options based on a given + key. + Useful to clearly set conditional values without branches. + + For example: + color = Conditional( + { + "Paris": "bash", + "Texas": "zsh", + }, + key=os.getenv("STUDIO_LOCATION"), + default="bash" + ) + """ + + def __new__(cls, options, key, default=ConditionalConfigurationError): + """ + Returns the option value based on key. If the key does not exist + it will raise the given default exception or return the default value. + + Args: + options: dict of options with potential values to construct + key: key to choose within given options + default: value to return or exception to raise when key is not + found in options + + Raises: KeyError if no other `default` is specified + Returns: option value or default + + """ + try: + return options[key] + except KeyError as e: + if isclass(default) and issubclass(default, BaseException): + raise default(*e.args) + return default + + +class PlatformDependent(Conditional): + """ + Specialized Conditional based on platform's name + """ + + def __new__(cls, options, default=ConditionalConfigurationError): + # Mapped platform depends on config so lazy import + from rez.utils.platform_ import platform_ + return Conditional.__new__(cls, options, key=platform_.name, + default=default) + + +class ArchDependent(Conditional): + """ + Specialized Conditional based on platform's arch + """ + _cache = None + + def __new__(cls, options, default=ConditionalConfigurationError): + # Mapped platform depends on config so lazy import + from rez.utils.platform_ import platform_ + # Do not use platform_mapped properties. This implies that we have to + # use our own caching. + arch_value = cls._cache if cls._cache else platform_._arch() + cls._cache = arch_value + return Conditional.__new__(cls, options, key=arch_value, + default=default) + + +class OsDependent(Conditional): + """ + Specialized Conditional based on platform's os + """ + _cache = None + + def __new__(cls, options, default=ConditionalConfigurationError): + # Mapped platform depends on config so lazy import + from rez.utils.platform_ import platform_ + # Do not use platform_mapped properties. This implies that we have to + # use our own caching. + os_value = cls._cache if cls._cache else platform_._os() + cls._cache = os_value + return Conditional.__new__(cls, options, key=os_value, + default=default) + + # Copyright 2013-2016 Allan Johns. # # This library is free software: you can redistribute it and/or diff --git a/wiki/pages/_Configuring-Rez.md b/wiki/pages/_Configuring-Rez.md index da71c9d2d..57e5bef42 100644 --- a/wiki/pages/_Configuring-Rez.md +++ b/wiki/pages/_Configuring-Rez.md @@ -37,6 +37,31 @@ previous configuration sources (you can also supply a *prepend* argument): release_hooks = ModifyList(append=["custom_release_notify"]) +## Conditionals and platform differences + +Platform dependent configurations can be best expressed with the +`PlatformDependent` factory. For example: + + default_shell = PlatformDependent( + { + "linux": "bash", + "windows": "powershell", + }, + default="zsh" + ) + +Similar factories exist as `OsDependent` and `ArchDependent`, but please note +that their keys are not respecting the [platform_map](#platform_map). +A generic conditional with any key can be expressed via: + + default_shell = Conditional( + { + "Paris": "bash", + "Texas": "zsh", + }, + key=os.getenviron["STUDIO_LOCATION"] + ) + ## Package Overrides Packages themselves can override configuration settings. To show how this is useful, From 93f1849aff887762daaa8e302dff5c32e0bf1659 Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Mon, 16 Sep 2019 16:52:51 -0400 Subject: [PATCH 2/9] Better support for platform_map in Os and Arch conditionals. Since the conditionals can be called within a config, and this config might have a platform_map we try to respect it best we can. In an config.override() the containing configuration can not be known so it needs to be passed explicitly. --- src/rez/config.py | 3 +- src/rez/tests/data/config/test_conditional.py | 8 ++- src/rez/tests/test_config.py | 23 +++++--- src/rez/utils/data_utils.py | 52 ++++++++++++------- src/rez/utils/platform_.py | 42 +++++++++++---- src/rez/utils/platform_mapped.py | 15 ++++-- 6 files changed, 102 insertions(+), 41 deletions(-) diff --git a/src/rez/config.py b/src/rez/config.py index 502bd8c64..5faaac351 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -877,7 +877,8 @@ def _replace_config(other): @lru_cache() def _load_config_py(filepath): - from rez.utils.data_utils import Conditional, PlatformDependent, ArchDependent, OsDependent + from rez.utils.data_utils import Conditional, PlatformDependent, \ + ArchDependent, OsDependent reserved = dict( # Standard Python module variables # Made available from within the module, diff --git a/src/rez/tests/data/config/test_conditional.py b/src/rez/tests/data/config/test_conditional.py index f4e068cb0..1f517f244 100644 --- a/src/rez/tests/data/config/test_conditional.py +++ b/src/rez/tests/data/config/test_conditional.py @@ -1,5 +1,11 @@ from rez.utils.platform_ import platform_ +platform_map = { + "arch": { + ".*": "IMPOSSIBLE_ARCH", + }, +} + # Test fallback of Conditional release_hooks = Conditional( { @@ -10,7 +16,7 @@ ) prune_failed_graph = ArchDependent({ - platform_._arch(): False, + "IMPOSSIBLE_ARCH": True, }) # Fallback of OsDependent diff --git a/src/rez/tests/test_config.py b/src/rez/tests/test_config.py index 0f4b65d31..ffabd341a 100644 --- a/src/rez/tests/test_config.py +++ b/src/rez/tests/test_config.py @@ -66,9 +66,13 @@ def test_conditional_overrides(self): from rez.utils.platform_ import platform_ platform_name = platform_.name platform_arch = platform_.arch - platform_os = platform_.os + platform_os = "IMPOSSIBLE_OS" c = Config([self.root_config_file], locked=True) + c.override("platform_map", { + "os": {".*": "IMPOSSIBLE_OS"}, + }) + c.validate_data() # Schema validation still works? @@ -104,11 +108,16 @@ def test_conditional_overrides(self): } )) - # Os variant - c.override("implicit_packages", OsDependent({ - platform_os: ["a list", "of values"], - } - )) + # Os variant with explicit platform_map override + c.override( + "implicit_packages", + OsDependent( + { + platform_os: ["a list", "of values"], + }, + platform_map=c.platform_map + ) + ) c.validate_data() @@ -125,7 +134,7 @@ def test_conditional_in_file(self): self.assertListEqual(c.plugins.release_hook.emailer.recipients, ["joe@here.com"]) self.assertEqual(c.release_hooks, ["foo"]) - self.assertEqual(c.prune_failed_graph, False) + self.assertEqual(c.prune_failed_graph, True) self.assertEqual(c.warn_all, True) def test_1(self): diff --git a/src/rez/utils/data_utils.py b/src/rez/utils/data_utils.py index 3e025f67e..9b3343369 100644 --- a/src/rez/utils/data_utils.py +++ b/src/rez/utils/data_utils.py @@ -702,16 +702,25 @@ class ArchDependent(Conditional): """ Specialized Conditional based on platform's arch """ - _cache = None - def __new__(cls, options, default=ConditionalConfigurationError): - # Mapped platform depends on config so lazy import - from rez.utils.platform_ import platform_ - # Do not use platform_mapped properties. This implies that we have to - # use our own caching. - arch_value = cls._cache if cls._cache else platform_._arch() - cls._cache = arch_value - return Conditional.__new__(cls, options, key=arch_value, + def __new__(cls, options, default=ConditionalConfigurationError, + platform_map=None): + + if platform_map is None: + # The owner of this conditional might be a config and as such we + # have to respect its platform_map as opposed to the platform map of + # a global config. + import inspect + x = inspect.currentframe().f_back + platform_map = inspect.currentframe().f_back.f_locals.get( + "platform_map", + None + ) + + from rez.utils.platform_ import create_platform + platform_ = create_platform(platform_map=platform_map) + + return Conditional.__new__(cls, options, key=platform_.arch, default=default) @@ -719,16 +728,23 @@ class OsDependent(Conditional): """ Specialized Conditional based on platform's os """ - _cache = None - def __new__(cls, options, default=ConditionalConfigurationError): - # Mapped platform depends on config so lazy import - from rez.utils.platform_ import platform_ - # Do not use platform_mapped properties. This implies that we have to - # use our own caching. - os_value = cls._cache if cls._cache else platform_._os() - cls._cache = os_value - return Conditional.__new__(cls, options, key=os_value, + def __new__(cls, options, default=ConditionalConfigurationError, + platform_map=None): + + if platform_map is None: + # Compare to ArchDependent. + import inspect + x = inspect.currentframe().f_back + platform_map = inspect.currentframe().f_back.f_locals.get( + "platform_map", + None + ) + + from rez.utils.platform_ import create_platform + platform_ = create_platform(platform_map=platform_map) + + return Conditional.__new__(cls, options, key=platform_.os, default=default) diff --git a/src/rez/utils/platform_.py b/src/rez/utils/platform_.py index d0f03f219..5c25f2f8a 100644 --- a/src/rez/utils/platform_.py +++ b/src/rez/utils/platform_.py @@ -17,8 +17,14 @@ class Platform(object): """ name = None - def __init__(self): - pass + def __init__(self, platform_map=None): + """ + Construct platform + + Args: + platform_map: Force explicit platform map + """ + self._platform_map = platform_map @cached_property @platform_mapped @@ -570,15 +576,31 @@ def _difftool(self): return which("meld", "fc") +def create_platform(name=None, platform_map=None): + """ + Static factory for + + Args: + name (str): force a particular platform construction + platform_map (dict): Explicit platform_map. Defaults to platform_ma[ + from config. + + Returns: + Platform or None + """ + if name is None: + name = name=platform.system().lower() + if name == "linux": + return LinuxPlatform(platform_map=platform_map) + elif name == "darwin": + return OSXPlatform(platform_map=platform_map) + elif name == "windows": + return WindowsPlatform(platform_map=platform_map) + return None + + # singleton -platform_ = None -name = platform.system().lower() -if name == "linux": - platform_ = LinuxPlatform() -elif name == "darwin": - platform_ = OSXPlatform() -elif name == "windows": - platform_ = WindowsPlatform() +platform_ = create_platform() # Copyright 2013-2016 Allan Johns. diff --git a/src/rez/utils/platform_mapped.py b/src/rez/utils/platform_mapped.py index b63c7b182..2ded0eb77 100644 --- a/src/rez/utils/platform_mapped.py +++ b/src/rez/utils/platform_mapped.py @@ -23,15 +23,22 @@ def platform_mapped(func): """ def inner(*args, **kwargs): - # Since platform is being used within config lazy import config to prevent - # circular dependencies - from rez.config import config + # The caller could have a _platform_map attribute which overrides the + # default config.platform_map. + platform_map = None + if len(args) > 0: + platform_map = getattr(args[0], "_platform_map", None) + if platform_map is None: + # Since platform is being used within config lazy import config to + # prevent circular dependencies + from rez.config import config + platform_map = config.platform_map # Original result result = func(*args, **kwargs) # The function name is used as primary key - entry = config.platform_map.get(func.__name__) + entry = platform_map.get(func.__name__) if entry: for key, value in entry.items(): result, changes = re.subn(key, value, result) From 987e849380bd4e79dbf69472a40d6ffe39ca8e45 Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Mon, 16 Sep 2019 17:41:35 -0400 Subject: [PATCH 3/9] Replaced platform_mapped decorator. This makes less assumptions. No need to have introduced a decorator for two functions only (original author speaking). --- src/rez/utils/platform_.py | 51 ++++++++++++++++++++++++++++---- src/rez/utils/platform_mapped.py | 49 ------------------------------ 2 files changed, 46 insertions(+), 54 deletions(-) delete mode 100644 src/rez/utils/platform_mapped.py diff --git a/src/rez/utils/platform_.py b/src/rez/utils/platform_.py index 5c25f2f8a..8a9ee9910 100644 --- a/src/rez/utils/platform_.py +++ b/src/rez/utils/platform_.py @@ -7,7 +7,6 @@ from rez.util import which from rez.utils.execution import Popen from rez.utils.data_utils import cached_property -from rez.utils.platform_mapped import platform_mapped from rez.exceptions import RezSystemError from tempfile import gettempdir @@ -26,17 +25,59 @@ def __init__(self, platform_map=None): """ self._platform_map = platform_map + def _platform_mapped(self, key, value): + """Looks the value up within the _platform_map of the instance or the + config.platform_map dictionary. + + Note that there is no guaranteed order within the dictionary evaluation. + Only the first matching regular expression is being used. + For example: + + config.platform_map = { + "os": { + r"Scientific Linux-(.*)": r"Scientific-\1", # Scientific Linux-x.x -> Scientific-x.x + r"Ubuntu-14.\d": r"Ubuntu-14", # Any Ubuntu-14.x -> Ubuntu-14 + }, + "arch": { + "x86_64": "64bit", # Maps both x86_64 and amd64 -> 64bit (don't) + "amd64": "64bit", + }, + } + + Args: + key (str): key to use in the platform_map + value (str): the value to map + + Returns: + platform mapped value or original value if no map is found + + """ + platform_map = self._platform_map + if platform_map is None: + # Since platform is being used within config lazy import config to + # prevent circular dependencies + from rez.config import config + platform_map = config.platform_map + + # The function name is used as primary key + entry = platform_map.get(key) + if entry: + for key, value in entry.iteritems(): + value, changes = re.subn(key, value, value) + if changes > 0: + break + + return value + @cached_property - @platform_mapped def arch(self): """Returns the name of the architecture.""" - return self._arch() + return self._platform_mapped("arch", self._arch()) @cached_property - @platform_mapped def os(self): """Returns the name of the operating system.""" - return self._os() + return self._platform_mapped("os", self._os()) @cached_property def terminal_emulator_command(self): diff --git a/src/rez/utils/platform_mapped.py b/src/rez/utils/platform_mapped.py deleted file mode 100644 index 2ded0eb77..000000000 --- a/src/rez/utils/platform_mapped.py +++ /dev/null @@ -1,49 +0,0 @@ -import re - - -def platform_mapped(func): - """Decorates functions for lookups within a config.platform_map dictionary. - - The first level key is mapped to the func.__name__ of the decorated function. - Regular expressions are used on the second level key, values. - Note that there is no guaranteed order within the dictionary evaluation. Only the first matching - regular expression is being used. - For example: - - config.platform_map = { - "os": { - r"Scientific Linux-(.*)": r"Scientific-\1", # Scientific Linux-x.x -> Scientific-x.x - r"Ubuntu-14.\d": r"Ubuntu-14", # Any Ubuntu-14.x -> Ubuntu-14 - }, - "arch": { - "x86_64": "64bit", # Maps both x86_64 and amd64 -> 64bit (don't) - "amd64": "64bit", - }, - } - """ - def inner(*args, **kwargs): - - # The caller could have a _platform_map attribute which overrides the - # default config.platform_map. - platform_map = None - if len(args) > 0: - platform_map = getattr(args[0], "_platform_map", None) - if platform_map is None: - # Since platform is being used within config lazy import config to - # prevent circular dependencies - from rez.config import config - platform_map = config.platform_map - - # Original result - result = func(*args, **kwargs) - - # The function name is used as primary key - entry = platform_map.get(func.__name__) - if entry: - for key, value in entry.items(): - result, changes = re.subn(key, value, result) - if changes > 0: - break - - return result - return inner From 0f2597951fdb546c76917ed74c6637d966cba3a2 Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Tue, 17 Sep 2019 09:59:19 -0400 Subject: [PATCH 4/9] Makes in-config Os- and ArchDepedent inspection explicit to this case. Before it would affect the general use of the classes so I made the particular case where these Conditionals are bound explicit classes. --- src/rez/config.py | 6 ++-- src/rez/utils/data_utils.py | 59 ++++++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/rez/config.py b/src/rez/config.py index 5faaac351..f874ae077 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -878,7 +878,7 @@ def _replace_config(other): @lru_cache() def _load_config_py(filepath): from rez.utils.data_utils import Conditional, PlatformDependent, \ - ArchDependent, OsDependent + InConfigArchDependent, InConfigOsDependent reserved = dict( # Standard Python module variables # Made available from within the module, @@ -891,8 +891,8 @@ def _load_config_py(filepath): DelayLoad=DelayLoad, Conditional=Conditional, PlatformDependent=PlatformDependent, - ArchDependent=ArchDependent, - OsDependent=OsDependent + ArchDependent=InConfigArchDependent, + OsDependent=InConfigOsDependent ) g = reserved.copy() diff --git a/src/rez/utils/data_utils.py b/src/rez/utils/data_utils.py index 9b3343369..a9b9c9f09 100644 --- a/src/rez/utils/data_utils.py +++ b/src/rez/utils/data_utils.py @@ -6,7 +6,7 @@ from rez.vendor.schema.schema import Schema, Optional from rez.vendor.six import six -from inspect import isclass +from inspect import isclass, currentframe from threading import Lock if six.PY2: @@ -706,17 +706,6 @@ class ArchDependent(Conditional): def __new__(cls, options, default=ConditionalConfigurationError, platform_map=None): - if platform_map is None: - # The owner of this conditional might be a config and as such we - # have to respect its platform_map as opposed to the platform map of - # a global config. - import inspect - x = inspect.currentframe().f_back - platform_map = inspect.currentframe().f_back.f_locals.get( - "platform_map", - None - ) - from rez.utils.platform_ import create_platform platform_ = create_platform(platform_map=platform_map) @@ -732,15 +721,6 @@ class OsDependent(Conditional): def __new__(cls, options, default=ConditionalConfigurationError, platform_map=None): - if platform_map is None: - # Compare to ArchDependent. - import inspect - x = inspect.currentframe().f_back - platform_map = inspect.currentframe().f_back.f_locals.get( - "platform_map", - None - ) - from rez.utils.platform_ import create_platform platform_ = create_platform(platform_map=platform_map) @@ -748,6 +728,43 @@ def __new__(cls, options, default=ConditionalConfigurationError, default=default) +class InspectedDependent(Conditional): + + def __new__(cls, base, frame, options, default=ConditionalConfigurationError): + + # The owner of this conditional might be a config and as such we + # have to respect its platform_map as opposed to the platform map of + # a global config. + platform_map = frame.f_locals.get( + "platform_map", + None + ) + + return base(options, default, platform_map=platform_map) + + +class InConfigArchDependent(ArchDependent): + """ + Constructs an ArchDependent that works with the platform_map within its own + config. + """ + + def __new__(cls, options, default=ConditionalConfigurationError): + return InspectedDependent(ArchDependent, currentframe().f_back, options, + default) + + +class InConfigOsDependent(OsDependent): + """ + Constructs an OsDependent that works with the platform_map within its own + config. + """ + + def __new__(cls, options, default=ConditionalConfigurationError): + return InspectedDependent(OsDependent, currentframe().f_back, options, + default) + + # Copyright 2013-2016 Allan Johns. # # This library is free software: you can redistribute it and/or From cd75ac517e97e7dd7020256727783a00c5fe7016 Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Wed, 11 Sep 2019 14:58:29 -0400 Subject: [PATCH 5/9] Fixes unnecassary config.plugins clearing if override happens after plugin override --- src/rez/config.py | 6 +++--- src/rez/tests/test_config.py | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rez/config.py b/src/rez/config.py index f874ae077..aa4c883bc 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -536,7 +536,7 @@ def override(self, key, value): self.plugins.override(keys[1:], value) else: self.overrides[key] = value - self._uncache(key) + self._uncache(key, keep_plugins=True) def is_overridden(self, key): return (key in self.overrides) @@ -636,7 +636,7 @@ def _get_plugin_completions(prefix_): keys += _get_plugin_completions('') return keys - def _uncache(self, key=None): + def _uncache(self, key=None, keep_plugins=False): # deleting the attribute falls up back to the class attribute, which is # the cached_property descriptor if key and hasattr(self, key): @@ -647,7 +647,7 @@ def _uncache(self, key=None): if hasattr(self, "_data"): delattr(self, "_data") - if hasattr(self, "plugins"): + if not keep_plugins and hasattr(self, "plugins"): delattr(self, "plugins") def _swap(self, other): diff --git a/src/rez/tests/test_config.py b/src/rez/tests/test_config.py index ffabd341a..562888644 100644 --- a/src/rez/tests/test_config.py +++ b/src/rez/tests/test_config.py @@ -39,11 +39,13 @@ def _test_overrides(self, c): c.override("build_directory", "floober") c.override("plugins.release_vcs.tag_name", "bah") c.override("plugins.release_hook.emailer.sender", "joe.bloggs") + c.override("implicit_packages", ["a list", "of values"]) self.assertEqual(c.debug_none, True) self.assertEqual(c.build_directory, "floober") self.assertEqual(c.plugins.release_vcs.tag_name, "bah") self.assertEqual(c.plugins.release_hook.emailer.sender, "joe.bloggs") + self.assertListEqual(c.implicit_packages, ["a list", "of values"]) # second override c.override("build_directory", "flabber") From 29be01a3214209fc55eee54280df1f4e5b9dbd0a Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Wed, 23 Oct 2019 10:12:02 -0400 Subject: [PATCH 6/9] Python3 --- src/rez/utils/platform_.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rez/utils/platform_.py b/src/rez/utils/platform_.py index 8a9ee9910..fe6d09a66 100644 --- a/src/rez/utils/platform_.py +++ b/src/rez/utils/platform_.py @@ -62,7 +62,7 @@ def _platform_mapped(self, key, value): # The function name is used as primary key entry = platform_map.get(key) if entry: - for key, value in entry.iteritems(): + for key, value in entry.items(): value, changes = re.subn(key, value, value) if changes > 0: break From 15d0148b60c6970a657af47f6a66d35b3320a555 Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Mon, 4 Nov 2019 14:41:26 -0500 Subject: [PATCH 7/9] Fix repetition of keys in platform_map --- src/rez/tests/data/config/test_conditional.py | 2 +- src/rez/tests/test_config.py | 2 +- src/rez/utils/platform_.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rez/tests/data/config/test_conditional.py b/src/rez/tests/data/config/test_conditional.py index 1f517f244..d7a519333 100644 --- a/src/rez/tests/data/config/test_conditional.py +++ b/src/rez/tests/data/config/test_conditional.py @@ -2,7 +2,7 @@ platform_map = { "arch": { - ".*": "IMPOSSIBLE_ARCH", + "^.*$": "IMPOSSIBLE_ARCH", }, } diff --git a/src/rez/tests/test_config.py b/src/rez/tests/test_config.py index 562888644..d5eee77f6 100644 --- a/src/rez/tests/test_config.py +++ b/src/rez/tests/test_config.py @@ -72,7 +72,7 @@ def test_conditional_overrides(self): c = Config([self.root_config_file], locked=True) c.override("platform_map", { - "os": {".*": "IMPOSSIBLE_OS"}, + "os": {"^.*$": "IMPOSSIBLE_OS"}, }) c.validate_data() diff --git a/src/rez/utils/platform_.py b/src/rez/utils/platform_.py index fe6d09a66..ea5c5812f 100644 --- a/src/rez/utils/platform_.py +++ b/src/rez/utils/platform_.py @@ -62,8 +62,8 @@ def _platform_mapped(self, key, value): # The function name is used as primary key entry = platform_map.get(key) if entry: - for key, value in entry.items(): - value, changes = re.subn(key, value, value) + for key, map_value in entry.items(): + value, changes = re.subn(key, map_value, value) if changes > 0: break From 045e553310c8e03bfa085fcfe4d6672a680c4715 Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Wed, 2 Sep 2020 01:33:29 -0400 Subject: [PATCH 8/9] Take previous config platform_map into account. --- src/rez/config.py | 18 +++++--- .../data/config/test_conditional_dependee.py | 4 ++ src/rez/tests/test_config.py | 13 ++++++ src/rez/tests/test_utils.py | 23 +++++++++++ src/rez/utils/data_utils.py | 41 ++++++++++++++++++- 5 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 src/rez/tests/data/config/test_conditional_dependee.py diff --git a/src/rez/config.py b/src/rez/config.py index aa4c883bc..dcaff58e0 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -2,7 +2,7 @@ from rez import __version__ from rez.utils.data_utils import AttrDictWrapper, RO_AttrDictWrapper, \ convert_dicts, cached_property, cached_class_property, LazyAttributeMeta, \ - deep_update, ModifyList, DelayLoad + deep_update, ModifyList, DelayLoad, HashableDict from rez.utils.formatting import expandvars, expanduser from rez.utils.logging_ import get_debug_printer from rez.utils.scope import scoped_format @@ -876,7 +876,7 @@ def _replace_config(other): @lru_cache() -def _load_config_py(filepath): +def _load_config_py(filepath, fallback_platform_map): from rez.utils.data_utils import Conditional, PlatformDependent, \ InConfigArchDependent, InConfigOsDependent reserved = dict( @@ -885,6 +885,7 @@ def _load_config_py(filepath): # and later excluded from the `Config` class __name__=os.path.splitext(os.path.basename(filepath))[0], __file__=filepath, + __fallback_platform_map=fallback_platform_map, rez_version=__version__, ModifyList=ModifyList, @@ -892,7 +893,7 @@ def _load_config_py(filepath): Conditional=Conditional, PlatformDependent=PlatformDependent, ArchDependent=InConfigArchDependent, - OsDependent=InConfigOsDependent + OsDependent=InConfigOsDependent, ) g = reserved.copy() @@ -909,14 +910,15 @@ def _load_config_py(filepath): for k, v in g.items(): if k != '__builtins__' \ and not ismodule(v) \ - and k not in reserved: + and k not in reserved \ + and k != "__fallback_platform_map": result[k] = v return result @lru_cache() -def _load_config_yaml(filepath): +def _load_config_yaml(filepath, _): with open(filepath) as f: content = f.read() try: @@ -948,7 +950,11 @@ def _load_config_from_filepaths(filepaths): if not os.path.isfile(filepath_with_ext): continue - data_ = loader(filepath_with_ext) + previous_platform_map = data.get("platform_map", None) + if previous_platform_map is not None: + previous_platform_map = HashableDict(previous_platform_map) + + data_ = loader(filepath_with_ext, previous_platform_map) deep_update(data, data_) sourced_filepaths.append(filepath_with_ext) break diff --git a/src/rez/tests/data/config/test_conditional_dependee.py b/src/rez/tests/data/config/test_conditional_dependee.py new file mode 100644 index 000000000..71fcc3860 --- /dev/null +++ b/src/rez/tests/data/config/test_conditional_dependee.py @@ -0,0 +1,4 @@ + +dot_image_format = ArchDependent({ + "IMPOSSIBLE_ARCH": "hello", +}) diff --git a/src/rez/tests/test_config.py b/src/rez/tests/test_config.py index d5eee77f6..1bcb9de89 100644 --- a/src/rez/tests/test_config.py +++ b/src/rez/tests/test_config.py @@ -139,6 +139,19 @@ def test_conditional_in_file(self): self.assertEqual(c.prune_failed_graph, True) self.assertEqual(c.warn_all, True) + def test_conidition_nested(self): + conf = os.path.join(self.config_path, "test_conditional.py") + conf_dependee = os.path.join(self.config_path, "test_conditional_dependee.py") + c = Config([conf, conf_dependee]) + self.assertEqual(c.dot_image_format, "hello") + + def test_conidition_nested_inbeween(self): + conf = os.path.join(self.config_path, "test_conditional.py") + conf_middle = os.path.join(self.config_path, "test2.py") + conf_dependee = os.path.join(self.config_path, "test_conditional_dependee.py") + c = Config([conf, conf_middle, conf_dependee]) + self.assertEqual(c.dot_image_format, "hello") + def test_1(self): """Test just the root config file.""" diff --git a/src/rez/tests/test_utils.py b/src/rez/tests/test_utils.py index c79721e76..ba34867b1 100644 --- a/src/rez/tests/test_utils.py +++ b/src/rez/tests/test_utils.py @@ -2,8 +2,10 @@ unit tests for 'utils.filesystem' module """ import os +from collections import OrderedDict from rez.tests.util import TestBase from rez.utils import filesystem +from rez.utils.data_utils import HashableDict from rez.utils.platform_ import Platform, platform_ @@ -46,6 +48,27 @@ def test_unix_case_insensistive_platform(self): self.assertEqual(path, expects) +class TestDataUtils(TestBase): + + def test_hashabledict(self): + # Make sure that hashes matches even on dicts with different order. + + a = HashableDict(OrderedDict([("a", 1), ("b", 3)])) + b = HashableDict(OrderedDict([("b", 3), ("a", 1)])) + c = HashableDict(OrderedDict([("a", 1), ("c", 3)])) + + self.assertEqual(a, b) + self.assertNotEqual(a, c) + + # Immutable + + with self.assertRaises(TypeError): + a["d"] = 5 + + self.assertEqual(a, b) + + + # Copyright 2013-2016 Allan Johns. # # This library is free software: you can redistribute it and/or diff --git a/src/rez/utils/data_utils.py b/src/rez/utils/data_utils.py index a9b9c9f09..cf16c06f3 100644 --- a/src/rez/utils/data_utils.py +++ b/src/rez/utils/data_utils.py @@ -1,6 +1,7 @@ """ Utilities related to managing data types. """ +from collections import Mapping from rez.exceptions import ConditionalConfigurationError import os.path @@ -737,7 +738,10 @@ def __new__(cls, base, frame, options, default=ConditionalConfigurationError): # a global config. platform_map = frame.f_locals.get( "platform_map", - None + frame.f_locals.get( + "__fallback_platform_map", + None + ) ) return base(options, default, platform_map=platform_map) @@ -765,6 +769,41 @@ def __new__(cls, options, default=ConditionalConfigurationError): default) +class HashableDict(Mapping): + """ + Immutable Hashable dict + + Hash is based on key and value. + It supports nested dict-like values. + It does ignore any order of the items. + + Could be replaced in future with: + https://www.python.org/dev/peps/pep-0603/#why-frozenmap-and-not-frozendict + """ + + def __init__(self, *args, **kwargs): + self._d = dict(*args, **kwargs) + self._hash = None + + for k, v in self._d.items(): + if isinstance(v, dict): + self._d[k] = HashableDict(v) + + def __iter__(self): + return iter(self._d) + + def __len__(self): + return len(self._d) + + def __getitem__(self, key): + return self._d[key] + + def __hash__(self): + return hash( + (frozenset(six.iteritems(self._d)), frozenset(six.itervalues(self._d))) + ) + + # Copyright 2013-2016 Allan Johns. # # This library is free software: you can redistribute it and/or From 2b5096f4cb3d79467aad61565cd2fc9122d06b0e Mon Sep 17 00:00:00 2001 From: Blazej Floch Date: Mon, 4 Nov 2019 16:26:39 -0500 Subject: [PATCH 9/9] Fixes Python3 tests In python 3 the caught execption is not recoverable in an override. Since we are testing for expected failure it is probably ok to not expect any tests after the failure to pass. --- src/rez/tests/test_config.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/rez/tests/test_config.py b/src/rez/tests/test_config.py index 1bcb9de89..b6ad727fe 100644 --- a/src/rez/tests/test_config.py +++ b/src/rez/tests/test_config.py @@ -77,13 +77,6 @@ def test_conditional_overrides(self): c.validate_data() - # Schema validation still works? - c.override("debug_none", PlatformDependent({ - platform_name: "Wrong Type", - })) - with self.assertRaises(ConfigurationError): - c.validate_data() - # Missing valid key or fallback with self.assertRaises(ConfigurationError): c.override("debug_none", PlatformDependent({ @@ -129,6 +122,14 @@ def test_conditional_overrides(self): self.assertEqual(c.plugins.release_hook.emailer.sender, "joe.bloggs") self.assertListEqual(c.implicit_packages, ["a list", "of values"]) + with self.assertRaises(ConfigurationError): + # Schema validation still works? + c.override("debug_none", PlatformDependent({ + platform_name: "Wrong Type", + })) + c.validate_data() + + def test_conditional_in_file(self): """Test package config overrides.""" conf = os.path.join(self.config_path, "test_conditional.py")