* Patch: allow the 'python' used to run gentpl.py to be configured @ 2018-07-04 17:08 Adam Williamson 2018-07-06 17:25 ` Daniel Kiper 0 siblings, 1 reply; 8+ messages in thread From: Adam Williamson @ 2018-07-04 17:08 UTC (permalink / raw) To: grub-devel, pjones [-- Attachment #1: Type: text/plain, Size: 725 bytes --] 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 [-- Attachment #2: 0001-Make-python-interpreter-used-to-run-gentpl.py-config.patch --] [-- Type: text/x-patch, Size: 1887 bytes --] From 111e9d54c1c5bd57d65d09549831cb6c4a37f530 Mon Sep 17 00:00:00 2001 From: Adam Williamson <awilliam@redhat.com> Date: Wed, 4 Jul 2018 09:55:52 -0700 Subject: [PATCH] Make python interpreter used to run gentpl.py configurable 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. Signed-off-by: Adam Williamson <awilliam@redhat.com> --- conf/Makefile.common | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/conf/Makefile.common b/conf/Makefile.common index 311da61c6..a6e05cd91 100644 --- a/conf/Makefile.common +++ b/conf/Makefile.common @@ -5,6 +5,9 @@ CFLAGS_PLATFORM= export LC_COLLATE := C unexport LC_ALL +# default python is 'python', can be overridden +PYTHONBIN = python + # Platform specific options if COND_sparc64_ieee1275 LDFLAGS_PLATFORM = -Wl,-melf64_sparc @@ -128,11 +131,11 @@ BUILT_SOURCES = .PRECIOUS: $(top_srcdir)/Makefile.util.am $(top_srcdir)/Makefile.util.am: $(top_srcdir)/gentpl.py $(top_srcdir)/Makefile.util.def $(top_srcdir)/Makefile.utilgcry.def - python $^ > $@.new || (rm -f $@.new; exit 1) + $(PYTHONBIN) $^ > $@.new || (rm -f $@.new; exit 1) mv $@.new $@ .PRECIOUS: $(top_srcdir)/grub-core/Makefile.core.am $(top_srcdir)/grub-core/Makefile.core.am: $(top_srcdir)/gentpl.py $(top_srcdir)/grub-core/Makefile.core.def $(top_srcdir)/grub-core/Makefile.gcry.def if [ "x$$GRUB_CONTRIB" != x ]; then echo "You need to run ./autogen.sh manually." >&2; exit 1; fi - python $^ > $@.new || (rm -f $@.new; exit 1) + $(PYTHONBIN) $^ > $@.new || (rm -f $@.new; exit 1) mv $@.new $@ -- 2.18.0.rc2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-07-04 17:08 Patch: allow the 'python' used to run gentpl.py to be configured Adam Williamson @ 2018-07-06 17:25 ` Daniel Kiper 2018-07-06 20:19 ` Adam Williamson 0 siblings, 1 reply; 8+ messages in thread From: Daniel Kiper @ 2018-07-06 17:25 UTC (permalink / raw) To: awilliam; +Cc: pjones, grub-devel On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > 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. > > Signed-off-by: Adam Williamson <awilliam@redhat.com> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-07-06 17:25 ` Daniel Kiper @ 2018-07-06 20:19 ` Adam Williamson 2018-07-09 9:25 ` Daniel Kiper 2018-07-17 19:35 ` Adam Williamson 0 siblings, 2 replies; 8+ messages in thread From: Adam Williamson @ 2018-07-06 20:19 UTC (permalink / raw) To: Daniel Kiper; +Cc: pjones, grub-devel On Fri, 2018-07-06 at 19:25 +0200, Daniel Kiper wrote: > On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > > 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. > > > > Signed-off-by: Adam Williamson <awilliam@redhat.com> > > 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). > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-07-06 20:19 ` Adam Williamson @ 2018-07-09 9:25 ` Daniel Kiper 2018-07-17 19:35 ` Adam Williamson 1 sibling, 0 replies; 8+ messages in thread From: Daniel Kiper @ 2018-07-09 9:25 UTC (permalink / raw) To: Adam Williamson; +Cc: Daniel Kiper, pjones, grub-devel On Fri, Jul 06, 2018 at 01:19:18PM -0700, Adam Williamson wrote: > On Fri, 2018-07-06 at 19:25 +0200, Daniel Kiper wrote: > > On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > > > 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. > > > > > > Signed-off-by: Adam Williamson <awilliam@redhat.com> > > > > 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. > 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. > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-07-06 20:19 ` Adam Williamson 2018-07-09 9:25 ` Daniel Kiper @ 2018-07-17 19:35 ` Adam Williamson 2018-09-14 4:04 ` Adam Williamson 2018-09-19 12:43 ` Daniel Kiper 1 sibling, 2 replies; 8+ messages in thread From: Adam Williamson @ 2018-07-17 19:35 UTC (permalink / raw) To: The development of GNU GRUB, Daniel Kiper [-- Attachment #1: Type: text/plain, Size: 3076 bytes --] On Fri, 2018-07-06 at 13:19 -0700, Adam Williamson wrote: > On Fri, 2018-07-06 at 19:25 +0200, Daniel Kiper wrote: > > On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > > > 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. > > > > > > Signed-off-by: Adam Williamson <awilliam@redhat.com> > > > > 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 [-- Attachment #2: 0001-Use-AM_PATH_PYTHON-to-determine-interpreter-for-gent.patch --] [-- Type: text/x-patch, Size: 2918 bytes --] From 5fc0bb5f16a4189aa9581ebca4ea3abbc5c96593 Mon Sep 17 00:00:00 2001 From: Adam Williamson <awilliam@redhat.com> 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. Signed-off-by: Adam Williamson <awilliam@redhat.com> --- conf/Makefile.common | 4 ++-- configure.ac | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/conf/Makefile.common b/conf/Makefile.common index 311da61c6..f761f7224 100644 --- a/conf/Makefile.common +++ b/conf/Makefile.common @@ -128,11 +128,11 @@ BUILT_SOURCES = .PRECIOUS: $(top_srcdir)/Makefile.util.am $(top_srcdir)/Makefile.util.am: $(top_srcdir)/gentpl.py $(top_srcdir)/Makefile.util.def $(top_srcdir)/Makefile.utilgcry.def - python $^ > $@.new || (rm -f $@.new; exit 1) + $(PYTHON) $^ > $@.new || (rm -f $@.new; exit 1) mv $@.new $@ .PRECIOUS: $(top_srcdir)/grub-core/Makefile.core.am $(top_srcdir)/grub-core/Makefile.core.am: $(top_srcdir)/gentpl.py $(top_srcdir)/grub-core/Makefile.core.def $(top_srcdir)/grub-core/Makefile.gcry.def if [ "x$$GRUB_CONTRIB" != x ]; then echo "You need to run ./autogen.sh manually." >&2; exit 1; fi - python $^ > $@.new || (rm -f $@.new; exit 1) + $(PYTHON) $^ > $@.new || (rm -f $@.new; exit 1) mv $@.new $@ diff --git a/configure.ac b/configure.ac index c7888e40f..596f7c3f2 100644 --- a/configure.ac +++ b/configure.ac @@ -337,6 +337,7 @@ gl_EARLY AC_PROG_CXX AM_PROG_CC_C_O AM_PROG_AS +AM_PATH_PYTHON([2.6]) # Must be GCC. test "x$GCC" = xyes || AC_MSG_ERROR([GCC is required]) -- 2.18.0.rc2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-07-17 19:35 ` Adam Williamson @ 2018-09-14 4:04 ` Adam Williamson 2018-09-14 10:02 ` Daniel Kiper 2018-09-19 12:43 ` Daniel Kiper 1 sibling, 1 reply; 8+ messages in thread From: Adam Williamson @ 2018-09-14 4:04 UTC (permalink / raw) To: The development of GNU GRUB, Daniel Kiper On Tue, 2018-07-17 at 12:35 -0700, Adam Williamson wrote: > On Fri, 2018-07-06 at 13:19 -0700, Adam Williamson wrote: > > On Fri, 2018-07-06 at 19:25 +0200, Daniel Kiper wrote: > > > On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > > > > 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. > > > > > > > > Signed-off-by: Adam Williamson <awilliam@redhat.com> > > > > > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-09-14 4:04 ` Adam Williamson @ 2018-09-14 10:02 ` Daniel Kiper 0 siblings, 0 replies; 8+ messages in thread From: Daniel Kiper @ 2018-09-14 10:02 UTC (permalink / raw) To: Adam Williamson; +Cc: The development of GNU GRUB, Daniel Kiper On Thu, Sep 13, 2018 at 09:04:44PM -0700, Adam Williamson wrote: > On Tue, 2018-07-17 at 12:35 -0700, Adam Williamson wrote: > > On Fri, 2018-07-06 at 13:19 -0700, Adam Williamson wrote: > > > On Fri, 2018-07-06 at 19:25 +0200, Daniel Kiper wrote: > > > > On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > > > > > 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. > > > > > > > > > > Signed-off-by: Adam Williamson <awilliam@redhat.com> > > > > > > > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Patch: allow the 'python' used to run gentpl.py to be configured 2018-07-17 19:35 ` Adam Williamson 2018-09-14 4:04 ` Adam Williamson @ 2018-09-19 12:43 ` Daniel Kiper 1 sibling, 0 replies; 8+ messages in thread From: Daniel Kiper @ 2018-09-19 12:43 UTC (permalink / raw) To: Adam Williamson; +Cc: The development of GNU GRUB, Daniel Kiper On Tue, Jul 17, 2018 at 12:35:42PM -0700, Adam Williamson wrote: > On Fri, 2018-07-06 at 13:19 -0700, Adam Williamson wrote: > > On Fri, 2018-07-06 at 19:25 +0200, Daniel Kiper wrote: > > > On Wed, Jul 04, 2018 at 10:08:53AM -0700, Adam Williamson wrote: > > > > 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. > > > > > > > > Signed-off-by: Adam Williamson <awilliam@redhat.com> > > > > > > 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 > From: Adam Williamson <awilliam@redhat.com> > 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. > > Signed-off-by: Adam Williamson <awilliam@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Daniel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-09-19 12:43 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-04 17:08 Patch: allow the 'python' used to run gentpl.py to be configured Adam Williamson 2018-07-06 17:25 ` Daniel Kiper 2018-07-06 20:19 ` Adam Williamson 2018-07-09 9:25 ` Daniel Kiper 2018-07-17 19:35 ` Adam Williamson 2018-09-14 4:04 ` Adam Williamson 2018-09-14 10:02 ` Daniel Kiper 2018-09-19 12:43 ` Daniel Kiper
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).