Discussion:
Patch: allow the 'python' used to run gentpl.py to be configured
(too old to reply)
Adam Williamson
2018-07-04 17:08:53 UTC
Permalink
A patch from Jonathan McCune back in 2015 allowed build-time
configuration of the python interpreter used in autogen.sh. However,
this doesn't cover the whole build process, as we still have use of
'python' hardcoded into Makefile.common when running gentpl.py (the
script's own shebang is ignored). This patch allows you to set the make
variable 'PYTHONBIN' to change the interpreter used to run gentpl.py.
With this, it's possible to build grub on Fedora with 'PYTHON=python3
autogen.sh' and 'make PYTHONBIN=python3', without python2 installed at
all, and the build succeeds.
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net
Daniel Kiper
2018-07-06 17:25:45 UTC
Permalink
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
And there are more references to the python binary in other makefiles
which should be fixed too.

Daniel
Adam Williamson
2018-07-06 20:19:18 UTC
Permalink
Post by Daniel Kiper
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
That's possible, but it depends what you mean by "correct", doesn't it?
There could be many python interpreters installed on a system; which
are we to assume is "correct"?

We could make it configurable with some sort of default heuristic, I
guess, I was just going for a simple patch approximately in line with
what was done for autogen.sh for now. (And I wouldn't want to reinvent
python interpreter discovery, which has been invented enough times
already; if we were to go that route it'd probably make sense to use an
existing autoconf extension or something).
Post by Daniel Kiper
And there are more references to the python binary in other makefiles
which should be fixed too.
I can't find any...could those perhaps be in files generated from the
ones actually in the git repo? I grepped the whole of a clean git
checkout for 'python' and these were all I found, and with this patch
(as mentioned) I can successfully build grub using python3 on a system
with no 'python' executable at all.
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net
Daniel Kiper
2018-07-09 09:25:47 UTC
Permalink
Post by Adam Williamson
Post by Daniel Kiper
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
That's possible, but it depends what you mean by "correct", doesn't it?
There could be many python interpreters installed on a system; which
are we to assume is "correct"?
Yep. I think that we should start with python name and if that does not
work then look for another common names.
Post by Adam Williamson
We could make it configurable with some sort of default heuristic, I
guess, I was just going for a simple patch approximately in line with
what was done for autogen.sh for now. (And I wouldn't want to reinvent
python interpreter discovery, which has been invented enough times
already; if we were to go that route it'd probably make sense to use an
existing autoconf extension or something).
This is what I was thinking about. Reinventing the wheel does not make sense.
Post by Adam Williamson
Post by Daniel Kiper
And there are more references to the python binary in other makefiles
which should be fixed too.
I can't find any...could those perhaps be in files generated from the
ones actually in the git repo? I grepped the whole of a clean git
checkout for 'python' and these were all I found, and with this patch
(as mentioned) I can successfully build grub using python3 on a system
with no 'python' executable at all.
Ugh... My fault, sorry. It looks that "make distclean" does not work well.
It have to be fixed.

Daniel
Adam Williamson
2018-07-17 19:35:42 UTC
Permalink
Post by Adam Williamson
Post by Daniel Kiper
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
That's possible, but it depends what you mean by "correct", doesn't it?
There could be many python interpreters installed on a system; which
are we to assume is "correct"?
We could make it configurable with some sort of default heuristic, I
guess, I was just going for a simple patch approximately in line with
what was done for autogen.sh for now. (And I wouldn't want to reinvent
python interpreter discovery, which has been invented enough times
already; if we were to go that route it'd probably make sense to use an
existing autoconf extension or something).
So I've just been looking into this, and it's actually very easy to do
using AM_PATH_PYTHON, provided by autoconf. However, there's a rather
funny wrinkle for grub's particular build process. AM_PATH_PYTHON has a
documented behaviour where, if the 'PYTHON' env var is set when
autoconf is run, it will *only* consider the value that env var is set
to as a candidate for the Python interpreter - it won't do its usual
attempt to discover one. And grub's autogen.sh already uses the PYTHON
env var - setting it if it was not set already - and then calls
autoreconf!

So the practical effect of just applying this patch would be that only
'python', or whatever the env var PYTHON is set to when running
autogen.sh, would be considered, when using autogen.sh. All the
'discovery' bits of AM_PATH_PYTHON wouldn't be used: if you just call
'autogen.sh' without setting PYTHON, but your Python binary is called
'python3' or 'python2.7' or whatever, it won't work.

This seems a bit funny, but I'm not sure if it's really a
*problem*...because autogen.sh *itself* will only work if whatever
value PYTHON is set to is a working Python interpreter, anyway. We
could in fact argue that the consequence is quite nice as it makes
everything sort of consistent: whatever PYTHON is set to will be the
interpreter used both by autogen.sh and for running gentpl.py. But I
figured I should point it out.

With that in mind, I'm attaching the updated patch in git format.
Thanks!
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net
Adam Williamson
2018-09-14 04:04:44 UTC
Permalink
Post by Adam Williamson
Post by Adam Williamson
Post by Daniel Kiper
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
That's possible, but it depends what you mean by "correct", doesn't it?
There could be many python interpreters installed on a system; which
are we to assume is "correct"?
We could make it configurable with some sort of default heuristic, I
guess, I was just going for a simple patch approximately in line with
what was done for autogen.sh for now. (And I wouldn't want to reinvent
python interpreter discovery, which has been invented enough times
already; if we were to go that route it'd probably make sense to use an
existing autoconf extension or something).
So I've just been looking into this, and it's actually very easy to do
using AM_PATH_PYTHON, provided by autoconf. However, there's a rather
funny wrinkle for grub's particular build process. AM_PATH_PYTHON has a
documented behaviour where, if the 'PYTHON' env var is set when
autoconf is run, it will *only* consider the value that env var is set
to as a candidate for the Python interpreter - it won't do its usual
attempt to discover one. And grub's autogen.sh already uses the PYTHON
env var - setting it if it was not set already - and then calls
autoreconf!
So the practical effect of just applying this patch would be that only
'python', or whatever the env var PYTHON is set to when running
autogen.sh, would be considered, when using autogen.sh. All the
'discovery' bits of AM_PATH_PYTHON wouldn't be used: if you just call
'autogen.sh' without setting PYTHON, but your Python binary is called
'python3' or 'python2.7' or whatever, it won't work.
This seems a bit funny, but I'm not sure if it's really a
*problem*...because autogen.sh *itself* will only work if whatever
value PYTHON is set to is a working Python interpreter, anyway. We
could in fact argue that the consequence is quite nice as it makes
everything sort of consistent: whatever PYTHON is set to will be the
interpreter used both by autogen.sh and for running gentpl.py. But I
figured I should point it out.
With that in mind, I'm attaching the updated patch in git format.
Thanks!
Ping? I never received any follow up to this.
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net
Daniel Kiper
2018-09-14 10:02:31 UTC
Permalink
Post by Adam Williamson
Post by Adam Williamson
Post by Adam Williamson
Post by Daniel Kiper
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
That's possible, but it depends what you mean by "correct", doesn't it?
There could be many python interpreters installed on a system; which
are we to assume is "correct"?
We could make it configurable with some sort of default heuristic, I
guess, I was just going for a simple patch approximately in line with
what was done for autogen.sh for now. (And I wouldn't want to reinvent
python interpreter discovery, which has been invented enough times
already; if we were to go that route it'd probably make sense to use an
existing autoconf extension or something).
So I've just been looking into this, and it's actually very easy to do
using AM_PATH_PYTHON, provided by autoconf. However, there's a rather
funny wrinkle for grub's particular build process. AM_PATH_PYTHON has a
documented behaviour where, if the 'PYTHON' env var is set when
autoconf is run, it will *only* consider the value that env var is set
to as a candidate for the Python interpreter - it won't do its usual
attempt to discover one. And grub's autogen.sh already uses the PYTHON
env var - setting it if it was not set already - and then calls
autoreconf!
So the practical effect of just applying this patch would be that only
'python', or whatever the env var PYTHON is set to when running
autogen.sh, would be considered, when using autogen.sh. All the
'discovery' bits of AM_PATH_PYTHON wouldn't be used: if you just call
'autogen.sh' without setting PYTHON, but your Python binary is called
'python3' or 'python2.7' or whatever, it won't work.
This seems a bit funny, but I'm not sure if it's really a
*problem*...because autogen.sh *itself* will only work if whatever
value PYTHON is set to is a working Python interpreter, anyway. We
could in fact argue that the consequence is quite nice as it makes
everything sort of consistent: whatever PYTHON is set to will be the
interpreter used both by autogen.sh and for running gentpl.py. But I
figured I should point it out.
With that in mind, I'm attaching the updated patch in git format.
Thanks!
Ping? I never received any follow up to this.
Yours and some other patches are on my TODO list for next week. Sorry for delay.

Daniel
Daniel Kiper
2018-09-19 12:43:22 UTC
Permalink
Post by Adam Williamson
Post by Adam Williamson
Post by Daniel Kiper
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to allow
a make variable PYTHONBIN to be set to the desired interpreter.
This will make it easier in situations where we specifically
want to build with 'python2' or 'python3' or whatever.
Thanks for the patch. However, I think that the configure should find
correct python binary and set PYTHON variable (instead of PYTHONBIN)
in the Makefile (good example is BUILD_CC variable in Makefile.in).
That's possible, but it depends what you mean by "correct", doesn't it?
There could be many python interpreters installed on a system; which
are we to assume is "correct"?
We could make it configurable with some sort of default heuristic, I
guess, I was just going for a simple patch approximately in line with
what was done for autogen.sh for now. (And I wouldn't want to reinvent
python interpreter discovery, which has been invented enough times
already; if we were to go that route it'd probably make sense to use an
existing autoconf extension or something).
So I've just been looking into this, and it's actually very easy to do
using AM_PATH_PYTHON, provided by autoconf. However, there's a rather
funny wrinkle for grub's particular build process. AM_PATH_PYTHON has a
documented behaviour where, if the 'PYTHON' env var is set when
autoconf is run, it will *only* consider the value that env var is set
to as a candidate for the Python interpreter - it won't do its usual
attempt to discover one. And grub's autogen.sh already uses the PYTHON
env var - setting it if it was not set already - and then calls
autoreconf!
So the practical effect of just applying this patch would be that only
'python', or whatever the env var PYTHON is set to when running
autogen.sh, would be considered, when using autogen.sh. All the
'discovery' bits of AM_PATH_PYTHON wouldn't be used: if you just call
'autogen.sh' without setting PYTHON, but your Python binary is called
'python3' or 'python2.7' or whatever, it won't work.
This seems a bit funny, but I'm not sure if it's really a
*problem*...because autogen.sh *itself* will only work if whatever
value PYTHON is set to is a working Python interpreter, anyway. We
could in fact argue that the consequence is quite nice as it makes
everything sort of consistent: whatever PYTHON is set to will be the
interpreter used both by autogen.sh and for running gentpl.py. But I
figured I should point it out.
With that in mind, I'm attaching the updated patch in git format.
Thanks!
From 5fc0bb5f16a4189aa9581ebca4ea3abbc5c96593 Mon Sep 17 00:00:00 2001
Date: Wed, 4 Jul 2018 09:55:52 -0700
Subject: [PATCH] Use AM_PATH_PYTHON to determine interpreter for gentpl.py
gentpl.py is python2/3-agnostic, but there's no way to cause it
to be run with any interpreter other than 'python', it's just
hard-coded into Makefile.common that way. Adjust that to use
AM_PATH_PYTHON (provided by automake) to find an interpreter
and run gentpl.py with that instead. This makes grub buildable
when `python` does not exist (but rather `python3` or `python2`
or `python2.7`, etc.) Minimum version is set to 2.6 as this is
the first version with `__future__.print_function` available.
Note, AM_PATH_PYTHON respects the PYTHON environment variable
and will treat its value as the *only* candidate for a valid
interpreter if it is set - when PYTHON is set, AM_PATH_PYTHON
will not try to find any alternative interpreter, it will only
check whether the interpreter set as the value of PYTHON meets
the requirements and use it if so or fail if not. This means
that when using grub's `autogen.sh`, as it too uses the value
of the PYTHON environment variable (and if it is not set, just
sets it to 'python') you cannot rely on AM_PATH_PYTHON
interpreter discovery. If your desired Python interpreter is
not just 'python', you must set the PYTHON environment variable,
e.g. 'PYTHON=/usr/local/bin/python3 ./autogen.sh'. The specified
interpreter will then be used both by autogen.sh itself and by
the autotools-driven build scripts.
Reviewed-by: Daniel Kiper <***@oracle.com>

Daniel

Loading...