search

kyamagu / skia-python

Python binding to Skia Graphics Library
https://kyamagu.github.io/skia-python/
BSD 3-Clause "New" or "Revised" License
246 stars 43 forks source link

`git-sync-deps` seems to no longer require or support Python 2 #245

Closed pavpanchekha closed 6 months ago

pavpanchekha commented 6 months ago

This PR removes the mention of Python 2 from the macOS build instructions.

I just ran through the installation instructions on macOS and tools/git-sync-deps in the Skia build instructions no longer need or allow Python 2. I am guessing the Windows and Linux build instructions also need to be updated; that said, I'm a bit worried to propose those because I haven't tested on those operating systems.

HinTak commented 6 months ago

You are correct - skia's git-sync-deps these days requires python 3 and we updated the build-script 9 months ago to explicitly use python 3: https://github.com/kyamagu/skia-python/commit/0da5cfd2d234d986396c72ecde33c872221a7d6e

I think "python" on CI mac runner is still python 2, (and python 2 is the default on older macs) so your update is not quite safe... it is probably best to remove those, and refer the users to scripts/build_* for build instructions, since that's how the pypi wheels are built.

HinTak commented 6 months ago

At a glance, it is quite outdated... scripts/build*.sh is current. Besides python 2 vs python3, skia v116+ requires c++ 17 instead of c++ 14 now. Ie. Needs a recent c++ compiler.

pavpanchekha commented 6 months ago

Feel free to close if the actual plan is to get rid of that page instead.

pavpanchekha commented 6 months ago

Or recommend the build scripts. I do believe I tried those and it didn't work, but that might have been a red herring.

HinTak commented 6 months ago

It is probably a bit more complicated than that - I assume you have an up-to-date mac os, for which python is python 3 (while python2 exists?). In somewhat older mac, python2 and python3 both exists, and python is python2. Apple is behind Linux and window's WSL by a few years on switching default python to python3.

I'd remove/update the build instructions and put more emphasis on the looking at the build scripts - which we use to build on somewhat old but still currently supported systems for max backwards compatibility. So they are the standard, but if you have a very up to date system, you might need to adapt.

HinTak commented 6 months ago

I think your pull is fine other than the python2 to python line - perhaps python3? Is "python3" available on your system? It sounds as if it isn't?

pavpanchekha commented 6 months ago

python3 is available on my system. I just re-built using the scripts and they seem to work; it was probably something unrelated.

HinTak commented 6 months ago

I have added some other python 2 to 3 changes, and a c++ 17 change, to the doc, and also cherry-pick a commit from #236 which fixes intermittent ci failures on windows, to let this finish ci, before merging.