fix pkg_resources import failures w/ py3-only loaders#2918
Merged
Conversation
Member
|
This looks good. Thanks for reporting it. My only concern is that someone could "optimize" the change back to the old code and no one would notice. Usually in such a situation, where creating a test is difficult, I'll instead protect the change with a comment or separate function. I'll illustrate that in a subsequent change. |
Member
|
On further consideration, after struggling to come up with a clean abstraction, I've decided to proceed with this implementation. Thanks! |
blitzy Bot
pushed a commit
to blitzy-showcase/ansible
that referenced
this pull request
Apr 20, 2026
Fix AttributeError when Python 3 falls back to the legacy find_module() protocol after find_spec() returns None. On modern Python 3 runtimes (>=3.4) the cached importlib.machinery.FileFinder does not accept a 'path' keyword argument on find_module(); on Python 3.12+ the method was removed entirely. Changes in lib/ansible/utils/collection_loader/_collection_finder.py: 1. Add a try/except/else block importing FileFinder from importlib.machinery with a HAS_FILE_FINDER sentinel for graceful degradation on runtimes that do not expose FileFinder (e.g., Py2). 2. Flatten _AnsibleCollectionFinder.find_spec() to the early-return-on- None idiom. Behavior preserved, control flow simplified. 3. Type-dispatch _AnsiblePathHookFinder.find_module() so that when the returned finder is a native FileFinder instance, find_module() is called WITHOUT the 'path' keyword argument. Other finder types continue to receive 'path=[self._pathctx]' unchanged. THIS IS THE CORE BUG FIX. 4. Flatten _AnsiblePathHookFinder.find_spec() to the early-return-on- None three-way if/elif/else idiom for structural symmetry with find_module(). Refs: ansible/ansible#76448 Refs: pypa/setuptools#2918
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
#1563 introduced a subtle bug in the fallback code for
pkg_resourcesenumeration. The original code that only usedfind_modulelater accounts for aNoneloader return value (ie, the requested module couldn't be found), but the new code blindly dots into the return fromfind_spec(which can also beNone) to grab itsloader. When this occurs, it inadvertently trips theAttributeErrorthat was supposed to be there for py2 loaders that don't implementfind_spec, causingfind_moduleto be called even on a py3-native loader. Loaders that still implement the deprecated 2.x methods will (correctly) returnNone, triggering the old fallback path, all is well. However, new loaders that don't implementfind_modulewill blow up thepkg_resourcesenumeration with anotherAttributeError.The inadvertent trip of the py2 fallback happens a lot even with just the built-in loaders (eg, instrument the fallback exception handler with a
printor something, then installstraight.pluginin a fresh venv andimport pkg_resources)- it works by accident until the builtin loaders start droppingfind_module(as is planned for Python 3.11+), at which point it gets ugly fast.This PR minimizes the try block's surface area to only the necessary loader method call, and defensively checks for a non-empty response from
find_specbefore trying to grab itsloader(otherwise setting the loader toNoneto restore the previous early exit behavior for modules that can't be loaded.This one's pretty tricky to add tests for without wiring up a bespoke py3 loader. Happy to add a changelog entry if the fix otherwise looks good...
Pull Request Checklist
changelog.d/.(See documentation for details)