The first commit can be generally useful if e.g. Python is not generally found in the PATH, for selecting `python3` by default, or if the environment doesn't provide a generic `python` link but only versioned ones (like `python2` and `python3`).
The second one implements the suggestion from #2211 on top of this. I have no idea if that suggestion makes sense as I don't use Windows and don't have enough Python-on-Windows knowledge. @eht16 what do you think?
@novel-yet-trivial could you test this if you can build Geany? I didn't actually test it on Winodws so it properly working is only theoretical, although I'm quite confident. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/2223
-- Commit Summary --
* Add support for selecting the default Python command * Use the `py` launcher for Python filetype on Windows
-- File Changes --
M configure.ac (11) M data/Makefile.am (12) R data/filedefs/filetypes.python.in (4)
-- Patch Links --
https://github.com/geany/geany/pull/2223.patch https://github.com/geany/geany/pull/2223.diff
I just tested the changes and they work as expected (no surprise on a @b4n PR :D):
```ini [build-menu] # %f will be replaced by the complete filename # %e will be replaced by the filename without extension # (use only one of it at one time) FT_00_LB=_Compile FT_00_CM=py -m py_compile "%f" FT_00_WD= FT_02_LB=_Lint FT_02_CM=pep8 --max-line-length=80 "%f" FT_02_WD= error_regex=(.+):([0-9]+):([0-9]+) EX_00_LB=_Execute EX_00_CM=py "%f" EX_00_WD= ```
eht16 approved this pull request.
codebrainz approved this pull request.
Looks like an OK workaround for the silly Python installer. There are probably other tools that could use similar too (ex. `mingw32-make`); now there's a pattern to follow to add them later.
elextr commented on this pull request.
@@ -75,6 +74,10 @@ filetypes = \
filedefs/filetypes.yaml \ filedefs/filetypes.zephir
+# generated filetypes +filetypes_nodist = \ + filedefs/filetypes.python
shouldn't it be `filetypes.python.in` thats nodist?
codebrainz commented on this pull request.
@@ -75,6 +74,10 @@ filetypes = \
filedefs/filetypes.yaml \ filedefs/filetypes.zephir
+# generated filetypes +filetypes_nodist = \ + filedefs/filetypes.python
It needs `filetypes.python.in` to generate `filetypes.python` when the user of the dist configures it.
elextr commented on this pull request.
@@ -75,6 +74,10 @@ filetypes = \
filedefs/filetypes.yaml \ filedefs/filetypes.zephir
+# generated filetypes +filetypes_nodist = \ + filedefs/filetypes.python
oh, ok, it needs to be generated on the target
Tested again on Windows and it still works :). I suggest to merge.
N.B. The `py` launcher on Windows is a bit strange: it seems to try to auto-detect the desired Python version and it fails (at least sometimes). A simple Hello-World program using Python3 syntax can be compiled with `py -m py_compile` as the launcher chooses Python3 but executing it with `py "%f"` chooses Python2, for whatever reason. But since this happens also in the terminal, it is unrelated to this PR and Geany in general.
Does this make sense? It kind of does for distributions having versioned Pythons and moving away from Python2 (e.g. Debian IIUC), so they could simply use `--python-command=python3`, but I'm not positive how generally useful that is. However, it seems fairly harmless to have, and possibly useful.
What do you guys think?
@b4n well the only thing to do is what you have done, make it a Geany build time option. Any attempt at automatics is wrong since its on the build machine, not the execute machine.
Only thing is I would have made the default `python3`, IIRC the relevant Python PEP says `python` is not required but if present could point to either 2 or 3 (making it basically useless).
Only thing is I would have made the default `python3`, IIRC the relevant Python PEP says `python` is not required but if present could point to either 2 or 3 (making it basically useless).
I guess I was conservative with this PR not to change the current behavior, but it we want to change the default (to `python3`) that makes sense. It's kind of orthogonal to the change here though.
N.B. The `py` launcher on Windows is a bit strange: it seems to try to auto-detect the desired Python version and it fails (at least sometimes). A simple Hello-World program using Python3 syntax can be compiled with `py -m py_compile` as the launcher chooses Python3 but executing it with `py "%f"` chooses Python2, for whatever reason. But since this happens also in the terminal, it is unrelated to this PR and Geany in general.
Well, if it doesn't work maybe it's worse to use it than something less magical? I don't know what the Windows installer does, but if there is `python`, `python2` or `python3`, maybe we should pick one that is consistent in behavior instead of using `py` that apparently behaves randomly?
The only valid one now is `python3` since python 2 is officially EOLed.
I don't know what the Windows installer does...
<img width="543" alt="python_installer_step1" src="https://user-images.githubusercontent.com/181177/95642229-0873ae00-0a5c-11eb-9d28-2ba05647e84a.png">
By default it only puts `py.exe` (and `pyw.exe`) into `%PATH%` (specifically, in `C:\Windows`), and not the normal binaries. In the installation directory (presumably added to `%PATH%` if the non-default option shown is ticked), only includes binaries called `python.exe` and `pythonw.exe`.
@codebrainz approved this pull request.
Not tested but looks correct to use `py.exe`, having examined a default Windows installation.
I don't think it matters so much (for the default) about weird behaviour of `py.exe` with Python2 on Windows since anyone going forward who will be installing a dead/unsupported version is going to probably know enough to figure out how to customize the filetype/build-commands to use their special Python2 installation.
Yeah, see [PEP 394](https://www.python.org/dev/peps/pep-0394/) for Unix systems and [PEP 397](https://www.python.org/dev/peps/pep-0397/) specially for windows for an explanation.
My summary from those, use `python3` on unix and `py.exe` on windows.
And to be clear do _NOT_ use `python` as the default because it does not exist on some systems.
My summary from those, use `python3` on unix and `py.exe` on windows.
Pragmatically on non-Windows, `python` is probably still the best. I think it's pretty much installed everywhere, follows the distribution choice of actual version, supports older distros that didn't install `python2`/`python3` binaries, and matches the shebang in `main.py` template.
I think it's pretty much installed everywhere
The whole point of this issue is that it is not installed on Debian, which means also not on any derived distros.
The whole point of this issue is that it is not installed on Debian, which means also not on any derived distros.
The point of this issue is to: * Allow people compiling Geany to specify the Python binary. * Use the proper default binary on Windows by default.
I don't think it's meant to necessarily change the existing behaviour on non-Windows, just to allow more flexibility, given that some distros (like Mint 20 apparently) are not providing `python` binary/link, so they no longer have to patch Geany's build system, they can just update the package build options to specify `--with-python=python3`.
That said, one could open a new Issue/PR to change from `python` to `python3` in the `filetypes.python` file and `main.py` templates and such.
That said, one could open a new Issue/PR to change from python to python3 in the filetypes.python file and main.py templates and such.
No, it would have to change the default in the `<shudder>m4</shudder>` since this edits those files using the configured values. I'm just trying to save the effort of another PR as well, especially with a *release* coming we don't want the default to fail.
I don't feel strongly, either use `python` and it fails by default sometimes, or `python3` and fails by default sometimes. My personal opinion on how it _should be_; that distros provide `python` symlink to whichever sub version of Python 3.x (or even Python 2.x if olde-timey/LTS) that is ther "default" version, but alas `virtualenv` and friends exist for a reason.
Well, if we keep `python` better make a big song and dance in the release notes about `configure --with-python-command=` so packagers on no-python distros set it to `python3` otherwise there will be reports of it failing.
@elextr approved this pull request.
Irrespective of what the default python is, this needs to be merged pre-release
IMO we should merge this as is if the Windows part is OK, and change the default post-release to `python3` if we agree on it. The advantage I see with `python3` is that apparently it's gonna stay, and all distros I came across that support Python3 have that in path, even ye olde ones; but I think it's the kind of changes we should rather do early-ish -- e.g. right after the next release.
but I think it's the kind of changes we should rather do early-ish -- e.g. right after the next release.
Well, given this is proposed as the last GTK2 release, last Scintilla 3 release, last windows 32 bit release, changing Pythons will be the least of the changes in the next release :)
Merged #2223 into master.
@codebrainz commented on this pull request.
@@ -75,6 +74,10 @@ filetypes = \
filedefs/filetypes.yaml \ filedefs/filetypes.zephir
+# generated filetypes +filetypes_nodist = \ + filedefs/filetypes.python
I think `filetypes.python` should be added to `.gitignore` or such now.
@eht16 commented on this pull request.
@@ -75,6 +74,10 @@ filetypes = \
filedefs/filetypes.yaml \ filedefs/filetypes.zephir
+# generated filetypes +filetypes_nodist = \ + filedefs/filetypes.python
Done in 6ffaf515.
github-comments@lists.geany.org