Horton

Thread

Commits

  1. unaccent: Tweak value of PYTHON when building without Python support

  1. Could not run generate_unaccent_rules.py script when update unicode

    Japin Li <japinli@hotmail.com> — 2023-09-26T02:43:40Z

    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.

  2. Re: Could not run generate_unaccent_rules.py script when update unicode

    Michael Paquier <michael@paquier.xyz> — 2023-09-27T00:03:19Z

    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

  3. Re: Could not run generate_unaccent_rules.py script when update unicode

    Japin Li <japinli@hotmail.com> — 2023-09-27T01:15:00Z

    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.

  4. Re: Could not run generate_unaccent_rules.py script when update unicode

    Michael Paquier <michael@paquier.xyz> — 2023-09-27T05:46:01Z

    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

  5. Re: Could not run generate_unaccent_rules.py script when update unicode

    Japin Li <japinli@hotmail.com> — 2023-09-27T05:56:10Z

    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.