GNOME Bugzilla – Bug 733067
cerbero: support python3
Last modified: 2018-04-09 10:52:13 UTC
Created attachment 280519 [details] [review] support python3 This patch enables python3 support for cerbero. It's a single codebase, but I had to drop support for 2.5 as it would make things just too ugly. Mostly, this addresses changes in iterating over lists and dictionaties, and some byte/string stuff to supports python3's notion of unicode. Running cerbero with python3 and enabling the python3 variant does however create some redundancy I'd still like to get rid of. When enabling the python3 variant, gobject-introspection will depend on python3, which in turn will get built from source. This seems unneeded, since python3 is obviously already available.
Just wanted to say thanks for the patch, and sorry this fell through the cracks!
No problem, though after nearly two years I doubt the patch will still apply :) I doubt I can find time to update it...
(In reply to Dirk Van Haerenborgh from comment #2) > No problem, though after nearly two years I doubt the patch will still apply > :) > I doubt I can find time to update it... I've been meaning to work on this myself, so I'll take a look at the patch and update it sometime soon. Most likely during the spring hackfest next month. :)
I am working on a PR to add Python3 support here: https://github.com/centricular/cerbero/pull/1 It is work in progress still. I appreciate any feedback or comments. Most libraries (definitely those as useful as cerbero) should have support for Python3, and also they should be in pypi. Next I would like to see what it takes for setup.py to add cerbero to pypi. At this point I am interested in having a straightforward way for native compilation of gstreamer on Windows (with pdb to debug!) but I think I will rely more on cerbero in the future!
It is working in Python3 (and Python2). I tested it in Linux and Windows.
Looking at the patch, it's mostly trivial change. Though, a question raised into my mind, do we really need or want to keep Python 2 support ?
I think it is useful to keep Python 2 support for macOS since it still doesn't ship Python 3, and it would be strange to require Homebrew just for that. It would be great if you could review the GitHub pull request; we can then move the patch(es) to bugzilla and get them in. I've been a bit occupied lately. :/
meson is going to be python3 only anyway, so I wouldn't be too worried about keeping python2 support in cerbero. Especially since it's really complicate to build meson inside cerbero since that mix python2 and python3. See bug #789316.
I think we should get this merged ASAP and deal with any breakage later. Such big patch gets out of sync quickly and is a pain to rebase and re-test. If we find regressions it will be easy to fix anyway.
Maybe we can require Python 3 in Meson mode and accept either Python 2 or 3 in Autotools mode? To be honest, I would love it to just support Python 3, but we need to be sure that there's an easy way for macOS developers to use Cerbero. If we can do that with Python 3, we should definitely go for it.
I never used macos, but as far as I understand on the web, it's exactly the same as on windows: click click on an installer then set the PATH in your ~/.bash_profile. I don't think it's too much to ask to have python3 in 2018, sooner or later we'll have something depending on it.
macos has python2 by default though without requiring any additional installation. The question is if we want to require them to download python3 or be able to use the already installed python2 version.
Require them to install python3. It will be needed in the near future for meson builds of GStreamer anyway.
Created attachment 365598 [details] [review] python3: Run 2to3 script to conver the whole codebase This is only the raw output of the script, no manual editing. The code is now python3 only.
Created attachment 365599 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
Build on linux works, I'm currently running cross build to windows/android. Would be nice if CI can be run on this, if someone knows how to setup that.
Created attachment 365603 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
I've tested on osx and it's litterally just clicking on an installer from python.org, no setup needed. Build is ongoing, we'll see if it pass. cross compile linux->android passed. Open questions: - I see there are python2 references in bootstrap/windows.py:install_python_sdk(), I've no idea what it does. - config/windows.py setup python2 stuff, I don't know what it does. - libxml2 install python2 modules - cross compile on linux for win64 fails at the end with an error in hacks.py:write(), I don't know how to fix it.
Created attachment 365604 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
(In reply to Xavier Claessens from comment #18) > - cross compile on linux for win64 fails at the end with an error in > hacks.py:write(), I don't know how to fix it. Fixed.
I noticed gobject-introspection needs libpython 2.7 so we still need both when building it, but I don't think anything essential depends on it.
gobject-introspection builds just fine against python 3.4. In fact there's a WIP Meson port too: https://git.gnome.org/browse/gobject-introspection/log/?h=wip/meson
I think it needs both, as it builds support for both when possible (there a native side). Same for gst-python iirc, should not be affected by meson too much hopefully.
It doesn't need the interpreter but headers: https://git.gnome.org/browse/gobject-introspection/tree/configure.ac#n268
nevermind, with --with-python=python3 it will lookup for py3 headers as well. But that probably won't work on windows, it seems to extract gstreamer-external-sdk which is py2.7 headers. That needs to be updated as well I guess?
(In reply to Xavier Claessens from comment #25) > nevermind, with --with-python=python3 it will lookup for py3 headers as > well. But that probably won't work on windows, it seems to extract > gstreamer-external-sdk which is py2.7 headers. That needs to be updated as > well I guess? Yes, we shouldn't need gstreamer-external-sdk at all, especially if you build g-i with meson. I've already verified that g-i + meson works on Windows with a standard native Python 3 installation. Autotools will require some changes, but you can find the Python headers and libs easily, see: https://github.com/mesonbuild/meson/blob/6f42f83867a52c1f9c61a2780dfd808a21984e41/mesonbuild/dependencies/misc.py#L687. Perhaps copying from there makes sense instead of using the outdated SDK. We can get rid of it when we move to Meson.
Don't you still need it when cross building for windows, otherwise it would use de linux sdk, no?
Created attachment 365701 [details] [review] python3: Run 2to3 script to conver the whole codebase This is only the raw output of the script, no manual editing. The code is now python3 only.
Created attachment 365702 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
Created attachment 365703 [details] [review] WIP: python3: Use the SDK version corresponding to the interpreter On modern Linux, Windows and MacOS it will be python3.6, but Ubuntu 16.04 LTS has python3.5. WIP: gobject-introspection fails to build when cross compiling on Ubuntu 16.04 for Windows.
Created attachment 365704 [details] [review] python3: Update README
Created attachment 365721 [details] [review] python3: Run 2to3 script to conver the whole codebase This is only the raw output of the script, no manual editing. The code is now python3 only.
Created attachment 365722 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
Created attachment 365723 [details] [review] python3: Update README
ok so g-i never worked when cross compiling to windows, so that's correct. I needs windows-external-sdk from https://gitlab.collabora.com/xclaesse/windows-external-sdk
Review of attachment 365721 [details] [review]: Can you fix the commit message ? conver -> convert
(In reply to Xavier Claessens from comment #35) > ok so g-i never worked when cross compiling to windows, so that's correct. > > I needs windows-external-sdk from > https://gitlab.collabora.com/xclaesse/windows-external-sdk gstreamer-sdk/ is read-only now, we'll clone this back into gstreamer/ first, Tim will take care.
(In reply to Xavier Claessens from comment #27) > Don't you still need it when cross building for windows, otherwise it would > use de linux sdk, no? We don't enable gi while cross-compiling at all: if not self.prefix_is_executable() and self.variants.gi: m.warning(_("gobject introspection requires an executable " "prefix, 'gi' variant will be removed")) self.variants.gi = False You need Wine to actually run g-ir-scanner g-ir-compiler etc and build typelibs/girs, which is why it's disabled. (In reply to Nicolas Dufresne (stormer) from comment #37) > (In reply to Xavier Claessens from comment #35) > > ok so g-i never worked when cross compiling to windows, so that's correct. > > > > I needs windows-external-sdk from > > https://gitlab.collabora.com/xclaesse/windows-external-sdk > > gstreamer-sdk/ is read-only now, we'll clone this back into gstreamer/ > first, Tim will take care. So, let's not do all this? :)
(In reply to Nirbheek Chauhan from comment #38) > > gstreamer-sdk/ is read-only now, we'll clone this back into gstreamer/ > > first, Tim will take care. > > So, let's not do all this? :) We still need that Python 3.6 SDK for native Windows build. It's temporary until we use meson for g-i AFAIU.
(In reply to Xavier Claessens from comment #39) > (In reply to Nirbheek Chauhan from comment #38) > > > > gstreamer-sdk/ is read-only now, we'll clone this back into gstreamer/ > > > first, Tim will take care. > > > > So, let's not do all this? :) > > We still need that Python 3.6 SDK for native Windows build. It's temporary > until we use meson for g-i AFAIU. I'm saying that instead of doing that, we should copy the headers and DLLs from the Python installation, and the paths where you will find all that are: (In reply to Nirbheek Chauhan from comment #26) > Autotools will require some changes, but you can find the Python headers and > libs easily, see: > https://github.com/mesonbuild/meson/blob/ > 6f42f83867a52c1f9c61a2780dfd808a21984e41/mesonbuild/dependencies/misc. > py#L687. Perhaps copying from there makes sense instead of using the > outdated SDK. We can get rid of it when we move to Meson.
oh ok, I see! It's indeed much better. Why hasn't it always been like that?
(In reply to Xavier Claessens from comment #41) > oh ok, I see! It's indeed much better. Why hasn't it always been like that? No idea. You'd have to ask the folks who made it. Perhaps that's how a client needed it, or maybe there was some problem with older versions of Python 2. Everyone else builds against the system python, whether that's an MSYS2 Python or native Windows one.
I don't see how it can possibly work since 2012! That commit in g-i drop the support for env variables that cerbero sets: https://git.gnome.org/browse/gobject-introspection/commit/m4/python.m4?id=46ccb9dfe51372b0fc51c93890088853d0f1617e So I think we can just drop completely all that code.
(In reply to Xavier Claessens from comment #43) > I don't see how it can possibly work since 2012! That commit in g-i drop the > support for env variables that cerbero sets: > > https://git.gnome.org/browse/gobject-introspection/commit/m4/python. > m4?id=46ccb9dfe51372b0fc51c93890088853d0f1617e > > So I think we can just drop completely all that code. So weird. I know people are using GStreamer bindings through g-i on Windows, but I guess they don't use Cerbero for it? I checked, and we don't ship typelibs and girs in the Windows gstreamer binaries either. Let's drop g-i on Windows for Autotools then. I'll fix it with Meson.
Created attachment 365755 [details] [review] python3: Run 2to3 script to conver the whole codebase This is only the raw output of the script, no manual editing. The code is now python3 only.
Created attachment 365756 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
Created attachment 365757 [details] [review] python3: Remove unused SDK on Windows Those environment variables are not used by python.m4 since 2012. It is using python-config instead which is not present on Windows. so I don't think it has been working on Windows since then. See: https://git.gnome.org/browse/gobject-introspection/commit/m4/python.m4?id=46ccb9dfe51372b0fc51c93890088853d0f1617e
Created attachment 365758 [details] [review] python3: Update README
Created attachment 365760 [details] [review] python3: Run 2to3 script to convert the whole codebase This is only the raw output of the script, no manual editing. The code is now python3 only.
Created attachment 365761 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
Created attachment 365762 [details] [review] python3: Remove unused Windows SDK Those environment variables are not used by python.m4 since 2012. It is using python-config instead which is not present on Windows. so I don't think it has been working on Windows since then. See: https://git.gnome.org/browse/gobject-introspection/commit/m4/python.m4?id=46ccb9dfe51372b0fc51c93890088853d0f1617e
Created attachment 365763 [details] [review] python3: Update README
For the record, I successfully built on linux, cross-android-arm64, cross-win64 and on macosx. Slomo thinks we should wait for 1.14.0 to be released early next year first.
(In reply to Xavier Claessens from comment #53) > For the record, I successfully built on linux, cross-android-arm64, > cross-win64 and on macosx. > Nice! > Slomo thinks we should wait for 1.14.0 to be released early next year first. It would certainly give more time for people to test and complain about things we broken. :)
Created attachment 369911 [details] [review] python3: Run 2to3 script to conver the whole codebase This is only the raw output of the script, no manual editing. The code is now python3 only.
Created attachment 369912 [details] [review] python3: Manual fixing of errors left by 2to3 script Some of those fixes come from Ehsan Azar's branch.
Created attachment 369913 [details] [review] python3: Update README
Rebased on master, now that 1.14 got released.
I tested native build on Ubuntu 17.10, cross build for android arm64 and cross build for win64.
Review of attachment 369912 [details] [review]: Looks good. ::: cerbero/config.py @@ +538,3 @@ version = shell.check_call("perl -e 'print \"$]\";'") # FIXME: when perl's mayor is >= 10 + mayor = str(version[0]) I bet mayor is a typo here. Maybe a small follow up patch ?
Review of attachment 369911 [details] [review]: I've randomly looked at the changed and they seems ok. There is one with ugly exec(open(blabla)) somewhere, but I bet if we stumble across that we can come-up with something better later.
Review of attachment 369913 [details] [review]: ::: README @@ +66,1 @@ +The python.exe also needs to be renamed to python3.exe. As discuss, I think we need to think a little more about this one, that's quite ugly and might clash with user existing work.
(In reply to Nicolas Dufresne (stormer) from comment #62) > Review of attachment 369913 [details] [review] [review]: > > ::: README > @@ +66,1 @@ > +The python.exe also needs to be renamed to python3.exe. > > As discuss, I think we need to think a little more about this one, that's > quite ugly and might clash with user existing work. That's because the shebang is set to "python3" but on windows both python2 and python3 install python.exe. I guess the alternative is to invoke cerbero-uninstalled like: $ python ./cerbero-uninstalled But then we have to make sure that when cerbero invokes meson it will pass use argv[0] instead of "python3". I guess that's the best way to dodge all those PATH issues.
Created attachment 370216 [details] [review] python3: Update README
Review of attachment 370216 [details] [review]: d3dfaa3c47fd930a08f5496982c3eb97d3d17e15
Review of attachment 369912 [details] [review]: fa78642c4a7f16ff309dd24aed271032e33d8e0a
Review of attachment 369911 [details] [review]: 029a2a8d3b7e2024941792ac94d03d5dcb55a406
Keeping open, we'll follow up base on the CI report. I'll wipe each build after their first run in order to check full rebuild (and ensure clean state).
Seems ok, no ?
Yep.