Skip to content

python module: fix broken non-embed dependency #11104

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

Merged
merged 3 commits into from
Nov 24, 2022

Conversation

eli-schwartz
Copy link
Member

The py.dependency(embed: false) method is supposed to consistently provide a distutils-like python.pc / python-embed.pc interface regardless of Python version. It handles both pkg-config and sysconfig scraping. For the latter, we respect the value of self.link_libpython as determined by distutils, and construct a fully custom dependency. For the former, we blindly assume pkg-config is correct.

It isn't correct, not until Python 3.8 when embed was added. Before then, we need to process the pkg-config dependency based on link_libpython. We did this, but only inside the extension_module method, which is obviously wrong.

Delete the special casing from extension_module, and handle it inside the dependency.

Fixes #11097

@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

Merging #11104 (c4abddf) into master (fd43842) will decrease coverage by 1.33%.
The diff coverage is n/a.

❗ Current head c4abddf differs from pull request most recent head 9d1b59f. Consider uploading reports for the commit 9d1b59f to get more accurate results

@@            Coverage Diff             @@
##           master   #11104      +/-   ##
==========================================
- Coverage   68.64%   67.30%   -1.34%     
==========================================
  Files         412      207     -205     
  Lines       87863    44920   -42943     
  Branches    20728     9279   -11449     
==========================================
- Hits        60314    30235   -30079     
+ Misses      23041    12403   -10638     
+ Partials     4508     2282    -2226     
Impacted Files Coverage Δ
modules/cuda.py 0.00% <0.00%> (-72.64%) ⬇️
templates/cudatemplates.py 37.50% <0.00%> (-62.50%) ⬇️
compilers/cuda.py 19.63% <0.00%> (-45.40%) ⬇️
dependencies/cuda.py 20.19% <0.00%> (-42.79%) ⬇️
compilers/mixins/clang.py 52.87% <0.00%> (-13.80%) ⬇️
scripts/coverage.py 56.43% <0.00%> (-7.93%) ⬇️
compilers/detect.py 42.97% <0.00%> (-3.63%) ⬇️
linkers/linkers.py 56.35% <0.00%> (-1.25%) ⬇️
environment.py 80.12% <0.00%> (-1.08%) ⬇️
dependencies/misc.py 44.55% <0.00%> (-0.75%) ⬇️
... and 213 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dnicolodi
Copy link
Member

Too bad this missed 0.64.1. Can we have this for 0.64.2 please?

The `py.dependency(embed: false)` method is supposed to consistently
provide a distutils-like `python.pc` / `python-embed.pc` interface
regardless of Python version. It handles both pkg-config and sysconfig
scraping. For the latter, we respect the value of self.link_libpython
as determined by distutils, and construct a fully custom dependency. For
the former, we blindly assume pkg-config is correct.

It isn't correct, not until Python 3.8 when embed was added. Before
then, we need to process the pkg-config dependency based on
link_libpython. We did this, but only inside the extension_module
method, which is obviously wrong.

Delete the special casing from extension_module, and handle it inside
the dependency.

Fixes mesonbuild#11097
These are trivially inferred based on their initialized values.
flake8 6 upgrades to pyflakes 3, and in turn this means that support for
parsing `# type: ` style annotations has been removed.

PyCQA/pyflakes#684

This caused one file to fail linting, because it had a typing import
which was only used by a type comment.

```
mesonbuild/cmake/interpreter.py:55:5: F401 '.common.CMakeConfiguration' imported but unused
```

Updating it to actual annotations allows pyflakes to detect its usage
again, and flake8 passes. Do the whole file while we are here.
@eli-schwartz eli-schwartz force-pushed the python-dependency-embed branch from 52939b5 to 9d1b59f Compare November 24, 2022 16:50
@eli-schwartz
Copy link
Member Author

r-b from xclaesse.

@eli-schwartz eli-schwartz merged commit 9d1b59f into mesonbuild:master Nov 24, 2022
@eli-schwartz eli-schwartz deleted the python-dependency-embed branch November 24, 2022 18:32
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.

python.extension_module() linking with libpython when it should not
2 participants