Use native meson python module for python install. #253
Conversation
libgstc/python/meson.build
Outdated
| if pythonver.version_compare('<3.7') | ||
| error('Python @0@ is not supported anymore, please port your code to python3.7 or newer.'.format(python.language_version())) | ||
| endif |
There was a problem hiding this comment.
We need to relax this a bit. Some Jetson boards still ship will lower Python versions. For example:
Jetson Xavier AGX - Jetpack 4.6
nvidia@localhost:~$ python3 --version
Python 3.6.9
Is Python officially dropping maintenance for pre 3.7?
There was a problem hiding this comment.
Oh, that may have been due to a feature(asynccontextmanager) requiring 3.7 that I used in #252, I can probably rework that for python 3.6 compatibility if needed.
There was a problem hiding this comment.
Rebased and reverted minimum python version to 3.5, I'll re-base the asyncio pull request on top of this one after. Is python3.6 the minimum version we need to support?
There was a problem hiding this comment.
I would drop it to plain 3, and add a special requirement for the asyncio client independently (in that branch). We can't drop the original client for backwards compatibility reasons, so I'm thinking a more model alternative of the client.
There was a problem hiding this comment.
I would drop it to plain 3
Well it's currently minimum 3.5 here, is that requirement accurate?:
gstd-1.x/libgstc/python/setup.py
Line 50 in fa523d5
We can't drop the original client for backwards compatibility reasons, so I'm thinking a more model alternative of the client.
Oh, I guess there's clients that expect a stable API across library versions? I can probably just add some compatibility shims for API compatibility.
jameshilliard
force-pushed
the
python-install
branch
2 times, most recently
from
8e9e983
to
231e48a
Compare
jameshilliard
force-pushed
the
python-install
branch
from
231e48a
to
05912e9
Compare
libgstc/python/meson.build
Outdated
| pymod = import('python') | ||
| python = pymod.find_installation( | ||
| get_option('with-python-version'), | ||
| required : get_option('enable-python').enabled(), |
There was a problem hiding this comment.
This accepts a feature option directly, no need to convert it into a boolean.
And properly using feature options means that when passing -Denable-python=disabled, this python installation will be skipped rather than "looked for, but not required".
... Also the "enable-" in this option name seems a bit redundant given it needs to be defined as one of:
-Denable-python=disabled-Denable-python=enabled-Denable-python=auto
and the first two are... pretty confusing.
There was a problem hiding this comment.
Oh hmm, okay, I see the existing feature options use that "enable-" stuff already... never mind I guess.
:(
There was a problem hiding this comment.
This accepts a feature option directly, no need to convert it into a boolean.
Fixed.
Oh hmm, okay, I see the existing feature options use that "enable-" stuff already... never mind I guess.
Yeah, I had used that style for consistency.
There was a problem hiding this comment.
I agree, it is redundant. I assume this came from the autotools port. I may change them in a future branch.
jameshilliard
force-pushed
the
python-install
branch
from
05912e9
to
124e921
Compare
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
jameshilliard
force-pushed
the
python-install
branch
from
124e921
to
21230e2
Compare
This seems to be more reliable than shelling out to pip.