From 90e91b9dbf6ae65937c928d1c564311b59a585d1 Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Wed, 18 Oct 2023 16:40:58 -0700 Subject: [PATCH 01/12] Add support for `error_on_missing_variant_packages` which treats a missing package in a variant as a failed variant rather than an immediate error Signed-off-by: Dhruv Govil --- src/rez/config.py | 1 + src/rez/rezconfig.py | 5 +++++ src/rez/solver.py | 9 ++++++--- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/rez/config.py b/src/rez/config.py index 4309e2036..1f9d56ea5 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -400,6 +400,7 @@ def _parse_env_var(self, value): "alias_back": OptionalStr, "package_preprocess_function": OptionalStrOrFunction, "package_preprocess_mode": PreprocessMode_, + "error_on_missing_variant_packages": Bool, "context_tracking_host": OptionalStr, "variant_shortlinks_dirname": OptionalStr, "build_thread_count": BuildThreadCount_, diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 81f7347f1..c28fa19ff 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -657,6 +657,11 @@ # - "override": Package's preprocess function completely overrides the global preprocess. package_preprocess_mode = "override" +# Defines whether a resolve should immediately fail if any variants have a package that can't be found. +# It is enabled by default. +# If disabled, it will try other variants before giving up. +# This can be useful if you have variants that aren't available to all users. +error_on_missing_variant_packages = True ############################################################################### # Context Tracking diff --git a/src/rez/solver.py b/src/rez/solver.py index 2ec0f35fd..4a4420eed 100644 --- a/src/rez/solver.py +++ b/src/rez/solver.py @@ -1383,10 +1383,13 @@ def _create_phase(status=None): # Raise with more info when match found searched = "; ".join(self.solver.package_paths) requested = ", ".join(requesters) + + fail_message = "package family not found: {}, was required by: {} (searched: {})".format(req.name, requested, searched) + if not config.error_on_missing_variant_packages: + print(fail_message, file=sys.stderr) + return _create_phase(SolverStatus.failed) raise PackageFamilyNotFoundError( - "package family not found: %s, " - "was required by: %s (searched: %s)" - % (req.name, requested, searched)) + fail_message) scopes.append(scope) if self.pr: From 958cb14ba06ceac5de88b767e844916485627df2 Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Fri, 20 Oct 2023 14:12:10 -0700 Subject: [PATCH 02/12] Handle error case for missing failure reason Signed-off-by: Dhruv Govil --- src/rez/solver.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rez/solver.py b/src/rez/solver.py index 4a4420eed..9a7c2264b 100644 --- a/src/rez/solver.py +++ b/src/rez/solver.py @@ -2401,7 +2401,10 @@ def _get_failed_phase(self, index=None): except IndexError: raise IndexError("failure index out of range") - fail_description = phase.failure_reason.description() + if phase.failure_reason is None: + fail_description = "Solver failed with unknown reason." + else: + fail_description = phase.failure_reason.description() if prepend_abort_reason and self.abort_reason: fail_description = "%s:\n%s" % (self.abort_reason, fail_description) From c3dc3c575bba78aeb163e8da6ec78d0fc378f60c Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Fri, 20 Oct 2023 14:20:22 -0700 Subject: [PATCH 03/12] Add tests for missing_variant_package Signed-off-by: Dhruv Govil --- .../packages/missing_variant_package/1/package.py | 10 ++++++++++ src/rez/tests/test_solver.py | 8 ++++++++ 2 files changed, 18 insertions(+) create mode 100644 src/rez/data/tests/solver/packages/missing_variant_package/1/package.py diff --git a/src/rez/data/tests/solver/packages/missing_variant_package/1/package.py b/src/rez/data/tests/solver/packages/missing_variant_package/1/package.py new file mode 100644 index 000000000..20a06feef --- /dev/null +++ b/src/rez/data/tests/solver/packages/missing_variant_package/1/package.py @@ -0,0 +1,10 @@ +name = "missing_variant_package" +version = "1" + +def commands(): + pass + +variants = [ + ["noexist"], + ["nada"] +] \ No newline at end of file diff --git a/src/rez/tests/test_solver.py b/src/rez/tests/test_solver.py index 6422bcf07..1c229911c 100644 --- a/src/rez/tests/test_solver.py +++ b/src/rez/tests/test_solver.py @@ -7,6 +7,7 @@ """ from __future__ import print_function +import rez.exceptions from rez.vendor.version.requirement import Requirement from rez.solver import Solver, Cycle, SolverStatus from rez.config import config @@ -248,6 +249,13 @@ def test_11_variant_splitting(self): "test_variant_split_mid2-2.0[0]", "test_variant_split_start-1.0[1]"]) + def test_12_missing_variant_package(self): + config.override("error_on_missing_variant_packages", True) + with self.assertRaises(rez.exceptions.PackageFamilyNotFoundError): + self._solve(["missing_variant_package"], []) + + config.override("error_on_missing_variant_packages", False) + self._solve(["missing_variant_package"], ["nada[]", "missing_variant_package-1[1]"]) if __name__ == '__main__': unittest.main() From de55d7fc60ac1e85f9ba56aa7be34b5f64e3c010 Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Sat, 21 Oct 2023 09:32:57 -0700 Subject: [PATCH 04/12] Fix failing tests caused by addition of new test package Signed-off-by: Dhruv Govil --- src/rez/tests/test_completion.py | 2 +- src/rez/tests/test_packages.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rez/tests/test_completion.py b/src/rez/tests/test_completion.py index 87be442fd..f10a976fa 100644 --- a/src/rez/tests/test_completion.py +++ b/src/rez/tests/test_completion.py @@ -52,7 +52,7 @@ def _eq(prefix, expected_completions): _eq("", ["bahish", "nada", "nopy", "pybah", "pydad", "pyfoo", "pymum", "pyodd", "pyson", "pysplit", "python", "pyvariants", "test_variant_split_start", "test_variant_split_mid1", - "test_variant_split_mid2", "test_variant_split_end"]) + "test_variant_split_mid2", "test_variant_split_end", "missing_variant_package"]) _eq("py", ["pybah", "pydad", "pyfoo", "pymum", "pyodd", "pyson", "pysplit", "python", "pyvariants"]) _eq("pys", ["pyson", "pysplit"]) diff --git a/src/rez/tests/test_packages.py b/src/rez/tests/test_packages.py index b94a0dde9..40a6dacd4 100644 --- a/src/rez/tests/test_packages.py +++ b/src/rez/tests/test_packages.py @@ -57,7 +57,8 @@ 'late_binding-1.0', 'timestamped-1.0.5', 'timestamped-1.0.6', 'timestamped-1.1.0', 'timestamped-1.1.1', 'timestamped-1.2.0', 'timestamped-2.0.0', 'timestamped-2.1.0', 'timestamped-2.1.5', - 'multi-1.0', 'multi-1.1', 'multi-1.2', 'multi-2.0' + 'multi-1.0', 'multi-1.1', 'multi-1.2', 'multi-2.0', + 'missing_variant_package-1' ]) From 3aa2b5c8a035ed5cc6351719e14479f5bf71f42b Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Sat, 21 Oct 2023 15:22:33 -0700 Subject: [PATCH 05/12] Address notes, fix tests Signed-off-by: Dhruv Govil --- src/rez/config.py | 2 +- .../1/package.py | 2 +- src/rez/rezconfig.py | 4 ++-- src/rez/solver.py | 2 +- src/rez/tests/test_completion.py | 2 +- src/rez/tests/test_packages.py | 2 +- src/rez/tests/test_solver.py | 10 +++++----- 7 files changed, 12 insertions(+), 12 deletions(-) rename src/rez/data/tests/solver/packages/{missing_variant_package => missing_variant_requires}/1/package.py (71%) diff --git a/src/rez/config.py b/src/rez/config.py index 1f9d56ea5..747b75cc3 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -400,7 +400,7 @@ def _parse_env_var(self, value): "alias_back": OptionalStr, "package_preprocess_function": OptionalStrOrFunction, "package_preprocess_mode": PreprocessMode_, - "error_on_missing_variant_packages": Bool, + "error_on_missing_variant_requires": Bool, "context_tracking_host": OptionalStr, "variant_shortlinks_dirname": OptionalStr, "build_thread_count": BuildThreadCount_, diff --git a/src/rez/data/tests/solver/packages/missing_variant_package/1/package.py b/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py similarity index 71% rename from src/rez/data/tests/solver/packages/missing_variant_package/1/package.py rename to src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py index 20a06feef..5c505d20d 100644 --- a/src/rez/data/tests/solver/packages/missing_variant_package/1/package.py +++ b/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py @@ -1,4 +1,4 @@ -name = "missing_variant_package" +name = "missing_variant_requires" version = "1" def commands(): diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index c28fa19ff..8c61661ac 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -657,11 +657,11 @@ # - "override": Package's preprocess function completely overrides the global preprocess. package_preprocess_mode = "override" -# Defines whether a resolve should immediately fail if any variants have a package that can't be found. +# Defines whether a resolve should immediately fail if any variants have a required package that can't be found. # It is enabled by default. # If disabled, it will try other variants before giving up. # This can be useful if you have variants that aren't available to all users. -error_on_missing_variant_packages = True +error_on_missing_variant_requires = True ############################################################################### # Context Tracking diff --git a/src/rez/solver.py b/src/rez/solver.py index 9a7c2264b..56830913e 100644 --- a/src/rez/solver.py +++ b/src/rez/solver.py @@ -1385,7 +1385,7 @@ def _create_phase(status=None): requested = ", ".join(requesters) fail_message = "package family not found: {}, was required by: {} (searched: {})".format(req.name, requested, searched) - if not config.error_on_missing_variant_packages: + if not config.error_on_missing_variant_requires: print(fail_message, file=sys.stderr) return _create_phase(SolverStatus.failed) raise PackageFamilyNotFoundError( diff --git a/src/rez/tests/test_completion.py b/src/rez/tests/test_completion.py index f10a976fa..ef8c390c6 100644 --- a/src/rez/tests/test_completion.py +++ b/src/rez/tests/test_completion.py @@ -52,7 +52,7 @@ def _eq(prefix, expected_completions): _eq("", ["bahish", "nada", "nopy", "pybah", "pydad", "pyfoo", "pymum", "pyodd", "pyson", "pysplit", "python", "pyvariants", "test_variant_split_start", "test_variant_split_mid1", - "test_variant_split_mid2", "test_variant_split_end", "missing_variant_package"]) + "test_variant_split_mid2", "test_variant_split_end", "missing_variant_requires"]) _eq("py", ["pybah", "pydad", "pyfoo", "pymum", "pyodd", "pyson", "pysplit", "python", "pyvariants"]) _eq("pys", ["pyson", "pysplit"]) diff --git a/src/rez/tests/test_packages.py b/src/rez/tests/test_packages.py index 40a6dacd4..11dc60c1f 100644 --- a/src/rez/tests/test_packages.py +++ b/src/rez/tests/test_packages.py @@ -58,7 +58,7 @@ 'timestamped-1.0.5', 'timestamped-1.0.6', 'timestamped-1.1.0', 'timestamped-1.1.1', 'timestamped-1.2.0', 'timestamped-2.0.0', 'timestamped-2.1.0', 'timestamped-2.1.5', 'multi-1.0', 'multi-1.1', 'multi-1.2', 'multi-2.0', - 'missing_variant_package-1' + 'missing_variant_requires-1' ]) diff --git a/src/rez/tests/test_solver.py b/src/rez/tests/test_solver.py index 1c229911c..4bfcf5db3 100644 --- a/src/rez/tests/test_solver.py +++ b/src/rez/tests/test_solver.py @@ -249,13 +249,13 @@ def test_11_variant_splitting(self): "test_variant_split_mid2-2.0[0]", "test_variant_split_start-1.0[1]"]) - def test_12_missing_variant_package(self): - config.override("error_on_missing_variant_packages", True) + def test_12_missing_variant_requires(self): + config.override("error_on_missing_variant_requires", True) with self.assertRaises(rez.exceptions.PackageFamilyNotFoundError): - self._solve(["missing_variant_package"], []) + self._solve(["missing_variant_requires"], []) - config.override("error_on_missing_variant_packages", False) - self._solve(["missing_variant_package"], ["nada[]", "missing_variant_package-1[1]"]) + config.override("error_on_missing_variant_requires", False) + self._solve(["missing_variant_requires"], ["nada[]", "missing_variant_requires-1[1]"]) if __name__ == '__main__': unittest.main() From 21a87e7cab0a9221b54fce392a78af044c6350ba Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Sat, 21 Oct 2023 15:26:56 -0700 Subject: [PATCH 06/12] Overshot section Signed-off-by: Dhruv Govil --- src/rez/rezconfig.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 8c61661ac..26fe2eeab 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -499,6 +499,11 @@ # this value is False. allow_unversioned_packages = True +# Defines whether a resolve should immediately fail if any variants have a required package that can't be found. +# It is enabled by default. +# If disabled, it will try other variants before giving up. +# This can be useful if you have variants that aren't available to all users. +error_on_missing_variant_requires = True ############################################################################### # Environment Resolution @@ -657,12 +662,6 @@ # - "override": Package's preprocess function completely overrides the global preprocess. package_preprocess_mode = "override" -# Defines whether a resolve should immediately fail if any variants have a required package that can't be found. -# It is enabled by default. -# If disabled, it will try other variants before giving up. -# This can be useful if you have variants that aren't available to all users. -error_on_missing_variant_requires = True - ############################################################################### # Context Tracking ############################################################################### From 5650b129314586e6fbdea5ea4299da45cb4011bb Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Sun, 22 Oct 2023 09:21:20 -0700 Subject: [PATCH 07/12] Fix missing line lint Signed-off-by: Dhruv Govil --- src/rez/tests/test_solver.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rez/tests/test_solver.py b/src/rez/tests/test_solver.py index 4bfcf5db3..82ee932d1 100644 --- a/src/rez/tests/test_solver.py +++ b/src/rez/tests/test_solver.py @@ -215,6 +215,7 @@ def test_07(self): def test_08(self): """Cyclic failures.""" + def _test(*pkgs): s = self._fail(*pkgs) self.assertTrue(isinstance(s.failure_reason(), Cycle)) @@ -257,5 +258,6 @@ def test_12_missing_variant_requires(self): config.override("error_on_missing_variant_requires", False) self._solve(["missing_variant_requires"], ["nada[]", "missing_variant_requires-1[1]"]) + if __name__ == '__main__': unittest.main() From 5621e93d10377fd18d0ded12b546087421403f06 Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Sun, 22 Oct 2023 09:22:44 -0700 Subject: [PATCH 08/12] Add copyright notice to package def Signed-off-by: Dhruv Govil --- .../solver/packages/missing_variant_requires/1/package.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py b/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py index 5c505d20d..8ddd5b1eb 100644 --- a/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py +++ b/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py @@ -1,3 +1,6 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Contributors to the Rez Project + name = "missing_variant_requires" version = "1" From 2313693fa805ca3492d29fe3fd58e06b9952cc0b Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Sun, 22 Oct 2023 09:26:51 -0700 Subject: [PATCH 09/12] Fix solver.py lint Signed-off-by: Dhruv Govil --- src/rez/solver.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rez/solver.py b/src/rez/solver.py index 56830913e..228619199 100644 --- a/src/rez/solver.py +++ b/src/rez/solver.py @@ -1384,7 +1384,8 @@ def _create_phase(status=None): searched = "; ".join(self.solver.package_paths) requested = ", ".join(requesters) - fail_message = "package family not found: {}, was required by: {} (searched: {})".format(req.name, requested, searched) + fail_message = ("package family not found: {}, was required by: {} (searched: {})" + .format(req.name, requested, searched)) if not config.error_on_missing_variant_requires: print(fail_message, file=sys.stderr) return _create_phase(SolverStatus.failed) From 999526e466b5bab756639eea546cec70e51dc16c Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 22 Oct 2023 13:21:20 -0400 Subject: [PATCH 10/12] Fix copyright correctly Signed-off-by: Jean-Christophe Morin --- .../solver/packages/missing_variant_requires/1/package.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py b/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py index 8ddd5b1eb..6cf364e53 100644 --- a/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py +++ b/src/rez/data/tests/solver/packages/missing_variant_requires/1/package.py @@ -1,6 +1,3 @@ -# SPDX-License-Identifier: Apache-2.0 -# Copyright Contributors to the Rez Project - name = "missing_variant_requires" version = "1" @@ -10,4 +7,4 @@ def commands(): variants = [ ["noexist"], ["nada"] -] \ No newline at end of file +] From 76984594ed8e06f7cf6f62e6f71f9697df3e6912 Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Thu, 16 Nov 2023 13:37:28 -0800 Subject: [PATCH 11/12] Updated documentation to make mention of memcached caveats Signed-off-by: Dhruv Govil --- src/rez/rezconfig.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 26fe2eeab..96fbfea66 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -500,9 +500,15 @@ allow_unversioned_packages = True # Defines whether a resolve should immediately fail if any variants have a required package that can't be found. -# It is enabled by default. +# This can be useful to disable if you have packages that aren't available to all users. +# It is enabled by default. If a variant has requires that cannot be found , it will error immediately rather than +# trying the other variants. # If disabled, it will try other variants before giving up. -# This can be useful if you have variants that aren't available to all users. +# +# .. warning:: +# Memcached isn't tested with scenarios where you expect users to have access to different sets of packages. +# It expects that every user can access the same set of packages, which may cause incorrect resolves +# when this option is disabled. error_on_missing_variant_requires = True ############################################################################### From 678cdb047086502f4a161027e2c7e31de3f4220e Mon Sep 17 00:00:00 2001 From: Dhruv Govil Date: Thu, 16 Nov 2023 14:15:11 -0800 Subject: [PATCH 12/12] Adding TODO as requested Signed-off-by: Dhruv Govil --- src/rez/solver.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rez/solver.py b/src/rez/solver.py index 228619199..d91d9890f 100644 --- a/src/rez/solver.py +++ b/src/rez/solver.py @@ -1386,6 +1386,8 @@ def _create_phase(status=None): fail_message = ("package family not found: {}, was required by: {} (searched: {})" .format(req.name, requested, searched)) + # TODO: Test with memcached to see if this can cause any conflicting behaviour + # where a package may show as missing/available inadvertently if not config.error_on_missing_variant_requires: print(fail_message, file=sys.stderr) return _create_phase(SolverStatus.failed)