Skip to content

Conversation

@dnicolodi
Copy link
Member

Apply the information gathered through distutils also to the case when the dependecy has not been expressed explicitly. The spurious linking has been observed on Python 3.7 on Linux where distutils does not instruct to link libpython but the Python's pkg-config contains linker flags to link to libpython.

Fixes #11097.

@dnicolodi dnicolodi requested a review from jpakkane as a code owner November 22, 2022 20:48
@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #11098 (b6e2359) into master (bfc8132) will decrease coverage by 0.81%.
The diff coverage is n/a.

❗ Current head b6e2359 differs from pull request most recent head 45e737c. Consider uploading reports for the commit 45e737c to get more accurate results

@@            Coverage Diff             @@
##           master   #11098      +/-   ##
==========================================
- Coverage   68.58%   67.77%   -0.82%     
==========================================
  Files         412      207     -205     
  Lines       87861    44927   -42934     
  Branches    20728     9926   -10802     
==========================================
- Hits        60261    30449   -29812     
+ Misses      23093    12106   -10987     
+ Partials     4507     2372    -2135     
Impacted Files Coverage Δ
modules/icestorm.py 57.14% <0.00%> (-40.00%) ⬇️
compilers/mixins/clang.py 37.93% <0.00%> (-28.74%) ⬇️
scripts/coverage.py 42.57% <0.00%> (-21.79%) ⬇️
compilers/objc.py 80.95% <0.00%> (-19.05%) ⬇️
compilers/objcpp.py 80.95% <0.00%> (-19.05%) ⬇️
cmake/traceparser.py 71.11% <0.00%> (-9.10%) ⬇️
compilers/c.py 45.01% <0.00%> (-5.11%) ⬇️
compilers/cpp.py 42.47% <0.00%> (-2.93%) ⬇️
cmake/tracetargets.py 74.66% <0.00%> (-2.67%) ⬇️
dependencies/pkgconfig.py 65.47% <0.00%> (-2.61%) ⬇️
... and 239 more

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

pydep = self._dependency_method_impl({})
if not pydep.found():
raise mesonlib.MesonException('Python dependency not found')
# On macOS and some Linux distros (Debian) distutils doesn't link
Copy link
Member Author

@dnicolodi dnicolodi Nov 23, 2022

Choose a reason for hiding this comment

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

I copied this comment from a few lines above, however, I'm not sure this statement is accurate. Judging from the python.pc distributed with Python, it is never necessary to link with libpython https://github.com/python/cpython/blob/81f7359f67a7166d57a10a3d5366406d9c85f1de/Misc/python.pc.in#L12 on systems where pkg-config is used, which I assume is at least all variants of Linux. The python.pc.in does not allow for inserting anything for the Libs entry.

Apply the information gathered through distutils also to the case when
the dependecy has not been expressed explicitly. The spurious linking
has been observed on Python 3.7 on Linux where distutils does not
instruct to link libpython but the Python's pkg-config contains linker
flags to link to libpython.

Fixes mesonbuild#11097.
@dnicolodi
Copy link
Member Author

I reworded the comments to be clarify the need for this change. Because the fix applies only to the dependency constructed from the pkg-config metadata, an alternative approach would be to strip the linker flags in python_factory when the dependency method is pkg-config and the Python version is <= 3.7.

@dnicolodi
Copy link
Member Author

dnicolodi commented Nov 23, 2022

The flake8 error in the tests is not due to this PR.

@dnicolodi
Copy link
Member Author

Superseded by #11104.

@dnicolodi dnicolodi closed this Nov 24, 2022
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

1 participant