From 0d1e29cb5324891d0f833dc9ecddeb0d55e134df Mon Sep 17 00:00:00 2001 From: ttrently <41705925+ttrently@users.noreply.github.com> Date: Wed, 22 Feb 2023 15:56:04 -0800 Subject: [PATCH 01/20] Add `package_cache_async` flag. Adds a `package_cachy_async` flag which allows users to run caching synchronously (blocking) or asynchronously from the config. Signed-off-by: ttrently <41705925+ttrently@users.noreply.github.com> --- src/rez/config.py | 1 + src/rez/package_cache.py | 10 +++++++--- src/rez/resolved_context.py | 5 ++++- src/rez/rezconfig.py | 3 +++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/rez/config.py b/src/rez/config.py index e8542d1ed..c727050a4 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -453,6 +453,7 @@ def _parse_env_var(self, value): "package_cache_during_build": Bool, "package_cache_local": Bool, "package_cache_same_device": Bool, + "package_cache_async": Bool, "color_enabled": ForceOrBool, "resolve_caching": Bool, "cache_package_files": Bool, diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index e17b2c782..5e9f00e26 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -366,7 +366,7 @@ def remove_variant(self, variant): return self.VARIANT_REMOVED - def add_variants_async(self, variants): + def add_variants_async(self, variants, _async=False): """Update the package cache by adding some or all of the given variants. This method is called when a context is created or sourced. Variants @@ -460,8 +460,12 @@ def add_variants_async(self, variants): else: out_target = devnull - subprocess.Popen( - [exe, "--daemon", self.path], + func = subprocess.Popen + if not _async: + func = subprocess.call + + func( + args, stdout=out_target, stderr=out_target, **kwargs diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index bbf68fef0..0b8589c50 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -1844,7 +1844,10 @@ def _update_package_cache(self): pkgcache = self._get_package_cache() if pkgcache: - pkgcache.add_variants_async(self.resolved_packages) + pkgcache.add_variants_async( + self.resolved_packages, + config.package_cache_async + ) @classmethod def _init_context_tracking_payload_base(cls): diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index ca1f14b15..38ea03b27 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -278,6 +278,9 @@ # Enable package caching during a package build. package_cache_during_build = False +# Enable package caching to run asynchronously during a resolve. +package_cache_async = True + # Allow caching of local packages. You would only want to set this True for # testing purposes. package_cache_local = False From d6ede1b44a2dcc37e040fd57fe9f543182e2301c Mon Sep 17 00:00:00 2001 From: ttrently <41705925+ttrently@users.noreply.github.com> Date: Wed, 22 Feb 2023 16:04:42 -0800 Subject: [PATCH 02/20] Change _async to default True Change _async to default True instead of False to mimic previous behavior. Signed-off-by: ttrently <41705925+ttrently@users.noreply.github.com> --- src/rez/package_cache.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 5e9f00e26..821cb0677 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -366,7 +366,7 @@ def remove_variant(self, variant): return self.VARIANT_REMOVED - def add_variants_async(self, variants, _async=False): + def add_variants_async(self, variants, _async=True): """Update the package cache by adding some or all of the given variants. This method is called when a context is created or sourced. Variants @@ -461,6 +461,8 @@ def add_variants_async(self, variants, _async=False): out_target = devnull func = subprocess.Popen + + # use subprocess.call if we are not running async since it is blocking if not _async: func = subprocess.call From 98bd61f08b589f73bf3cfb00f897f7cbee6c6c6c Mon Sep 17 00:00:00 2001 From: ttrently <41705925+ttrently@users.noreply.github.com> Date: Wed, 22 Feb 2023 16:27:16 -0800 Subject: [PATCH 03/20] Rename add_variants_async to add_variants Renamed `add_variants_async` to `add_variants` as this method can now run with an `_async` flag. Signed-off-by: ttrently <41705925+ttrently@users.noreply.github.com> --- src/rez/package_cache.py | 8 ++++---- src/rez/resolved_context.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 821cb0677..95e6cf808 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -366,21 +366,21 @@ def remove_variant(self, variant): return self.VARIANT_REMOVED - def add_variants_async(self, variants, _async=True): + def add_variants(self, variants, _async=True): """Update the package cache by adding some or all of the given variants. This method is called when a context is created or sourced. Variants are then added to the cache in a separate process. """ - # A prod install is necessary because add_variants_async works by + # A prod install is necessary because add_variants works by # starting a rez-pkg-cache proc, and this can only be done reliably in # a prod install. On non-windows we could fork instead, but there would # remain no good solution on windows. # if not system.is_production_rez_install: raise PackageCacheError( - "PackageCache.add_variants_async is only supported in a " + "PackageCache.add_variants is only supported in a " "production rez installation." ) @@ -462,7 +462,7 @@ def add_variants_async(self, variants, _async=True): func = subprocess.Popen - # use subprocess.call if we are not running async since it is blocking + # use subprocess.call blocks where subprocess.Popen doesn't if not _async: func = subprocess.call diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index 0b8589c50..b7a82dcc8 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -1838,13 +1838,13 @@ def _update_package_cache(self): not self.success: return - # see PackageCache.add_variants_async + # see PackageCache.add_variants if not system.is_production_rez_install: return pkgcache = self._get_package_cache() if pkgcache: - pkgcache.add_variants_async( + pkgcache.add_variants( self.resolved_packages, config.package_cache_async ) From 9463a3bc28888aa494dd13a6b2ca1359597be8be Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Thu, 7 Mar 2024 11:56:53 +1100 Subject: [PATCH 04/20] added test cases and rez env flag Signed-off-by: Ben Andersen --- src/rez/cli/env.py | 7 +++- src/rez/package_cache.py | 12 +++---- src/rez/resolved_context.py | 12 +++++-- src/rez/rezconfig.py | 7 ++-- src/rez/tests/test_package_cache.py | 56 ++++++++++++++++++++++++++--- 5 files changed, 75 insertions(+), 19 deletions(-) diff --git a/src/rez/cli/env.py b/src/rez/cli/env.py index 20cf68edf..a917830db 100644 --- a/src/rez/cli/env.py +++ b/src/rez/cli/env.py @@ -116,6 +116,10 @@ def setup_parser(parser, completions=False): parser.add_argument( "--no-pkg-cache", action="store_true", help="Disable package caching") + parser.add_argument( + "--pkg-cache-sync", action="store_true", + help="Disable asynchronous package caching. " + "Process will block until packages are cached.") parser.add_argument( "--pre-command", type=str, help=SUPPRESS) PKG_action = parser.add_argument( @@ -212,7 +216,8 @@ def command(opts, parser, extra_arg_groups=None): caching=(not opts.no_cache), suppress_passive=opts.no_passive, print_stats=opts.stats, - package_caching=(not opts.no_pkg_cache) + package_caching=(not opts.no_pkg_cache), + package_cache_async=(not opts.pkg_cache_sync), ) success = (context.status == ResolverStatus.solved) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 95e6cf808..8e85ca7a1 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -366,7 +366,7 @@ def remove_variant(self, variant): return self.VARIANT_REMOVED - def add_variants(self, variants, _async=True): + def add_variants(self, variants, package_cache_async=True): """Update the package cache by adding some or all of the given variants. This method is called when a context is created or sourced. Variants @@ -460,18 +460,14 @@ def add_variants(self, variants, _async=True): else: out_target = devnull - func = subprocess.Popen - - # use subprocess.call blocks where subprocess.Popen doesn't - if not _async: - func = subprocess.call - - func( + process = subprocess.Popen( args, stdout=out_target, stderr=out_target, **kwargs ) + if not package_cache_async: + process.wait() except Exception as e: print_warning( diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index b7a82dcc8..c9814378a 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -167,7 +167,7 @@ def __init__(self, package_requests, verbosity=0, timestamp=None, package_filter=None, package_orderers=None, max_fails=-1, add_implicit_packages=True, time_limit=-1, callback=None, package_load_callback=None, buf=None, suppress_passive=False, - print_stats=False, package_caching=None): + print_stats=False, package_caching=None, package_cache_async=None): """Perform a package resolve, and store the result. Args: @@ -205,6 +205,8 @@ def __init__(self, package_requests, verbosity=0, timestamp=None, package_caching (bool|None): If True, apply package caching settings as per the config. If None, enable as determined by config setting :data:`package_cache_during_build`. + package_cache_async (Optional[bool]): If True, cache packages asynchronously. + If None, use the config setting :data:`package_cache_async` """ self.load_path = None @@ -249,6 +251,10 @@ def __init__(self, package_requests, verbosity=0, timestamp=None, self.package_caching = package_caching + if package_cache_async is None: + package_cache_async = config.package_cache_async + self.package_cache_async = package_cache_async + # patch settings self.default_patch_lock = PatchLock.no_lock self.patch_locks = {} @@ -1846,8 +1852,8 @@ def _update_package_cache(self): if pkgcache: pkgcache.add_variants( self.resolved_packages, - config.package_cache_async - ) + self.package_cache_async, + ) @classmethod def _init_context_tracking_payload_base(cls): diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 38ea03b27..59151c0ad 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -279,7 +279,8 @@ package_cache_during_build = False # Enable package caching to run asynchronously during a resolve. -package_cache_async = True +# If this is false, a resolve will block until all packages are cached. +package_cache_async = True # Allow caching of local packages. You would only want to set this True for # testing purposes. @@ -316,7 +317,7 @@ # This is useful as Platform.os might show different # values depending on the availability of ``lsb-release`` on the system. # The map supports regular expression, e.g. to keep versions. -# +# # .. note:: # The following examples are not necessarily recommendations. # @@ -1122,7 +1123,7 @@ # Enables/disables colorization globally. # -# .. warning:: +# .. warning:: # Turned off for Windows currently as there seems to be a problem with the colorama module. # # May also set to the string ``force``, which will make rez output color styling diff --git a/src/rez/tests/test_package_cache.py b/src/rez/tests/test_package_cache.py index c340dbfe7..8a5989c09 100644 --- a/src/rez/tests/test_package_cache.py +++ b/src/rez/tests/test_package_cache.py @@ -134,24 +134,72 @@ def test_caching_on_resolve(self): # Creating the context will asynchronously add variants to the cache # in a separate proc. - # + # NOTE: pyfoo will not cache, because its repo is set to non-caching (see above) c = ResolvedContext([ "timestamped-1.2.0", - "pyfoo-3.1.0" # won't cache, see earlier test + "pyfoo-3.1.0" ]) variant = c.get_resolved_package("timestamped") # Retry 50 times with 0.1 sec interval, 5 secs is more than enough for # the very small variant to be copied to cache. - # cached_root = None + resolve_not_always_cached = False for _ in range(50): - time.sleep(0.1) cached_root = pkgcache.get_cached_root(variant) if cached_root: break + resolve_not_always_cached = True + time.sleep(0.1) + + self.assertNotEqual(cached_root, None) + + # Test that the package is not immediately cached, since it is asynchronous + # WARNING: This is dangerous since it does open the test to a race condition and + # will fail if the cache happens faster than the resolve. + self.assertNotEqual(resolve_not_always_cached, False) + + expected_payload_file = os.path.join(cached_root, "stuff.txt") + self.assertTrue(os.path.exists(expected_payload_file)) + + # check that refs to root point to cache location in rex code + for ref in ("resolve.timestamped.root", "'{resolve.timestamped.root}'"): + proc = c.execute_rex_code( + code="info(%s)" % ref, + stdout=subprocess.PIPE, + universal_newlines=True + ) + + out, _ = proc.communicate() + root = out.strip() + + self.assertEqual( + root, cached_root, + "Reference %r should resolve to %s, but resolves to %s" + % (ref, cached_root, root) + ) + + @install_dependent() + def test_caching_on_resolve_synchronous(self): + """Test that cache is updated as expected on + resolved env using syncrhonous package caching.""" + pkgcache = self._pkgcache() + + with restore_os_environ(): + # set config settings into env so rez-pkg-cache proc sees them + os.environ.update(self.get_settings_env()) + + # Creating the context will synchronously add variants to the cache + c = ResolvedContext( + ["timestamped-1.2.0", "pyfoo-3.1.0",], + package_cache_async=False, + ) + + variant = c.get_resolved_package("timestamped") + # The first time we try to access it will be cached, because the cache is blocking + cached_root = pkgcache.get_cached_root(variant) self.assertNotEqual(cached_root, None) expected_payload_file = os.path.join(cached_root, "stuff.txt") From 5a55f516ecaba3b6cc678d583aeb69f1b45f9ac4 Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Thu, 7 Mar 2024 12:39:06 +1100 Subject: [PATCH 05/20] flake8 fix Signed-off-by: Ben Andersen --- src/rez/tests/test_package_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rez/tests/test_package_cache.py b/src/rez/tests/test_package_cache.py index 8a5989c09..749f12ce5 100644 --- a/src/rez/tests/test_package_cache.py +++ b/src/rez/tests/test_package_cache.py @@ -193,7 +193,7 @@ def test_caching_on_resolve_synchronous(self): # Creating the context will synchronously add variants to the cache c = ResolvedContext( - ["timestamped-1.2.0", "pyfoo-3.1.0",], + ["timestamped-1.2.0", "pyfoo-3.1.0"], package_cache_async=False, ) From 5ea25760d071a87b57e04c3906083fd12b071f77 Mon Sep 17 00:00:00 2001 From: Ben Andersen <156872503+isohedronpipeline@users.noreply.github.com> Date: Fri, 8 Mar 2024 15:11:56 +1100 Subject: [PATCH 06/20] Apply suggestions from code review Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> Signed-off-by: Ben Andersen <156872503+isohedronpipeline@users.noreply.github.com> --- src/rez/resolved_context.py | 2 +- src/rez/rezconfig.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index c9814378a..6018f7327 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -205,7 +205,7 @@ def __init__(self, package_requests, verbosity=0, timestamp=None, package_caching (bool|None): If True, apply package caching settings as per the config. If None, enable as determined by config setting :data:`package_cache_during_build`. - package_cache_async (Optional[bool]): If True, cache packages asynchronously. + package_cache_async (bool|None): If True, cache packages asynchronously. If None, use the config setting :data:`package_cache_async` """ self.load_path = None diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 59151c0ad..8338d3d42 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -278,8 +278,10 @@ # Enable package caching during a package build. package_cache_during_build = False -# Enable package caching to run asynchronously during a resolve. -# If this is false, a resolve will block until all packages are cached. +# Asynchronously cache packages. If this is false, resolves will block until +# all packages are cached. +# +# .. versionadded:: 2.1.0 package_cache_async = True # Allow caching of local packages. You would only want to set this True for From 2251de8dd70bf6ea758acd51e6c37b2eb5a38d54 Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Fri, 8 Mar 2024 15:26:16 +1100 Subject: [PATCH 07/20] Addressed some feedback Signed-off-by: Ben Andersen --- src/rez/cli/env.py | 15 +++++++++++---- src/rez/package_cache.py | 12 +++++++++++- src/rez/resolved_context.py | 1 - src/rez/tests/test_package_cache.py | 6 +++++- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/rez/cli/env.py b/src/rez/cli/env.py index a917830db..081389648 100644 --- a/src/rez/cli/env.py +++ b/src/rez/cli/env.py @@ -117,9 +117,9 @@ def setup_parser(parser, completions=False): "--no-pkg-cache", action="store_true", help="Disable package caching") parser.add_argument( - "--pkg-cache-sync", action="store_true", - help="Disable asynchronous package caching. " - "Process will block until packages are cached.") + "--pkg-cache-mode", choices=["sync", "async"], + help="Optionally disable for force enable asynchronous package caching. " + "If 'sync', the process will block until packages are cached.") parser.add_argument( "--pre-command", type=str, help=SUPPRESS) PKG_action = parser.add_argument( @@ -202,6 +202,13 @@ def command(opts, parser, extra_arg_groups=None): rule = Rule.parse_rule(rule_str) package_filter.add_inclusion(rule) + if opts.pkg_cache_sync == "async": + package_cache_async = True + elif opts.pkg_cache_sync == "sync": + package_cache_async = False + else: + package_cache_async = None + # perform the resolve context = ResolvedContext( package_requests=request, @@ -217,7 +224,7 @@ def command(opts, parser, extra_arg_groups=None): suppress_passive=opts.no_passive, print_stats=opts.stats, package_caching=(not opts.no_pkg_cache), - package_cache_async=(not opts.pkg_cache_sync), + package_cache_async=package_cache_async, ) success = (context.status == ResolverStatus.solved) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 8e85ca7a1..01a0e0abf 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -366,6 +366,16 @@ def remove_variant(self, variant): return self.VARIANT_REMOVED + def add_variants_async(self, variants): + """Update the package cache by adding some or all of the given variants. + + This method is called when a context is created or sourced. Variants + are then added to the cache in a separate process. + + This method is left for backwards compatibility. + """ + return self.add_variants(variants, package_cache_async=True) + def add_variants(self, variants, package_cache_async=True): """Update the package cache by adding some or all of the given variants. @@ -467,7 +477,7 @@ def add_variants(self, variants, package_cache_async=True): **kwargs ) if not package_cache_async: - process.wait() + process.communicate() except Exception as e: print_warning( diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index 6018f7327..bad6be45b 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -248,7 +248,6 @@ def __init__(self, package_requests, verbosity=0, timestamp=None, package_caching = config.package_cache_during_build else: package_caching = True - self.package_caching = package_caching if package_cache_async is None: diff --git a/src/rez/tests/test_package_cache.py b/src/rez/tests/test_package_cache.py index 749f12ce5..01f91ff7d 100644 --- a/src/rez/tests/test_package_cache.py +++ b/src/rez/tests/test_package_cache.py @@ -140,6 +140,9 @@ def test_caching_on_resolve(self): "pyfoo-3.1.0" ]) + # Prove that the resolved context used async mode. + self.assertTrue(c.package_cache_async) + variant = c.get_resolved_package("timestamped") # Retry 50 times with 0.1 sec interval, 5 secs is more than enough for @@ -154,7 +157,8 @@ def test_caching_on_resolve(self): resolve_not_always_cached = True time.sleep(0.1) - self.assertNotEqual(cached_root, None) + self.assertNotEqual(cached_root, None, + msg="Packages were expected to be cached, but were not.") # Test that the package is not immediately cached, since it is asynchronous # WARNING: This is dangerous since it does open the test to a race condition and From b0309eb295942432ba6b40f02bbd488294b4b2d7 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> Date: Fri, 8 Mar 2024 09:44:46 -0500 Subject: [PATCH 08/20] Add deprecated directive for add_variants_async Signed-off-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> --- src/rez/package_cache.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 01a0e0abf..890426626 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -372,7 +372,8 @@ def add_variants_async(self, variants): This method is called when a context is created or sourced. Variants are then added to the cache in a separate process. - This method is left for backwards compatibility. + .. deprecated:: 3.1.0 + Use :method:`add_variants` instead. """ return self.add_variants(variants, package_cache_async=True) From 2bb1ce646cbc92ffc0987e5e0f3dab4912aec165 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> Date: Fri, 8 Mar 2024 09:45:02 -0500 Subject: [PATCH 09/20] Fix versionadded version Signed-off-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> --- src/rez/rezconfig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 8338d3d42..f04688fb7 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -281,7 +281,7 @@ # Asynchronously cache packages. If this is false, resolves will block until # all packages are cached. # -# .. versionadded:: 2.1.0 +# .. versionadded:: 3.1.0 package_cache_async = True # Allow caching of local packages. You would only want to set this True for From b5950d64f8de25cecb4653ea1d8a0a4bf210b29b Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Sat, 9 Mar 2024 08:40:12 +1100 Subject: [PATCH 10/20] Updated rez env argument description for package-cache-mode Signed-off-by: Ben Andersen --- src/rez/cli/env.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/rez/cli/env.py b/src/rez/cli/env.py index 081389648..e55e8fa6e 100644 --- a/src/rez/cli/env.py +++ b/src/rez/cli/env.py @@ -118,8 +118,9 @@ def setup_parser(parser, completions=False): help="Disable package caching") parser.add_argument( "--pkg-cache-mode", choices=["sync", "async"], - help="Optionally disable for force enable asynchronous package caching. " - "If 'sync', the process will block until packages are cached.") + help="If provided, override the rezconfig's package_cache_async key. " + "If 'sync', the process will block until packages are cached. " + "If 'async', the process will not block while packages are cached.") parser.add_argument( "--pre-command", type=str, help=SUPPRESS) PKG_action = parser.add_argument( @@ -202,12 +203,12 @@ def command(opts, parser, extra_arg_groups=None): rule = Rule.parse_rule(rule_str) package_filter.add_inclusion(rule) - if opts.pkg_cache_sync == "async": - package_cache_async = True - elif opts.pkg_cache_sync == "sync": - package_cache_async = False + if opts.pkg_cache_mode == "async": + package_cache_mode = True + elif opts.pkg_cache_mode == "sync": + package_cache_mode = False else: - package_cache_async = None + package_cache_mode = None # perform the resolve context = ResolvedContext( @@ -224,7 +225,7 @@ def command(opts, parser, extra_arg_groups=None): suppress_passive=opts.no_passive, print_stats=opts.stats, package_caching=(not opts.no_pkg_cache), - package_cache_async=package_cache_async, + package_cache_async=package_cache_mode, ) success = (context.status == ResolverStatus.solved) From 461adb1cd0b8728fe27bc369b1134aa74ad3a45a Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Tue, 12 Mar 2024 12:34:36 +1100 Subject: [PATCH 11/20] sync mode no longer subprocesses a daemon mode Reports progress while waiting for another package to finish copying Signed-off-by: Ben Andersen --- src/rez/package_cache.py | 117 ++++++++++++++++++++++++++++++++++----- 1 file changed, 103 insertions(+), 14 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 890426626..3780a73d4 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -69,6 +69,18 @@ class PackageCache(object): VARIANT_PENDING = 5 #: Variant is pending caching VARIANT_REMOVED = 6 #: Variant was deleted + STATUS_DESCRIPTIONS = { + VARIANT_NOT_FOUND: "was not found", + VARIANT_FOUND: "was found", + VARIANT_CREATED: "was created", + VARIANT_COPYING: "payload is still being copied to this cache", + VARIANT_COPY_STALLED: "payload copy has stalled.\nSee " + "https://rez.readthedocs.io/en/stable/caching.html#cleaning-the-cache " + "for more information.", + VARIANT_PENDING: "is pending caching", + VARIANT_REMOVED: "was deleted", + } + _FILELOCK_TIMEOUT = 10 _COPYING_TIME_INC = 0.2 _COPYING_TIME_MAX = 5.0 @@ -116,7 +128,7 @@ def get_cached_root(self, variant): return rootpath - def add_variant(self, variant, force=False): + def add_variant(self, variant, force=False, wait_for_copying=False, logger=None): """Copy a variant's payload into the cache. The following steps are taken to ensure muti-thread/proc safety, and to @@ -147,6 +159,9 @@ def add_variant(self, variant, force=False): variant (Variant): The variant to copy into this cache force (bool): Copy the variant regardless. Use at your own risk (there is no guarantee the resulting variant payload will be functional). + wait_for_copying (bool): Whether the caching step should block when one of the + pending variants is marked as already copying. + logger (None | Logger): If a logger is provided, log information to it. Returns: tuple: 2-tuple: @@ -214,17 +229,44 @@ def add_variant(self, variant, force=False): % package.repository ) - no_op_statuses = ( + no_op_statuses = { self.VARIANT_FOUND, - self.VARIANT_COPYING, - self.VARIANT_COPY_STALLED - ) + self.VARIANT_COPY_STALLED, + } + if not wait_for_copying: + # Copying variants are only no-ops if we want to ignore them. + no_op_statuses.add(self.VARIANT_COPYING) # variant already exists, or is being copied to cache by another thread/proc status, rootpath = self._get_cached_root(variant) if status in no_op_statuses: + if logger: + logger.warning(f"Not caching {variant.name}-{variant.version}. " + f"Variant {self.STATUS_DESCRIPTIONS[status]}") return (rootpath, status) + if wait_for_copying and status == self.VARIANT_COPYING: + ticks = 0 + while status == self.VARIANT_COPYING: + self._print_with_spinner( + f"Waiting for {variant.name}-{variant.version} to finish copying.", ticks) + ticks += 1 + + time.sleep(self._COPYING_TIME_INC) + status, rootpath = self._get_cached_root(variant) + else: + # Status has changed, so report the change and return + if logger: + if status in no_op_statuses: + logger.warning(f"{variant.name}-{variant.version} " + f"{self.STATUS_DESCRIPTIONS[status]}") + elif status == self.VARIANT_FOUND: + # We have resolved into a satisfactory state + logger.info(f"{variant.name}-{variant.version} {self.STATUS_DESCRIPTIONS[status]}") + else: + logger.warning(f"{variant.name}-{variant.version} {self.STATUS_DESCRIPTIONS[status]}") + return (rootpath, status) + # 1. path = self._get_hash_path(variant) safe_makedirs(path) @@ -396,6 +438,17 @@ def add_variants(self, variants, package_cache_async=True): ) variants_ = [] + cachable_statuses = { + self.VARIANT_NOT_FOUND, + } + if not package_cache_async: + # We want to monitor copying variants if we're synchronous. + # We also want to report that a status has been stalled, so we'll + # hand that off to the caching function as well + cachable_statuses.update({ + self.VARIANT_COPYING, + self.VARIANT_COPY_STALLED, + }) # trim down to those variants that are cachable, and not already cached for variant in variants: @@ -403,7 +456,7 @@ def add_variants(self, variants, package_cache_async=True): continue status, _ = self._get_cached_root(variant) - if status == self.VARIANT_NOT_FOUND: + if status in cachable_statuses: variants_.append(variant) # if there are no variants to add, and no potential cleanup to do, then exit @@ -444,6 +497,20 @@ def add_variants(self, variants, package_cache_async=True): with open(filepath, 'w') as f: f.write(json.dumps(handle_dict)) + if package_cache_async: + self._subprocess_package_caching_daemon(self.path) + else: + # syncronous caching + self.run_caching_operation(wait_for_copying=True) + + @staticmethod + def _subprocess_package_caching_daemon(path): + """ + Run the package cache in a daemon process + + Returns: + subprocess.Popen : The package caching daemon process + """ # configure executable if platform.system() == "Windows": kwargs = { @@ -460,7 +527,7 @@ def add_variants(self, variants, package_cache_async=True): raise RuntimeError("Did not find rez-pkg-cache executable") # start caching subproc - args = [exe, "--daemon", self.path] + args = [exe, "--daemon", path] try: with open(os.devnull, 'w') as devnull: @@ -471,14 +538,12 @@ def add_variants(self, variants, package_cache_async=True): else: out_target = devnull - process = subprocess.Popen( + return subprocess.Popen( args, stdout=out_target, stderr=out_target, **kwargs ) - if not package_cache_async: - process.communicate() except Exception as e: print_warning( @@ -577,6 +642,15 @@ def run_daemon(self): if pid > 0: sys.exit(0) + self.run_caching_operation(wait_for_copying=False) + + def run_caching_operation(self, wait_for_copying=False): + """Copy pending variants. + + Args: + wait_for_copying (bool): Whether the caching step should block when one of the + pending variants is marked as already copying. + """ logger = self._init_logging() # somewhere for the daemon to store stateful info @@ -587,7 +661,7 @@ def run_daemon(self): # copy variants into cache try: while True: - keep_running = self._run_daemon_step(state) + keep_running = self._run_caching_step(state, wait_for_copying=wait_for_copying) if not keep_running: break except Exception: @@ -701,12 +775,13 @@ def _lock(self): except NotLocked: pass - def _run_daemon_step(self, state): + def _run_caching_step(self, state, wait_for_copying=False): logger = state["logger"] # pick a random pending variant to copy pending_filenames = set(os.listdir(self._pending_dir)) - pending_filenames -= set(state.get("copying", set())) + if not wait_for_copying: + pending_filenames -= set(state.get("copying", set())) if not pending_filenames: return False @@ -729,7 +804,11 @@ def _run_daemon_step(self, state): t = time.time() try: - rootpath, status = self.add_variant(variant) + rootpath, status = self.add_variant( + variant, + wait_for_copying=wait_for_copying, + logger=logger, + ) except PackageCacheError as e: # variant cannot be cached, so remove as a pending variant @@ -876,3 +955,13 @@ def _get_hash_path(self, variant): dirs.append(hash_dirname) return os.path.join(*dirs) + + @staticmethod + def _print_with_spinner(message, ticks): + """ + Report a message with a spinner wheel to indicate progress. + """ + wheel = "⣾⣽⣻⢿⡿⣟⣯⣷" + ticks = ticks % len(wheel) + spinner = wheel[ticks:1 + ticks] + print(f" {spinner} {message}", end="\r") From 2253d76ad9d3c8551e5e9615d8c8504dc0c8f5df Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Tue, 12 Mar 2024 14:04:59 +1100 Subject: [PATCH 12/20] line length fix Signed-off-by: Ben Andersen --- src/rez/package_cache.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 3780a73d4..0322d5ada 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -262,9 +262,11 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None) f"{self.STATUS_DESCRIPTIONS[status]}") elif status == self.VARIANT_FOUND: # We have resolved into a satisfactory state - logger.info(f"{variant.name}-{variant.version} {self.STATUS_DESCRIPTIONS[status]}") + logger.info(f"{variant.name}-{variant.version} " + f"{self.STATUS_DESCRIPTIONS[status]}") else: - logger.warning(f"{variant.name}-{variant.version} {self.STATUS_DESCRIPTIONS[status]}") + logger.warning(f"{variant.name}-{variant.version} " + f"{self.STATUS_DESCRIPTIONS[status]}") return (rootpath, status) # 1. From 1c6c94b26cee5718ecd22d931fd8887848cacd0f Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Thu, 9 May 2024 10:59:08 +1000 Subject: [PATCH 13/20] Cache reports variant.qualified_name and removed _print_with_spinner() Signed-off-by: Ben Andersen --- src/rez/package_cache.py | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 0322d5ada..db140bace 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -22,6 +22,7 @@ from rez.config import config from rez.exceptions import PackageCacheError from rez.vendor.lockfile import LockFile, NotLocked +from rez.vendor.progress.spinner import PixelSpinner from rez.utils.filesystem import safe_listdir, safe_makedirs, safe_remove, \ forceful_rmtree from rez.utils.colorize import ColorizedStreamHandler @@ -241,31 +242,28 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None) status, rootpath = self._get_cached_root(variant) if status in no_op_statuses: if logger: - logger.warning(f"Not caching {variant.name}-{variant.version}. " + logger.warning(f"Not caching {variant.qualified_name}. " f"Variant {self.STATUS_DESCRIPTIONS[status]}") return (rootpath, status) if wait_for_copying and status == self.VARIANT_COPYING: - ticks = 0 + spinner = PixelSpinner(f"Waiting for {variant.qualified_name} to finish copying. ") while status == self.VARIANT_COPYING: - self._print_with_spinner( - f"Waiting for {variant.name}-{variant.version} to finish copying.", ticks) - ticks += 1 - + spinner.next() time.sleep(self._COPYING_TIME_INC) status, rootpath = self._get_cached_root(variant) else: # Status has changed, so report the change and return if logger: if status in no_op_statuses: - logger.warning(f"{variant.name}-{variant.version} " + logger.warning(f"{variant.qualified_name} " f"{self.STATUS_DESCRIPTIONS[status]}") elif status == self.VARIANT_FOUND: # We have resolved into a satisfactory state - logger.info(f"{variant.name}-{variant.version} " + logger.info(f"{variant.qualified_name} " f"{self.STATUS_DESCRIPTIONS[status]}") else: - logger.warning(f"{variant.name}-{variant.version} " + logger.warning(f"{variant.qualified_name} " f"{self.STATUS_DESCRIPTIONS[status]}") return (rootpath, status) @@ -957,13 +955,3 @@ def _get_hash_path(self, variant): dirs.append(hash_dirname) return os.path.join(*dirs) - - @staticmethod - def _print_with_spinner(message, ticks): - """ - Report a message with a spinner wheel to indicate progress. - """ - wheel = "⣾⣽⣻⢿⡿⣟⣯⣷" - ticks = ticks % len(wheel) - spinner = wheel[ticks:1 + ticks] - print(f" {spinner} {message}", end="\r") From a5997b94a412b7534eb4be219ca85bf12ea88352 Mon Sep 17 00:00:00 2001 From: Ben Andersen <156872503+isohedronpipeline@users.noreply.github.com> Date: Fri, 14 Jun 2024 08:12:41 +1000 Subject: [PATCH 14/20] Update src/rez/package_cache.py Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> Signed-off-by: Ben Andersen <156872503+isohedronpipeline@users.noreply.github.com> --- src/rez/package_cache.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index db140bace..c2b4af94a 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -255,10 +255,7 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None) else: # Status has changed, so report the change and return if logger: - if status in no_op_statuses: - logger.warning(f"{variant.qualified_name} " - f"{self.STATUS_DESCRIPTIONS[status]}") - elif status == self.VARIANT_FOUND: + if status == self.VARIANT_FOUND: # We have resolved into a satisfactory state logger.info(f"{variant.qualified_name} " f"{self.STATUS_DESCRIPTIONS[status]}") From d8dbe4eed2cc397583f5d4f92f01b21ac1b354d5 Mon Sep 17 00:00:00 2001 From: Ben Andersen <156872503+isohedronpipeline@users.noreply.github.com> Date: Fri, 14 Jun 2024 08:13:09 +1000 Subject: [PATCH 15/20] Update src/rez/package_cache.py Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com> Signed-off-by: Ben Andersen <156872503+isohedronpipeline@users.noreply.github.com> --- src/rez/package_cache.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index c2b4af94a..d9227cc06 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -417,10 +417,7 @@ def add_variants_async(self, variants): return self.add_variants(variants, package_cache_async=True) def add_variants(self, variants, package_cache_async=True): - """Update the package cache by adding some or all of the given variants. - - This method is called when a context is created or sourced. Variants - are then added to the cache in a separate process. + """Add the given variants to the package payload cache. """ # A prod install is necessary because add_variants works by From 72747ca418cb8f710fda474d6cf46b265f6fd8ef Mon Sep 17 00:00:00 2001 From: Ben Andersen Date: Fri, 14 Jun 2024 08:20:51 +1000 Subject: [PATCH 16/20] Made run_caching_operation private changed wait_for_copying on run_caching_operation default to True Signed-off-by: Ben Andersen --- src/rez/package_cache.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index d9227cc06..8f7dec285 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -495,7 +495,7 @@ def add_variants(self, variants, package_cache_async=True): self._subprocess_package_caching_daemon(self.path) else: # syncronous caching - self.run_caching_operation(wait_for_copying=True) + self._run_caching_operation(wait_for_copying=True) @staticmethod def _subprocess_package_caching_daemon(path): @@ -636,9 +636,9 @@ def run_daemon(self): if pid > 0: sys.exit(0) - self.run_caching_operation(wait_for_copying=False) + self._run_caching_operation(wait_for_copying=False) - def run_caching_operation(self, wait_for_copying=False): + def _run_caching_operation(self, wait_for_copying=True): """Copy pending variants. Args: From a8d0439e39c3f9048ca23e0bb4f8ff326e46c0be Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 29 Jun 2024 10:10:32 -0400 Subject: [PATCH 17/20] Remove redundant else Signed-off-by: Jean-Christophe Morin --- src/rez/package_cache.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 8f7dec285..f4c3ad720 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -252,17 +252,17 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None) spinner.next() time.sleep(self._COPYING_TIME_INC) status, rootpath = self._get_cached_root(variant) - else: - # Status has changed, so report the change and return - if logger: - if status == self.VARIANT_FOUND: - # We have resolved into a satisfactory state - logger.info(f"{variant.qualified_name} " + + # Status has changed, so report the change and return + if logger: + if status == self.VARIANT_FOUND: + # We have resolved into a satisfactory state + logger.info(f"{variant.qualified_name} " + f"{self.STATUS_DESCRIPTIONS[status]}") + else: + logger.warning(f"{variant.qualified_name} " f"{self.STATUS_DESCRIPTIONS[status]}") - else: - logger.warning(f"{variant.qualified_name} " - f"{self.STATUS_DESCRIPTIONS[status]}") - return (rootpath, status) + return (rootpath, status) # 1. path = self._get_hash_path(variant) From 71e0c4a0f1f133d720e9145eb4324035fc8faf0a Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 29 Jun 2024 10:12:58 -0400 Subject: [PATCH 18/20] Fix flake8 warning Signed-off-by: Jean-Christophe Morin --- src/rez/package_cache.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index f4c3ad720..4e524e26f 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -257,8 +257,9 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None) if logger: if status == self.VARIANT_FOUND: # We have resolved into a satisfactory state - logger.info(f"{variant.qualified_name} " - f"{self.STATUS_DESCRIPTIONS[status]}") + logger.info( + f"{variant.qualified_name} {self.STATUS_DESCRIPTIONS[status]}" + ) else: logger.warning(f"{variant.qualified_name} " f"{self.STATUS_DESCRIPTIONS[status]}") From b12651ef927f62923d301c0ca99aaa5af9028b63 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 29 Jun 2024 10:18:31 -0400 Subject: [PATCH 19/20] More flake8 warnings fix Signed-off-by: Jean-Christophe Morin --- src/rez/package_cache.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 4e524e26f..50f9fbff2 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -261,8 +261,9 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None) f"{variant.qualified_name} {self.STATUS_DESCRIPTIONS[status]}" ) else: - logger.warning(f"{variant.qualified_name} " - f"{self.STATUS_DESCRIPTIONS[status]}") + logger.warning( + f"{variant.qualified_name} {self.STATUS_DESCRIPTIONS[status]}" + ) return (rootpath, status) # 1. From 355a891e2710e17585a35298a082d8b24b6f58d2 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 29 Jun 2024 10:22:32 -0400 Subject: [PATCH 20/20] Adjust versions to 3.2.0 Signed-off-by: Jean-Christophe Morin --- src/rez/package_cache.py | 2 +- src/rez/rezconfig.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 50f9fbff2..034320b58 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -413,7 +413,7 @@ def add_variants_async(self, variants): This method is called when a context is created or sourced. Variants are then added to the cache in a separate process. - .. deprecated:: 3.1.0 + .. deprecated:: 3.2.0 Use :method:`add_variants` instead. """ return self.add_variants(variants, package_cache_async=True) diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index f04688fb7..c1b2a7594 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -281,7 +281,7 @@ # Asynchronously cache packages. If this is false, resolves will block until # all packages are cached. # -# .. versionadded:: 3.1.0 +# .. versionadded:: 3.2.0 package_cache_async = True # Allow caching of local packages. You would only want to set this True for