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

hooks: qt: fix compatibility with old PyQt5 (e.g., 5.9.2) #6114

Merged
merged 1 commit into from
Aug 17, 2021

Conversation

rokm
Copy link
Member

@rokm rokm commented Aug 12, 2021

Older versions of PyQt5 (e.g., 5.9.2 from conda's main channel) lack __version__ attribute, causing is_module_satisfies() call to fail with AttributeError. Therefore, encountering this error means that the installed PyQt5 version is definitely still using the old PyQt5/Qt layout.

@bwoodsend
Copy link
Member

To me this raises the question of why on earth are we trying to access the __version__ attribute instead of using pkg_resources?

@rokm
Copy link
Member Author

rokm commented Aug 12, 2021

To me this raises the question of why on earth are we trying to access the __version__ attribute instead of using pkg_resources?

I think that's a fallback in is_module_satisfies when pkg_resources.get_distribution() fails. Which it does, with PyQt5 version in question.

@bwoodsend
Copy link
Member

bwoodsend commented Aug 12, 2021

Which it does, with PyQt5 version in question.

In what way? pkg_resources.get_distribution("PyQt5").version works fine for me with my current version of PyQt5.

Edit: I've just remembered that your example breakage is on Conda - is this some conda specific quirk?

@rokm
Copy link
Member Author

rokm commented Aug 12, 2021

Which it does, with PyQt5 version in question.

In what way? pkg_resources.get_distribution("PyQt5").version works fine for me with my current version of PyQt5.

This fix is targeting PyQt5 5.9.2 from conda main channel, where pkg_resources.get_distribution('PyQt5') raises pkg_resources.DistributionNotFound.

@bwoodsend
Copy link
Member

I don't suppose importlib.metadata.version("PyQt5") works either?

@bwoodsend
Copy link
Member

Actually, isn't Conda's PyQt5 just named PyQt instead of PyQt5? In which case would is_module_satisfies("PyQt") be what we should use on Conda?

@rokm
Copy link
Member Author

rokm commented Aug 12, 2021

I don't suppose importlib.metadata.version("PyQt5") works either?

Nope. And PyQt5 does not show up in pip freeze, either. Looks like that version is lacking dist metadata for some reason...

Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

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

OK, guess it has to be this way then.

@rokm
Copy link
Member Author

rokm commented Aug 12, 2021

Actually, isn't Conda's PyQt5 just named PyQt instead of PyQt5? In which case would is_module_satisfies("PyQt") be what we should use on Conda?

The conda package is named just PyQt, but the dist name is still PyQt5, at least for later versions (5.15.x) that are available on conda-forge. But those work without problems without this fix.

@rokm
Copy link
Member Author

rokm commented Aug 12, 2021

I've changed the commit description, the comment, and the news entry to reflect the fact that this is targeting PyQt5 5.9.2 from conda main channel. The old PyQt5 5.9.x wheels from PyPI seem to contain dist information, and would likely not trigger this issue.

@rokm
Copy link
Member Author

rokm commented Aug 12, 2021

Will merge after reformat, though.

PyQt5 5.9.2 from conda's main channel lacks dist information,
which forces the is_module_satisfies() call to take a fallback
path that checks for __version__ module attribute. This
attribute does not exist, causing the whole version check to
result in an AttributeError.

Handle such error gracefully, as it means that  the  installed
PyQt5 version is definitely still using the old PyQt5/Qt layout.
@bwoodsend bwoodsend added the merge-on-ci-pass This PR is ready to merge providing CI passes label Aug 17, 2021
@rokm rokm merged commit 9b80e07 into pyinstaller:develop Aug 17, 2021
@rokm rokm deleted the old-pyqt5 branch August 17, 2021 20:29
rokm added a commit that referenced this pull request Aug 22, 2021
…#6114)

PyQt5 5.9.2 from conda's main channel lacks dist information,
which forces the is_module_satisfies() call to take a fallback
path that checks for __version__ module attribute. This
attribute does not exist, causing the whole version check to
result in an AttributeError.

Handle such error gracefully, as it means that  the  installed
PyQt5 version is definitely still using the old PyQt5/Qt layout.
@rokm rokm added the backported Backported to v4 branch label Sep 26, 2021
@bwoodsend bwoodsend mentioned this pull request Feb 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backported Backported to v4 branch merge-on-ci-pass This PR is ready to merge providing CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants