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

meson: simplify lookup of python command #83

Merged
merged 1 commit into from
Nov 7, 2022
Merged

meson: simplify lookup of python command #83

merged 1 commit into from
Nov 7, 2022

Conversation

apprehensions
Copy link
Contributor

No description provided.

@kjellahl
Copy link
Contributor

kjellahl commented Nov 6, 2022

According to the commit message of 82005b8
it's wise to get the python installation with import('python').find_installation(),
to be sure to use the python version that Meson uses.
I don't use Visual Studio myself. I'm not sure what problems may occur with
find_program('python3', version: '>=3.5'). I'd prefer to stick to
import('python').find_installation(). But the version test is unnecessary.
It can be removed. Meson itself requires python3 >= 3.7.
See https://mesonbuild.com/Getting-meson.html.

@apprehensions
Copy link
Contributor Author

I'm not sure what problems may occur with

They solve problems with other meson specification implementations.

But the version test is unnecessary.

python3 = import('python').find_installation()
python_version = python3.language_version()
python_version_req = '>= 3.5'
if not python_version.version_compare(python_version_req)
error('Requires Python @0@, found @1@.'.format(python_version_req, python_version))
endif

So is this complex lookup of the version - which looks for '3.5', hence why its added.

@kjellahl
Copy link
Contributor

kjellahl commented Nov 6, 2022

@fanc999 Do you think it's safe to change

python3 = import('python').find_installation()

to

python3 = find_program('python3', version: '>=3.5')

wael thinks import('python').find_installation() is unnecessarily complex.

@wael444 I wonder if you misunderstood my But the version test is unnecessary.
I meant the 5-line test in meson.build just now.

@eli-schwartz
Copy link

The purpose of the python module's find_installation method is to provide a rich object with methods for building python extensions.

If you just need a script interpreter, and stick to the stdlib so you don't need to use the modules: kwarg, then find_program() is indeed simpler -- and will fall back to using the python that Meson itself uses.

@fanc999
Copy link
Collaborator

fanc999 commented Nov 7, 2022

Hi @kjellahl ,

I think it might be ok, but I don't think it's really necessary. We could just take out the version check below, since Meson 0.55.x+ already requires Python 3.5.x or later.

With blessings, and cheers!

@kjellahl kjellahl merged commit 9f1748e into libsigcplusplus:master Nov 7, 2022
@kjellahl
Copy link
Contributor

kjellahl commented Nov 7, 2022

I've merged this PR and copied the commit to the libsigc++-2-12 branch.

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.

None yet

4 participants