Skip to content

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Apr 16, 2024

This PR eliminates use of setuptools' deprecated pkg_resources in hotdoc.utils.utils and hotdoc.extensions.gi.utils, substituting equivalent APIs provided by importlib.metadata.entry_points().

The entry_points() function used is actually imported from backports.entry_points_selectable (a new dependency, added to setup.py), which ensures that calls to entry_points() can take filtering arguments even in Python < 3.10, where importlib.metadata didn't natively support such usage.

(In practice, for recent Python 3.10+ releases, backports.entry_points_selectable.entry_points will be importlib.metadata.entry_points.)

The resulting hotdoc package passes all unit tests in hotdoc.tests on Python 3.12, even in a virtual environment with the setuptools package removed:

$ python3 -m unittest
.............................................................................................................
/usr/lib64/python3.12/tempfile.py:936: ResourceWarning:
 Implicitly cleaning up <TemporaryDirectory '/tmp/tmpe0jsh57p'>
  _warnings.warn(warn_message, ResourceWarning)
............
----------------------------------------------------------------------
Ran 121 tests in 3.232s

OK

hotdoc from the master branch, in that same situation, fails several tests outright, and skips over 80% of them:

$ python3 -m unittest
[...]
======================================================================
ERROR: hotdoc.tests.test_hotdoc (unittest.loader._FailedTest.hotdoc.tests.test_hotdoc)
----------------------------------------------------------------------
ImportError: Failed to import test module: hotdoc.tests.test_hotdoc
Traceback (most recent call last):
  File "/usr/lib64/python3.12/unittest/loader.py", line 394, in _find_test_path
    module = self._get_module_from_name(name)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.12/unittest/loader.py", line 337, in _get_module_from_name
    __import__(name)
  File ".../hotdoc/tests/test_hotdoc.py", line 30, in <module>
    from hotdoc.utils.utils import touch
  File ".../hotdoc/utils/utils.py", line 37, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'


----------------------------------------------------------------------
Ran 19 tests in 0.002s

FAILED (errors=11)

Copy link
Contributor Author

@ferdnyc ferdnyc left a comment

Choose a reason for hiding this comment

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

Some notes.

Comment on lines 4 to 9
import traceback

from backports.entry_points_selectable import entry_points

from hotdoc.core.links import Link
from hotdoc.utils.loggable import info, debug
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are added simply because they're used in the get_language_classes() function later on, but would have been undefined if they'd actually been called.

group='hotdoc.extensions.gi.languages',
name='get_language_classes'):
try:
ep_name = entry_point.name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is here (despite ep_name never being used) because the results of importlib.metadata.entry_points() are documented as not being validated; invoking any property is the suggested means of triggering validation.

# pylint: disable=broad-except
except Exception as exc:
info("Failed to load %s" % entry_point.module_name, exc)
info("Failed to load %s" % str(entry_point), exc)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the same token, entry_point.name (the equivalent of entry_point.module_name from pkg_resources isn't necessarily going to exist on every result from entry_points(), so we can't rely on it for logging. Logging the string value of entry_point ensures that it'll be logged even if it isn't a valid EntryPoint object.

Comment on lines 209 to 214
if entry_point.module_name == 'hotdoc_c_extension.extensions':
continue
try:
if entry_point.module == 'hotdoc_c_extension.extensions':
continue
ep_name = entry_point.name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same validation issue here; since we can't be sure entry_point has a .module property, we need to check it inside the try:.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Apr 16, 2024

For the record, I didn't regenerate the pinned requirements.txt with pip-compile, because that process seems somewhat broken. Pinning requirements versions, though it sounds nice in theory, often has the unwanted side-effect of locking you in to a particular Python version.

In this case, pip install -r requirements.txt fails under Python 3.12, because the pinned lxml version isn't compatible.

Fortunately, pip install -e . uses the unpinned dependency from setup.py, so it installs an lxml that's compatible with Python 3.12.

@MathieuDuponchelle
Copy link
Collaborator

Thanks A TON for looking at this :)

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Apr 21, 2024

I believe this is now fixed. I also added importlib_metadata as an explicit dependency in Python < 3.10, even though backports.entry_points_selectable will always pull it in if needed.

(I'm now importing it explicitly to access its distributions() function, instead of relying solely on the backports entry_points(), so I figured it made sense to be explicit about the dependency.)

@MathieuDuponchelle
Copy link
Collaborator

lgtm now, huge thanks again for performing maintenance on this project :)

@MathieuDuponchelle MathieuDuponchelle merged commit cdba38d into hotdoc:master Apr 24, 2024
@ferdnyc ferdnyc deleted the no-pkg-resources branch April 24, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants