Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix get_plugins() bug #1723

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/source/package_definition.rst
Original file line number Diff line number Diff line change
Expand Up @@ -748,14 +748,14 @@ the data type, and includes a code snippet.
name = "maya_utils"

.. py:attribute:: plugin_for
:type: str
:type: list[str]

Provided if this package is a plugin of another package. For example, this might be a maya plugin.
This is useful when using the :ref:`rez-plugins` command. Also, see :attr:`has_plugins`.

.. code-block:: python

plugin_for = "maya"
plugin_for = ["maya"]

.. py:function:: post_commands() -> None

Expand Down
5 changes: 3 additions & 2 deletions src/rez/package_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import fnmatch
from collections import defaultdict
from typing import List
import sys

from rez.packages import iter_package_families, iter_packages, get_latest_package
Expand Down Expand Up @@ -126,7 +127,7 @@ def get_reverse_dependency_tree(package_name, depth=None, paths=None,
return pkgs_list, g


def get_plugins(package_name, paths=None):
def get_plugins(package_name: str, paths: List[str] = None) -> List[str]:
"""Find packages that are plugins of the given package.

Args:
Expand All @@ -152,7 +153,7 @@ def get_plugins(package_name, paths=None):
continue # not a plugin of itself

plugin_pkg = get_latest_package(package_name_, paths=paths)
if not plugin_pkg.plugin_for:
if not plugin_pkg or not plugin_pkg.plugin_for:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should instead make sure that get_latest_package returns valid packages. The docstring of get_latest_package says that it should either return a Package object or None is a package is not found. There is also this TODO

rez/src/rez/packages.py

Lines 925 to 926 in 71d990b

# FIXME this isn't correct, since the pkg fam may exist but a pkg
# in the range does not.

It my books, that's what I call smell.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of #1618 I would say that get_latest_package is correctly returning None, as there is no valid package in an empty folder. Perhaps then iter_package_families should be more thorough and not return package names if there is no valid package in the folders?

continue
for plugin_for in plugin_pkg.plugin_for:
if plugin_for == pkg.name:
Expand Down
64 changes: 64 additions & 0 deletions src/rez/tests/test_package_search.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright Contributors to the Rez Project


"""
Test cases for package_search.py (package searching)
"""
import os

from rez.package_maker import make_package
from rez.package_search import get_plugins
from rez.tests.util import TestBase, TempdirMixin


class TestPackageSearch(TestBase, TempdirMixin):
"""Class for a package searching test case"""
@classmethod
def setUpClass(cls):
TempdirMixin.setUpClass()

cls.settings = dict(
packages_path=[
cls.root
]
)

@classmethod
def tearDownClass(cls):
TempdirMixin.tearDownClass()

def test_not_has_plugins(self):
"""Ensure empty list is returned when has_plugins is False."""

with make_package('foo', self.root) as pkg:
pkg.has_plugins = False

plugins = get_plugins('foo')
self.assertEqual(plugins, [])

def test_get_plugins(self):
"""Ensure package plugins are obtained as expected."""

with make_package('foo', self.root) as pkg:
pkg.has_plugins = True

with make_package('foo_plugin', self.root) as pkg:
pkg.plugin_for = ["foo"]

plugins = get_plugins('foo')
self.assertEqual(plugins, ['foo_plugin'])

def test_get_plugins_empty_folder(self):
"""When an empty folder is present, plugins should still be valid."""

with make_package('foo', self.root) as pkg:
pkg.has_plugins = True

with make_package('foo_plugin', self.root) as pkg:
pkg.plugin_for = ["foo"]

os.mkdir(os.path.join(self.root, 'broken_pkg'))

plugins = get_plugins('foo')
self.assertEqual(plugins, ['foo_plugin'])
Loading