Skip to content

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

Merged
HinTak merged 3 commits intoskia-python:mainfrom
pavpanchekha:patch-1
Apr 30, 2024
Merged

git-sync-deps seems to no longer require or support Python 2#245
HinTak merged 3 commits intoskia-python:mainfrom
pavpanchekha:patch-1

Conversation

@pavpanchekha
Copy link
Contributor

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
Copy link
Collaborator

HinTak commented Apr 29, 2024

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:
0da5cfd

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
Copy link
Collaborator

HinTak commented Apr 29, 2024

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
Copy link
Contributor Author

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

@pavpanchekha
Copy link
Contributor Author

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
Copy link
Collaborator

HinTak commented Apr 29, 2024

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
Copy link
Collaborator

HinTak commented Apr 29, 2024

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
Copy link
Contributor Author

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

HinTak added 2 commits April 30, 2024 11:01
Use python 3 for Windows and Linux, and requires c++17.
@HinTak
Copy link
Collaborator

HinTak commented Apr 30, 2024

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.

@HinTak HinTak merged commit 59f98a8 into skia-python:main Apr 30, 2024
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.

2 participants