Use the system-configured version of Python 3 when building

[pR0Ps]

This fixes a build issue on systems where Python isn't installed as /usr/bin/python3

[github-actions]

Release Notes Preview

Fixed

• Fixed regression in hs.json which meant it sometimes returned a string instead of a table
• A possible crash in hs.webview:evaluateJavaScript() is fixed
• A possible crash in hs.task has been guarded against

Added

• Add hs.audiodevice.defaultEffectDevice() and hs.audiodevice:setDefaultEffectDevice()

Other:

[github-actions]

This pull request does not contain a valid label. Please add one of the following labels: pr-fix, pr-change, pr-feature, pr-maintenance

[cmsj]

I certainly understand the motivation behind this change, but it's a bit more subtle than just changing those two files - we call Python/pip in various places throughout build/docbuild/test/CI and if we were to change away from the system Python, we'd need to verify that all of those sites are consistent, and that there are no surprises from Homebrew/other Pythons being installed/upgraded over time.

Personally I would prefer to stick with the default system Python 3 - I recognise that it limits the versions of macOS that can be used to build Hammerspoon, but I suspect that making everything work reliably with whatever Python happens to be first in $PATH at any given moment, is going to be harder than it looks.

[pR0Ps]

Thanks for the explanation, it makes sense to use the known-good /usr/bin/python3 for the build. If possible I'd like to improve the case where it doesn't exist since the current error message is a bit misleading.

On my system (macOS Mojave) I don't have a /usr/bin/python3 binary. I installed Python 3 using MacPorts which installs it as /opt/local/bin/python3.

When building, this is the output I saw:

+ rm -f build/docs.json
make: scripts/docs/bin/build_docs.py: No such file or directory
make: *** [build/docs.json] Error 1
Command PhaseScriptExecution failed with a nonzero exit code

The scripts/docs/bin/build_docs.py: No such file or directory error is referring to the interpreter specified in it's shebang line not being found, not the script being missing or an error from within the script. In hindsight this is pretty obvious, but it took me a bit of digging to figure out.

Would an HAMMERSPOON_PYTHON3 env variable to override the python version used it be a good idea? Or maybe just a more obvious check? For example, if [ -e /usr/bin/python3 ] was added before the calls to scripts/docs/bin/build_docs.py in the Makefile it would error out on that line instead.

Feel free to close this out if you think it's fine the way it is - it's really not that bad to work around it.

[cmsj]

I just had a read of the env manpage and it looks like we might actually be able to make this work, somewhat, with:

#!/usr/bin/env -P/usr/bin:${PATH} python3

That will override $PATH to use the system python3 if it exists, otherwise rely on $PATH.

It's not perfect, there are still some pip related ways things could go wrong, but it won't affect CI or my release builds, so I'd be happy to merge a change like that.

[pR0Ps]

I just tried this and unfortunately it doesn't seem to work. It looks like environment variables can't be expanded in the shebang line.

[cmsj]

hmm, yeah. #!/usr/bin/env -S-P/usr/bin:${PATH} python3 seems to work though

[pR0Ps]

Huh, I've never seen that before but it seems to work. I've updated the PR to use this method, as well as mention that Python 3 needs to be installed in CONTRIBUTING.md

[codecov]

Codecov Report

Merging #2798 (a2d9366) into master (005d208) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2798      +/-   ##
==========================================
- Coverage   27.24%   27.23%   -0.02%     
==========================================
  Files         179      179              
  Lines       36063    36063              
==========================================
- Hits         9826     9822       -4     
- Misses      26237    26241       +4     

[cmsj]

Thanks for following up on this!

[github-actions]

View Test Results

    1 files  ±0    27 suites  ±0   4m 55s ⏱️ ±0s
330 tests ±0  330 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 91fe6cf. ± Comparison against base commit 91fe6cf.