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

Taskfile: Call 'python3' on non-Windows platforms #3752

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

mrpippy
Copy link
Contributor

@mrpippy mrpippy commented Nov 7, 2024

No description provided.

@OpenGOALBot
Copy link
Collaborator

Can one of the admins verify this patch?

Copy link
Member

@xTVaser xTVaser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes will break other environments in order for it to work properly on the default homebrew setup, you'd need to extract this into the Taskfile_darwin.yml file.

I'm not sure if this is definitely the right thing to do either, as even on macOS you can setup your python environment in a myriad of ways so that python is python3 (ie. pyenv and since monterary python2 is finally been removed). Considering it's the default for homebrew I'd merge it but the change has to be isolated from windows and linux.

@mrpippy
Copy link
Contributor Author

mrpippy commented Nov 10, 2024

Is there any environment where python3 doesn't launch Python 3? Out of all the environments I have handy (Gentoo Linux, FreeBSD, OpenBSD, macOS), python3 works on all of them.

Plain python only works on Gentoo, and I don't think that's universal for Linux distros either (doesn't Ubuntu need python-is-python3 installed?)

@VodBox
Copy link
Contributor

VodBox commented Nov 10, 2024

The default installation for Python on Windows does not add a python3 to the PATH. It does have the py launcher, where you can reliably call on Python 3.X with py -3, but I believe that is not a default on the Linux packages.

@xTVaser
Copy link
Member

xTVaser commented Nov 10, 2024

python is also what pyenv will use out of the box.

@xTVaser
Copy link
Member

xTVaser commented Nov 17, 2024

Do you plan to make the change or should this be closed?

@mrpippy
Copy link
Contributor Author

mrpippy commented Nov 19, 2024

I don't think using a Taskfile_darwin.yml is the right solution. This is an issue on many Linux/Unix OSes.

PEP 394 makes clear that distributors are free to not include a python, but python3 will always be present (excepting Windows). In addition, python may be Python 2--do any of the scripts work with that? python3 is obviously guaranteed to be Python 3.
If there's really a desire to use python, maybe a dynamic variable could be used which is set to python if it's present, and python3 if not? I haven't tested this yet to see if it's possible.

As for Windows, maybe that dynamic variable could be special cased to python.exe or py.exe?

@xTVaser
Copy link
Member

xTVaser commented Nov 19, 2024

using the Taskfile_{OS}.yml files is already how this exact problem is being handled successfully. For example calling our built binaries with a .exe extension or not. You just have to make it python3 for linux and macos, and python for windows. https://github.com/open-goal/jak-project/tree/master/scripts/tasks

There is a desire to use python unfortunately because otherwise it will break existing windows workflows, there are several people that work on the project from a windows environment.

@mrpippy mrpippy changed the title scripts: Have tasks call 'python3' instead of 'python' for Mac compatibility Taskfile: Call 'python3' on non-Windows platforms Nov 19, 2024
@mrpippy
Copy link
Contributor Author

mrpippy commented Nov 19, 2024

Ah I hadn't noticed the scripts/tasks/Taskfile_ files. I moved those vars into the main file (to remove the Mac/Linux duplication), and the Windows file overrides them.

@xTVaser
Copy link
Member

xTVaser commented Nov 19, 2024

Can you revert the removal? I'd rather there be a bit of duplication so that they exist when things need to be different between the two environments and so that it is obvious to contributors without having to dig through the taskfile documentation.

If someone on linux/macOS wanted or needed to change these values, they would have to copy them into the respective file.

It would then be potentially confusing which variable is the source of truth (the OS file wins).

Avoid this unneeded confusion, having the variables copied into the darwin and linux file is not a big deal.
@xTVaser xTVaser merged commit 3f7aca3 into open-goal:master Nov 25, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants