Обсуждение: Could not run generate_unaccent_rules.py script when update unicode
Hi, hackers When I try to update unicode mapping tables using make update-unicode [1], I encountered an error about following: generate_unaccent_rules.py --unicode-data-file ../../src/common/unicode/UnicodeData.txt --latin-ascii-file Latin-ASCII.xml>unaccent.rules /bin/sh: 1: generate_unaccent_rules.py: not found make: *** [Makefile:33: unaccent.rules] Error 127 make: *** Deleting file 'unaccent.rules' The generate_unaccent_rules.py is in contrib/unaccent and the Makefile: # Allow running this even without --with-python PYTHON ?= python $(srcdir)/unaccent.rules: generate_unaccent_rules.py ../../src/common/unicode/UnicodeData.txt Latin-ASCII.xml $(PYTHON) $< --unicode-data-file $(word 2,$^) --latin-ascii-file $(word 3,$^) >$@ It use python to run generate_unaccent_rules.py, However, the ?= operator in Makefile only check variable is defined or not, but do not check variable is empty. Since the PYTHON is defined in src/Makefile.global, so here PYTHON get empty when without --with-ptyhon. Here are some examples: japin@coltd-devel:~$ cat Makefile PYTHON = PYTHON ?= python test: echo '$(PYTHON)' japin@coltd-devel:~$ make echo '' japin@coltd-devel:~$ cat Makefile PYTHON = python3 PYTHON ?= python test: echo '$(PYTHON)' japin@coltd-devel:~$ make echo 'python3' python3 japin@coltd-devel:~$ cat Makefile PYTHON = ifeq ($(PYTHON),) PYTHON = python endif test: echo '$(PYTHON)' japin@coltd-devel:~$ make echo 'python' python japin@coltd-devel:~$ cat Makefile PYTHON = python3 ifeq ($(PYTHON),) PYTHON = python endif test: echo '$(PYTHON)' japin@coltd-devel:~$ make echo 'python3' python3 Here is a patch to fix this, any thoughts? diff --git a/contrib/unaccent/Makefile b/contrib/unaccent/Makefile index 652a3e774c..3ff49ba1e9 100644 --- a/contrib/unaccent/Makefile +++ b/contrib/unaccent/Makefile @@ -26,7 +26,9 @@ endif update-unicode: $(srcdir)/unaccent.rules # Allow running this even without --with-python -PYTHON ?= python +ifeq ($(PYTHON),) +PYTHON = python +endif $(srcdir)/unaccent.rules: generate_unaccent_rules.py ../../src/common/unicode/UnicodeData.txt Latin-ASCII.xml $(PYTHON) $< --unicode-data-file $(word 2,$^) --latin-ascii-file $(word 3,$^) >$@ [1] https://www.postgresql.org/message-id/MEYP282MB1669AC78EE8374B3DE797A09B6FCA%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li ChengDu WenWu Information Technology Co., Ltd.
On Tue, Sep 26, 2023 at 10:43:40AM +0800, Japin Li wrote: > # Allow running this even without --with-python > PYTHON ?= python > > $(srcdir)/unaccent.rules: generate_unaccent_rules.py ../../src/common/unicode/UnicodeData.txt Latin-ASCII.xml > $(PYTHON) $< --unicode-data-file $(word 2,$^) --latin-ascii-file $(word 3,$^) >$@ > > It use python to run generate_unaccent_rules.py, However, the ?= operator in > Makefile only check variable is defined or not, but do not check variable is > empty. Since the PYTHON is defined in src/Makefile.global, so here PYTHON > get empty when without --with-ptyhon. I am not sure that many people run this script frequently so that may not be worth adding a check for a defined, still empty or incorrect value, but.. If you were to change the Makefile we use in this path, how are you suggesting to change it? -- Michael
Вложения
On Wed, 27 Sep 2023 at 08:03, Michael Paquier <michael@paquier.xyz> wrote: > On Tue, Sep 26, 2023 at 10:43:40AM +0800, Japin Li wrote: >> # Allow running this even without --with-python >> PYTHON ?= python >> >> $(srcdir)/unaccent.rules: generate_unaccent_rules.py ../../src/common/unicode/UnicodeData.txt Latin-ASCII.xml >> $(PYTHON) $< --unicode-data-file $(word 2,$^) --latin-ascii-file $(word 3,$^) >$@ >> >> It use python to run generate_unaccent_rules.py, However, the ?= operator in >> Makefile only check variable is defined or not, but do not check variable is >> empty. Since the PYTHON is defined in src/Makefile.global, so here PYTHON >> get empty when without --with-ptyhon. > > I am not sure that many people run this script frequently so that may > not be worth adding a check for a defined, still empty or incorrect Yeah, not frequently, however, it already be used by me, since we provide this function, why not make it better? > value, but.. If you were to change the Makefile we use in this path, > how are you suggesting to change it? I provide a patch at bottom of in [1]. Attached here again. diff --git a/contrib/unaccent/Makefile b/contrib/unaccent/Makefile index 652a3e774c..3ff49ba1e9 100644 --- a/contrib/unaccent/Makefile +++ b/contrib/unaccent/Makefile @@ -26,7 +26,9 @@ endif update-unicode: $(srcdir)/unaccent.rules # Allow running this even without --with-python -PYTHON ?= python +ifeq ($(PYTHON),) +PYTHON = python +endif $(srcdir)/unaccent.rules: generate_unaccent_rules.py ../../src/common/unicode/UnicodeData.txt Latin-ASCII.xml $(PYTHON) $< --unicode-data-file $(word 2,$^) --latin-ascii-file $(word 3,$^) >$@ [1] https://www.postgresql.org/message-id/MEYP282MB1669F86C0DC7B4DC48489CB0B6C3A@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li ChengDu WenWu Information Technology Co., Ltd.
On Wed, Sep 27, 2023 at 09:15:00AM +0800, Japin Li wrote: > On Wed, 27 Sep 2023 at 08:03, Michael Paquier <michael@paquier.xyz> wrote: >> I am not sure that many people run this script frequently so that may >> not be worth adding a check for a defined, still empty or incorrect > > Yeah, not frequently, however, it already be used by me, since we provide this > function, why not make it better? Well, I don't mind doing as you suggest. >> value, but.. If you were to change the Makefile we use in this path, >> how are you suggesting to change it? > > I provide a patch at bottom of in [1]. Attached here again. No file was attached so I somewhat missed it. And indeed, you're right. The current rule is useless when compiling without --with-python, as PYTHON is empty but defined. What you are proposing is similar to what pgxs.mk does for bison and flex, and "python" would still be able to map to python3 or python2.7, which should be OK in most cases. I thought that this was quite old, but no, f85a485f8 was at the origin of that. So I've applied the patch down to 13. -- Michael
Вложения
On Wed, 27 Sep 2023 at 13:46, Michael Paquier <michael@paquier.xyz> wrote: > On Wed, Sep 27, 2023 at 09:15:00AM +0800, Japin Li wrote: >> On Wed, 27 Sep 2023 at 08:03, Michael Paquier <michael@paquier.xyz> wrote: >>> I am not sure that many people run this script frequently so that may >>> not be worth adding a check for a defined, still empty or incorrect >> >> Yeah, not frequently, however, it already be used by me, since we provide this >> function, why not make it better? > > Well, I don't mind doing as you suggest. > >>> value, but.. If you were to change the Makefile we use in this path, >>> how are you suggesting to change it? >> >> I provide a patch at bottom of in [1]. Attached here again. > > No file was attached so I somewhat missed it. And indeed, you're > right. The current rule is useless when compiling without > --with-python, as PYTHON is empty but defined. What you are proposing > is similar to what pgxs.mk does for bison and flex, and "python" would > still be able to map to python3 or python2.7, which should be OK in > most cases. > > I thought that this was quite old, but no, f85a485f8 was at the origin > of that. So I've applied the patch down to 13. Thanks for your review and push! -- Regrads, Japin Li ChengDu WenWu Information Technology Co., Ltd.