From 81810027a24597bf673d136c0082f10bfd331e3c Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Sat, 19 Mar 2022 11:35:58 +1100 Subject: [PATCH 1/2] added package fam removal func, and tests --- src/rez/cli/rm.py | 31 ++++++++++++++- .../packages/py_packages/empty/README.md | 1 + src/rez/package_remove.py | 17 ++++++++ src/rez/package_repository.py | 14 +++++++ src/rez/packages.py | 18 +++++++++ src/rez/tests/test_packages.py | 39 +++++++++++++++++-- .../package_repository/filesystem.py | 29 +++++++++++++- 7 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 src/rez/data/tests/packages/py_packages/empty/README.md diff --git a/src/rez/cli/rm.py b/src/rez/cli/rm.py index c4997d496..042f614c6 100644 --- a/src/rez/cli/rm.py +++ b/src/rez/cli/rm.py @@ -15,6 +15,11 @@ def setup_parser(parser, completions=False): "-p", "--package", help="remove the specified package (eg 'foo-1.2.3'). This will work " "even if the package is currently ignored.") + group.add_argument( + "-f", "--family", + help="remove the entire specified package family (eg 'python'). This " + "is only supported if the family is empty, or all its packages are " + "hidden") group.add_argument( "-i", "--ignored-since", type=int, metavar="DAYS", help="remove all packages that have been ignored for >= DAYS") @@ -24,7 +29,8 @@ def setup_parser(parser, completions=False): help="dry run mode") parser.add_argument( "PATH", nargs='?', - help="the repository containing the package(s) to remove.") + help="the repository containing the package(s) or package family " + " to remove.") def remove_package(opts, parser): @@ -46,6 +52,27 @@ def remove_package(opts, parser): sys.exit(1) +def remove_package_family(opts, parser): + from rez.vendor.version.requirement import VersionedObject + from rez.package_remove import remove_package_family + + if opts.dry_run: + parser.error("--dry-run is not supported with --family") + + if not opts.PATH: + parser.error("Must specify PATH with --family") + + obj = VersionedObject(opts.family) + if obj.version: + parser.error("Expected package name, not version") + + if remove_package_family(obj.name, opts.PATH): + print("Package family removed.") + else: + print("Package family not found.", file=sys.stderr) + sys.exit(1) + + def remove_ignored_since(opts, parser): from rez.package_remove import remove_packages_ignored_since @@ -73,6 +100,8 @@ def remove_ignored_since(opts, parser): def command(opts, parser, extra_arg_groups=None): if opts.package: remove_package(opts, parser) + elif opts.family: + remove_package_family(opts, parser) elif opts.ignored_since is not None: remove_ignored_since(opts, parser) else: diff --git a/src/rez/data/tests/packages/py_packages/empty/README.md b/src/rez/data/tests/packages/py_packages/empty/README.md new file mode 100644 index 000000000..11706b476 --- /dev/null +++ b/src/rez/data/tests/packages/py_packages/empty/README.md @@ -0,0 +1 @@ +A deliberately empty package family. diff --git a/src/rez/package_remove.py b/src/rez/package_remove.py index 1d22e0eb0..e721ae6fc 100644 --- a/src/rez/package_remove.py +++ b/src/rez/package_remove.py @@ -12,6 +12,23 @@ basestring = six.string_types[0] +def remove_package_family(name, path): + """Remove a package family from its repository. + + A family can only be deleted if it's empty, or if all packages it contains + are hidden. Raises a `PackageRepositoryError` if not empty. + + Args: + name (str): Name of package family. + path (str): Package repository path containing the package family. + + Returns: + bool: True if the package family was removed, False if not found. + """ + repo = package_repository_manager.get_repository(path) + return repo.remove_package_family(name) + + def remove_package(name, version, path): """Remove a package from its repository. diff --git a/src/rez/package_repository.py b/src/rez/package_repository.py index 3ceeeb7f1..33d71e1e6 100644 --- a/src/rez/package_repository.py +++ b/src/rez/package_repository.py @@ -269,6 +269,20 @@ def remove_package(self, pkg_name, pkg_version): """ raise NotImplementedError + def remove_package_family(self, pkg_name): + """Remove an empty package family. + + Non-empty families CANNOT be removed. You need to remove all packages + via `remove_package` beforehand. + + Please note that a package family IS considered empty if all packages + it contains are hidden. + + Returns: + bool: True if the family was removed, False if it wasn't found. + """ + raise NotImplementedError + def remove_ignored_since(self, days, dry_run=False, verbose=False): """Remove packages ignored for >= specified number of days. diff --git a/src/rez/packages.py b/src/rez/packages.py index 64394bb9d..b4c0c914d 100644 --- a/src/rez/packages.py +++ b/src/rez/packages.py @@ -598,6 +598,24 @@ def get_package(name, version, paths=None): return None +def get_package_family_from_repository(name, path): + """Get a package family from a repository. + + Args: + name (str): Name of the package, eg 'maya'. + + Returns: + `PackageFamily` object, or None if the family was not found. + """ + repo = package_repository_manager.get_repository(path) + + family_resource = repo.get_package_family(name) + if family_resource is None: + return None + + return PackageFamily(family_resource) + + def get_package_from_repository(name, version, path): """Get a package from a repository. diff --git a/src/rez/tests/test_packages.py b/src/rez/tests/test_packages.py index 82dde0dcd..5dc22fddf 100644 --- a/src/rez/tests/test_packages.py +++ b/src/rez/tests/test_packages.py @@ -7,11 +7,14 @@ """ from rez.packages import iter_package_families, iter_packages, get_package, \ create_package, get_developer_package, get_variant_from_uri, \ - get_package_from_uri, get_package_from_repository + get_package_from_uri, get_package_from_repository, \ + get_package_family_from_repository +from rez.exceptions import PackageRepositoryError from rez.package_py_utils import expand_requirement from rez.package_resources import package_release_keys from rez.package_move import move_package -from rez.package_remove import remove_package, remove_packages_ignored_since +from rez.package_remove import remove_package, remove_packages_ignored_since, \ + remove_package_family from rez.package_repository import package_repository_manager from rez.tests.util import TestBase, TempdirMixin from rez.utils.formatting import PackageRequest @@ -58,7 +61,10 @@ ]) -ALL_FAMILIES = set(x.split('-')[0] for x in ALL_PACKAGES) +ALL_FAMILIES = set( + [x.split('-')[0] for x in ALL_PACKAGES] + + ["empty"] +) def _to_names(it): @@ -544,6 +550,33 @@ def test_package_remove(self): i = repo.unignore_package(pkg_name, pkg_version) self.assertEqual(i, -1) + def test_package_family_remove(self): + """Test package family remove.""" + pkg_name = "pydad" + + # copy packages to a temp repo + repo_path = os.path.join(self.root, "tmp6_packages") + shutil.copytree(self.solver_packages_path, repo_path) + + # verify that source fam exists + src_fam = get_package_family_from_repository(pkg_name, repo_path) + self.assertNotEqual(src_fam, None) + + # remove it, will fail as not empty + with self.assertRaises(PackageRepositoryError): + remove_package_family(pkg_name, repo_path) + + # remove all pydad packages + versions = [pkg.version for pkg in src_fam.iter_packages()] + for version in versions: + self.assertTrue(remove_package(pkg_name, version, repo_path)) + + # now it should remove successfully + self.assertTrue(remove_package_family(pkg_name, repo_path)) + + # verify that the fam no longer exists + self.assertFalse(remove_package_family(pkg_name, repo_path)) + def test_remove_packages_ignored_since(self): pkg_name = "pydad" pkg_version = Version("2") diff --git a/src/rezplugins/package_repository/filesystem.py b/src/rezplugins/package_repository/filesystem.py index 6ed3743e4..3fa179a14 100644 --- a/src/rezplugins/package_repository/filesystem.py +++ b/src/rezplugins/package_repository/filesystem.py @@ -735,6 +735,31 @@ def remove_package(self, pkg_name, pkg_version): return True + def remove_package_family(self, pkg_name): + # get a non-cached copy and se if fam exists + repo_copy = self._copy(disable_memcache=True) + fam = repo_copy.get_package_family(pkg_name) + if fam is None: + return False + + # check that the pkg fam is empty, or contains only hidden packages + empty = True + for _ in repo_copy.iter_packages(fam): + empty = False + break + + if not empty: + raise PackageRepositoryError( + "Cannot remove non-empty package family %r" % pkg_name + ) + + # delete the fam dir + fam_dir = os.path.join(self.location, pkg_name) + shutil.rmtree(fam_dir) + + self._on_changed(pkg_name) + return True + def remove_ignored_since(self, days, dry_run=False, verbose=False): now = int(time.time()) num_removed = 0 @@ -1464,7 +1489,9 @@ def _on_changed(self, pkg_name): # stats are required to determine if a resolve cache entry is stale. # family_path = os.path.join(self.location, pkg_name) - os.utime(family_path, None) + + if os.path.exists(family_path): + os.utime(family_path, None) # clear internal caches, otherwise change may not be visible self.clear_caches() From 2321344ed3a035279faeb1601f3bc96ba496ba23 Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Tue, 22 Mar 2022 11:30:24 +1100 Subject: [PATCH 2/2] addressed PR comments -added force option -only allow rm of truly empty packages when force=false --- src/rez/cli/rm.py | 22 ++++++++++---- src/rez/package_remove.py | 9 +++--- src/rez/package_repository.py | 10 +++---- src/rez/tests/test_packages.py | 15 ++++++++++ .../package_repository/filesystem.py | 29 +++++++++++-------- 5 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/rez/cli/rm.py b/src/rez/cli/rm.py index 042f614c6..31512e2d2 100644 --- a/src/rez/cli/rm.py +++ b/src/rez/cli/rm.py @@ -17,9 +17,11 @@ def setup_parser(parser, completions=False): "even if the package is currently ignored.") group.add_argument( "-f", "--family", - help="remove the entire specified package family (eg 'python'). This " - "is only supported if the family is empty, or all its packages are " - "hidden") + help="remove the specified package family (eg 'python'). This is only " + "supported if the family is empty") + group.add_argument( + "--force-family", action="store_true", + help="like -f, but delete package family even if not empty") group.add_argument( "-i", "--ignored-since", type=int, metavar="DAYS", help="remove all packages that have been ignored for >= DAYS") @@ -52,9 +54,10 @@ def remove_package(opts, parser): sys.exit(1) -def remove_package_family(opts, parser): +def remove_package_family(opts, parser, force=False): from rez.vendor.version.requirement import VersionedObject from rez.package_remove import remove_package_family + from rez.exceptions import PackageRepositoryError if opts.dry_run: parser.error("--dry-run is not supported with --family") @@ -66,7 +69,14 @@ def remove_package_family(opts, parser): if obj.version: parser.error("Expected package name, not version") - if remove_package_family(obj.name, opts.PATH): + success = False + try: + success = remove_package_family(obj.name, opts.PATH, force=force) + except PackageRepositoryError as e: + print("Error: %s" % e, file=sys.stderr) + sys.exit(1) + + if success: print("Package family removed.") else: print("Package family not found.", file=sys.stderr) @@ -102,6 +112,8 @@ def command(opts, parser, extra_arg_groups=None): remove_package(opts, parser) elif opts.family: remove_package_family(opts, parser) + elif opts.force_family: + remove_package_family(opts, parser, force=True) elif opts.ignored_since is not None: remove_ignored_since(opts, parser) else: diff --git a/src/rez/package_remove.py b/src/rez/package_remove.py index e721ae6fc..cbd35676a 100644 --- a/src/rez/package_remove.py +++ b/src/rez/package_remove.py @@ -12,21 +12,22 @@ basestring = six.string_types[0] -def remove_package_family(name, path): +def remove_package_family(name, path, force=False): """Remove a package family from its repository. - A family can only be deleted if it's empty, or if all packages it contains - are hidden. Raises a `PackageRepositoryError` if not empty. + A family can only be deleted if it contains no packages, hidden or + otherwise, unless `force` is True. Args: name (str): Name of package family. path (str): Package repository path containing the package family. + force (bool): If True, delete family even if not empty. Returns: bool: True if the package family was removed, False if not found. """ repo = package_repository_manager.get_repository(path) - return repo.remove_package_family(name) + return repo.remove_package_family(name, force=force) def remove_package(name, version, path): diff --git a/src/rez/package_repository.py b/src/rez/package_repository.py index 33d71e1e6..67bba6049 100644 --- a/src/rez/package_repository.py +++ b/src/rez/package_repository.py @@ -269,14 +269,12 @@ def remove_package(self, pkg_name, pkg_version): """ raise NotImplementedError - def remove_package_family(self, pkg_name): + def remove_package_family(self, pkg_name, force=False): """Remove an empty package family. - Non-empty families CANNOT be removed. You need to remove all packages - via `remove_package` beforehand. - - Please note that a package family IS considered empty if all packages - it contains are hidden. + Args: + pkg_name (str): Package name + force (bool): If Trur, delete even if not empty. Returns: bool: True if the family was removed, False if it wasn't found. diff --git a/src/rez/tests/test_packages.py b/src/rez/tests/test_packages.py index 5dc22fddf..b94a0dde9 100644 --- a/src/rez/tests/test_packages.py +++ b/src/rez/tests/test_packages.py @@ -577,6 +577,21 @@ def test_package_family_remove(self): # verify that the fam no longer exists self.assertFalse(remove_package_family(pkg_name, repo_path)) + pkg_name2 = "pymum" + + # get another fam + src_fam2 = get_package_family_from_repository(pkg_name2, repo_path) + self.assertNotEqual(src_fam2, None) + + # force remove another fam + self.assertTrue(remove_package_family(pkg_name2, repo_path, force=True)) + + # verify that the fam no longer exists + self.assertEqual( + get_package_family_from_repository(pkg_name2, repo_path), + None + ) + def test_remove_packages_ignored_since(self): pkg_name = "pydad" pkg_version = Version("2") diff --git a/src/rezplugins/package_repository/filesystem.py b/src/rezplugins/package_repository/filesystem.py index 3fa179a14..77d9f54a0 100644 --- a/src/rezplugins/package_repository/filesystem.py +++ b/src/rezplugins/package_repository/filesystem.py @@ -735,23 +735,28 @@ def remove_package(self, pkg_name, pkg_version): return True - def remove_package_family(self, pkg_name): - # get a non-cached copy and se if fam exists - repo_copy = self._copy(disable_memcache=True) + def remove_package_family(self, pkg_name, force=False): + # get a non-cached copy and see if fam exists + repo_copy = self._copy( + disable_pkg_ignore=True, + disable_memcache=True + ) + fam = repo_copy.get_package_family(pkg_name) if fam is None: return False - # check that the pkg fam is empty, or contains only hidden packages - empty = True - for _ in repo_copy.iter_packages(fam): - empty = False - break + # check that the pkg fam is empty + if not force: + empty = True + for _ in repo_copy.iter_packages(fam): + empty = False + break - if not empty: - raise PackageRepositoryError( - "Cannot remove non-empty package family %r" % pkg_name - ) + if not empty: + raise PackageRepositoryError( + "Cannot remove non-empty package family %r" % pkg_name + ) # delete the fam dir fam_dir = os.path.join(self.location, pkg_name)