Repository: nerdvegas/rez
Pull request #1030 (2021-03-01 02:14:09)

add EphemeralsBinding.get_range
### Problem

In PR #993, new package command `ephemerals` and `intersects` were introduced, however it may not work as expect when you wish to disable a package feature (or other term ?) by default as this [example](https://github.com/nerdvegas/rez/wiki/Ephemeral-Packages#introspection).

See demo code below :
```python
from rez.rex_bindings import EphemeralsBinding, intersects
from rez.vendor.version.requirement import Requirement

# Topic: Is foo.bar on ? Default off
# case 1
ephemerals = EphemeralsBinding([Requirement(".foo.bar-1")])
bar_on = intersects(ephemerals.get("foo.bar", "0"), "1")
assert bar_on is True  # It's on.
# case 2
ephemerals = EphemeralsBinding([])
bar_on = intersects(ephemerals.get("foo.bar", "0"), "1")
assert bar_on is True  # Still on !
# case 3 (workaround)
ephemerals = EphemeralsBinding([])
bar_on = intersects(ephemerals.get("foo.bar", "foo.bar-0"), "1")
assert bar_on is False  # Now it's off..
```
As you can see, `intersects` still returns `True` even no ephemeral requirement given, unless you explicitly set default value to `foo.bar-0`.

Turns out this behavior was because the default value of `ephemerals.get` got parsed into `Requirement` obj in `intersects` :
https://github.com/nerdvegas/rez/blob/6ee535a71b7cf7e04c82db6a786a7cdffe65688f/src/rez/rex_bindings.py#L231-L233

So when no matched ephemeral given like in case 2 above, it was actually intersecting `Requirement('0')` with `VersionRange('1')`. Which always intersects !

### Feature propose

To overcome this problem and provide a much intuitive interface, I propose to add `ephemerals.get_range`, which should always return `VersionRange` object for intersecting.

With this, the above demo code will then be :
```python
# Topic: Is foo.bar on ? Default off
# case 1
ephemerals = EphemeralsBinding([Requirement(".foo.bar-1")])
bar_on = intersects(ephemerals.get_range("foo.bar", "0"), "1")
assert bar_on is True  # It's on.
# case 2
ephemerals = EphemeralsBinding([])
bar_on = intersects(ephemerals.get_range("foo.bar", "0"), "1")
assert bar_on is False  # It's off.
```

Hope this make sense to you all :)



Patch:
diff --git a/src/rez/rex_bindings.py b/src/rez/rex_bindings.py
index 750b3110..7a1e8477 100644
--- a/src/rez/rex_bindings.py
+++ b/src/rez/rex_bindings.py
@@ -173,6 +173,14 @@ class RequirementsBinding(RO_MappingBinding):
     def _attr_error(self, attr):
         raise AttributeError("request does not exist: '%s'" % attr)
 
+    def get_range(self, name, default=None):
+        """Returns requirement version range object"""
+        req_str = self._data.get(name)
+        if req_str:
+            return Requirement(req_str).range
+        elif default is not None:
+            return VersionRange(default)
+
 
 class EphemeralsBinding(RO_MappingBinding):
     """Binds a list of resolved ephemeral packages.
@@ -193,6 +201,14 @@ class EphemeralsBinding(RO_MappingBinding):
     def _attr_error(self, attr):
         raise AttributeError("ephemeral does not exist: '%s'" % attr)
 
+    def get_range(self, name, default=None):
+        """Returns ephemeral version range object"""
+        req_str = self._data.get(name)
+        if req_str:
+            return Requirement(req_str).range
+        elif default is not None:
+            return VersionRange(default)
+
 
 def intersects(obj, range_):
     """Test if an object intersects with the given version range.
@@ -235,6 +251,10 @@ def intersects(obj, range_):
             return False
         range2 = req.range
 
+    # eg 'if intersects(ephemerals.get_range('foo.cli', '1'), ...)'
+    elif isinstance(obj, VersionRange):
+        range2 = obj
+
     # eg 'if intersects(resolve.maya, ...)'
     elif isinstance(obj, VariantBinding):
         range2 = VersionRange(str(obj.version))
diff --git a/wiki/pages/Ephemeral-Packages.md b/wiki/pages/Ephemeral-Packages.md
index 15c3b0a6..d794fd57 100644
--- a/wiki/pages/Ephemeral-Packages.md
+++ b/wiki/pages/Ephemeral-Packages.md
@@ -80,7 +80,7 @@ to the [resolve](Package-Commands#resolve) object. You would typically use the
 
     # in package.py
     def commands()
-        if intersects(ephemerals.get('enable_tracking', '0'), '1'):
+        if intersects(ephemerals.get_range('enable_tracking', '0'), '1'):
             env.TRACKING_ENABLED = 1
 
 In this example, the given package would set the `TRACKING_ENABLED` environment
@@ -88,6 +88,11 @@ variable if an ephemeral such as `.enable_tracking-1` (or `.enable_tracking-1.2+
 etc) is present in the resolve. Note that the leading `.` is implied and not
 included when querying the `ephemerals` object.
 
+> [[media/icons/warning.png]] Since `ephemerals` is a dict-like object, so it has
+> a `get` function which will return a full request string if key exists. Hence,
+> the default value should also be a full request string, not just a version range
+> string like `'0'` in `get_range`. Or `intersects` may not work as expect. 
+
 ## Ephemeral Use Cases
 
 Why would you want to request packages that don't exist? There are two main use
@@ -101,7 +106,7 @@ to packages in a resolve. For example, consider the following package definition
     name = 'bah'
 
     def commands():
-        if intersects(ephemerals.get('bah.cli', '1'), '1'):
+        if intersects(ephemerals.get_range('bah.cli', '1'), '1'):
             env.PATH.append('{root}/bin')
 
 This package will disable its command line tools if an ephemeral like `.bah.cli-0`
@@ -120,7 +125,7 @@ we introduce a `.cli` ephemeral that acts as a global whitelist:
     name = 'bah'
 
     def commands():
-        if intersects(ephemerals.get('cli', ''), 'bah'):
+        if intersects(ephemerals.get_range('cli', ''), 'bah'):
             env.PATH.append('{root}/bin')
 
 Here, all packages' cli will be enabled if `.cli` is not specified, but if it is
diff --git a/wiki/pages/Package-Commands.md b/wiki/pages/Package-Commands.md
index 54be7505..f3f4c0ee 100644
--- a/wiki/pages/Package-Commands.md
+++ b/wiki/pages/Package-Commands.md
@@ -346,14 +346,23 @@ like *env.append*, but prepends the environment variable instead.
 ### ephemerals
 *Dict-like object*
 
-    if intersects(ephemerals.get("foo.cli", "1"), "1"):
-        env.PATH.append("{rot}/bin")
+    if intersects(ephemerals.get("foo.cli", default="foo.cli-1"), "1"):
+        env.PATH.append("{root}/bin")
 
 A dict representing the list of ephemerals in the resolved environment. Each item is a
 string (the full request, eg `.foo.cli-1`), keyed by the ephemeral package name. Note
 that you do **not** include the leading `.` when getting items from the `ephemerals`
 object.
 
+Noted that the default value of `ephemerals.get` method should be a full request string so the `intersects`
+can work as expect.
+
+Alternatively, you can use `ephemerals.get_range` to get `VersionRange` object, which may provide
+a bit intuitive way to interact with `intersects`.
+
+    if intersects(ephemerals.get_range("foo.cli", "1"), "1"):
+        env.PATH.append("{root}/bin")
+
 ### error
 *Function*
 
@@ -433,6 +442,20 @@ This request would yield the following *request* object:
         "corelib": "!corelib-1.4.4"
     }
 
+To use with `intersects`:
+
+    if intersects(request.get("foo.cli", default="foo.cli-1"), "1"):
+        env.PATH.append("{root}/bin")
+
+Noted that the default value of `request.get` method should be a full request string so the `intersects`
+can work as expect.
+
+Alternatively, you can use `request.get_range` to get `VersionRange` object, which may provide
+a bit intuitive way to interact with `intersects`.
+
+    if intersects(request.get_range("foo.cli", "1"), "1"):
+        env.PATH.append("{root}/bin")
+
 > [[media/icons/info.png]] If multiple requests are present that refer to the same package, the
 request is combined ahead of time. In other words, if requests *foo-4+* and *foo-<6* were both
 present, the single request *foo-4+<6* would be present in the *request* object.


Test patch:
diff --git a/src/rez/tests/test_rex.py b/src/rez/tests/test_rex.py
index 814e701d..b2712da4 100644
--- a/src/rez/tests/test_rex.py
+++ b/src/rez/tests/test_rex.py
@@ -4,13 +4,17 @@ test the rex command generator API
 from rez.rex import RexExecutor, Python, Setenv, Appendenv, Prependenv, Info, \
     Comment, Alias, Command, Source, Error, Shebang, Unsetenv, expandable, \
     literal
-from rez.rex_bindings import VersionBinding
+from rez.rex_bindings import VersionBinding, VariantsBinding, \
+    RequirementsBinding, EphemeralsBinding, intersects
 from rez.exceptions import RexError, RexUndefinedVariableError
 from rez.config import config
 import unittest
 from rez.vendor.version.version import Version
+from rez.vendor.version.requirement import Requirement
 from rez.tests.util import TestBase
 from rez.utils.backcompat import convert_old_commands
+from rez.package_repository import package_repository_manager
+from rez.packages import iter_package_families
 import inspect
 import textwrap
 import os
@@ -407,6 +411,107 @@ class TestRex(TestBase):
                                             annotate=False)
         self.assertEqual(rez_commands, expected)
 
+    def test_intersects_resolve(self):
+        """Test intersects with resolve object"""
+        resolved_pkg_data = {
+            "foo": {"1": {"name": "foo", "version": "1"}},
+            "maya": {"2020.1": {"name": "maya", "version": "2020.1"}},
+        }
+        mem_path = "memory@%s" % hex(id(resolved_pkg_data))
+        resolved_repo = package_repository_manager.get_repository(mem_path)
+        resolved_repo.data = resolved_pkg_data
+        resolved_packages = [
+            variant
+            for family in iter_package_families(paths=[mem_path])
+            for package in family.iter_packages()
+            for variant in package.iter_variants()
+        ]
+        resolve = VariantsBinding(resolved_packages)
+        self.assertTrue(intersects(resolve.foo, "1"))
+        self.assertFalse(intersects(resolve.foo, "0"))
+        self.assertTrue(intersects(resolve.maya, "2019+"))
+        self.assertFalse(intersects(resolve.maya, "<=2019"))
+
+    def test_intersects_request(self):
+        """Test intersects with request object"""
+        # request.get
+        request = RequirementsBinding([Requirement("foo.bar-1")])
+        bar_on = intersects(request.get("foo.bar", "0"), "1")
+        self.assertTrue(bar_on)
+
+        request = RequirementsBinding([])
+        bar_on = intersects(request.get("foo.bar", "0"), "1")
+        self.assertTrue(bar_on)  # should be False, but for backward compat
+
+        request = RequirementsBinding([])
+        bar_on = intersects(request.get("foo.bar", "foo.bar-0"), "1")
+        self.assertFalse(bar_on)  # workaround, see PR nerdvegas/rez#1030
+
+        # request.get_range
+        request = RequirementsBinding([Requirement("foo.bar-1")])
+        bar_on = intersects(request.get_range("foo.bar", "0"), "1")
+        self.assertTrue(bar_on)
+
+        request = RequirementsBinding([])
+        bar_on = intersects(request.get_range("foo.bar", "0"), "1")
+        self.assertFalse(bar_on)
+
+        request = RequirementsBinding([])
+        foo = intersects(request.get_range("foo", "==1.2.3"), "1.2")
+        self.assertTrue(foo)
+
+        request = RequirementsBinding([])
+        foo = intersects(request.get_range("foo", "==1.2.3"), "1.4")
+        self.assertFalse(foo)
+
+        request = RequirementsBinding([Requirement("foo-1.4.5")])
+        foo = intersects(request.get_range("foo", "==1.2.3"), "1.4")
+        self.assertTrue(foo)
+
+    def test_intersects_ephemerals(self):
+        """Test intersects with ephemerals object"""
+        # ephemerals.get
+        ephemerals = EphemeralsBinding([Requirement(".foo.bar-1")])
+        bar_on = intersects(ephemerals.get("foo.bar", "0"), "1")
+        self.assertTrue(bar_on)
+
+        ephemerals = EphemeralsBinding([])
+        bar_on = intersects(ephemerals.get("foo.bar", "0"), "1")
+        self.assertTrue(bar_on)  # should be False, but for backward compat
+
+        ephemerals = EphemeralsBinding([])
+        bar_on = intersects(ephemerals.get("foo.bar", "foo.bar-0"), "1")
+        self.assertFalse(bar_on)  # workaround, see PR nerdvegas/rez#1030
+
+        ephemerals = EphemeralsBinding([])
+        self.assertRaises(RuntimeError,  # no default
+                          intersects, ephemerals.get("foo.bar"), "0")
+
+        # ephemerals.get_range
+        ephemerals = EphemeralsBinding([Requirement(".foo.bar-1")])
+        bar_on = intersects(ephemerals.get_range("foo.bar", "0"), "1")
+        self.assertTrue(bar_on)
+
+        ephemerals = EphemeralsBinding([])
+        bar_on = intersects(ephemerals.get_range("foo.bar", "0"), "1")
+        self.assertFalse(bar_on)
+
+        ephemerals = EphemeralsBinding([])
+        foo = intersects(ephemerals.get_range("foo", "==1.2.3"), "1.2")
+        self.assertTrue(foo)
+
+        ephemerals = EphemeralsBinding([])
+        foo = intersects(ephemerals.get_range("foo", "==1.2.3"), "1.4")
+        self.assertFalse(foo)
+
+        ephemerals = EphemeralsBinding([Requirement(".foo-1.4.5")])
+        foo = intersects(ephemerals.get_range("foo", "==1.2.3"), "1.4")
+        self.assertTrue(foo)
+
+        ephemerals = EphemeralsBinding([])
+        self.assertRaises(RuntimeError,  # no default
+                          intersects, ephemerals.get_range("foo.bar"), "0")
+
 
 if __name__ == '__main__':
     unittest.main()