Re: Don't treat Apple's new Citrus/FreeBSD-based iconv like GNU libiconv

[Bruno Haible]

On macOS 12.5, I'm seeing this test failure:

$ ./test-striconveha
../../tests/test-striconveha.c:426: assertion 'retval == 0' failed

Apparently the transliteration of /usr/lib/libiconv.2.dylib does
not work like the GNU libiconv one.
Either this is a copy of GNU libiconv 1.11 with modified transliteration,
or it's a backport of the bastard Apple iconv that they deploy on newer
macOS releases since fall 2023.

This patch avoids the failure.


2024-02-24  Bruno Haible  

striconveha tests: Avoid test failure on macOS 12.5.
* tests/test-striconveha.c (main): Skip transliteration tests when using
Apple's modified GNU libiconv or the bastard Apple iconv.

diff --git a/tests/test-striconveha.c b/tests/test-striconveha.c
index c401177e29..376f3cb603 100644
--- a/tests/test-striconveha.c
+++ b/tests/test-striconveha.c
@@ -406,7 +406,7 @@ main ()
 }
 # endif
 
-# if (((__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2) || __GLIBC__ > 2) && !defined 
__UCLIBC__) || _LIBICONV_VERSION >= 0x0105
+# if (((__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2) || __GLIBC__ > 2) && !defined 
__UCLIBC__) || (_LIBICONV_VERSION >= 0x0105 && !(_LIBICONV_VERSION == 0x10b && 
defined __APPLE__))
   /* Test conversion from UTF-8 to ISO-8859-1 with transliteration.  */
   for (h = 0; h < SIZEOF (handlers); h++)
 {
@@ -586,7 +586,7 @@ main ()
 }
 # endif
 
-# if (((__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2) || __GLIBC__ > 2) && !defined 
__UCLIBC__) || _LIBICONV_VERSION >= 0x0105
+# if (((__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2) || __GLIBC__ > 2) && !defined 
__UCLIBC__) || (_LIBICONV_VERSION >= 0x0105 && !(_LIBICONV_VERSION == 0x10b && 
defined __APPLE__))
   /* Test conversion from UTF-8 to ISO-8859-1 with transliteration.  */
   for (h = 0; h < SIZEOF (handlers); h++)
 {

Re: gnulib-tool.py: Follow gnulib-tool changes, part 27.

[Bruno Haible]

Hi Collin,

> > I.e. you meant to write
> >   mode != None
> > not
> >   modules != None
> > ?
> 
> The second fixes this typo. Thanks for noticing it.

But there's another typo in the same line: The original code

  case $mode,$gnu_make in
*test*,true)
  echo "gnulib-tool: --gnu-make not supported when including tests"
  func_exit 1;;
  esac

has the intent to match the $mode values
  create-testdir
  create-megatestdir
  test
  megatest
But these do not contain the substring "tests".

Also, please can we stick with the syntax 'foo', not "foo", for literal
strings. Outside of gnulib, both syntaxes seem to be in use. But when I
search where the literal string 'foo' occurs, I don't want to make 2
searches (for 'foo' and for "foo"), nor a regex search (for ['"]foo['"]).
Simple things should remain simple. A certain canonical way to denote
literal strings is necessary for this. (Btw, JavaScript has the same
problem.)


2024-02-24  Bruno Haible  

gnulib-tool.py: Further fix last commit.
* gnulib-tool.py (main): Make the mode test match for 'create-testdir',
'create-megatestdir', 'test', 'megatest'.

diff --git a/gnulib-tool.py b/gnulib-tool.py
index e168e8fc91..1df790c496 100755
--- a/gnulib-tool.py
+++ b/gnulib-tool.py
@@ -607,7 +607,7 @@ def main():
 if cmdargs.pobase == None and cmdargs.podomain != None:
 message = '%s: warning: --po-domain has no effect without a --po-base 
option\n' % constants.APP['name']
 sys.stderr.write(message)
-if  mode != None and "tests" in mode and gnu_make:
+if mode != None and 'test' in mode and gnu_make:
 message = '%s: --gnu-make not supported when including tests\n' % 
constants.APP['name']
 sys.stderr.write(message)
 sys.exit(1)

Re: gnulib-tool.py: Follow gnulib-tool changes, part 28.

[Bruno Haible]

Collin Funk wrote:
> So the --avoid modules are emitted in the order they are passed to
> gnulib-tool, but the actual modules will be alphabetically sorted.
> Therefore, I think the correct code would be:
> 
> if len(avoids) > 0:
> actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in avoids])
> if len(modules) > 0:
> actioncmd += ''.join([f" \\\n#  {x}" for x in sorted(modules)])

OK. For the --avoid options, things are clear now.

Regarding the modules to include:
> Here is the diff with the two sorted instructions removed.

So, it looks like the original gnulib-tool does some sorting of the
module names that gnulib-tool.py does not do yet, in some earlier
processing steps. Since this can affect other parts of the output,
it would be good to have the sorting at the same processing stage.

None of the 'sort' invocations in gnulib-tool are covered by an
entry in the gnulib-tool.py.TODO file. Therefore the most promising
approach to finding the cause of the difference is to
  - go through all 'sort' invocations in gnulib-tool,
  - find the corresponding place in pygnulib/ and see whether
sorting happens there as well or has been forgotten.

Bruno

Re: gnulib-tool.py: Follow gnulib-tool changes, part 28.

[Collin Funk]

Hi Bruno,

On 2/24/24 5:25 PM, Bruno Haible wrote:
> None of the 'sort' invocations in gnulib-tool are covered by an
> entry in the gnulib-tool.py.TODO file. Therefore the most promising
> approach to finding the cause of the difference is to
>   - go through all 'sort' invocations in gnulib-tool,
>   - find the corresponding place in pygnulib/ and see whether
> sorting happens there as well or has been forgotten.

Thank you for the advice and for looking over all of these patches. I
think that this patch should be a solution to the issue as long as I
am understanding gnulib-tool correctly.

>From what I could understand, it seems that upon seeing that the mode
is "import", the specified_modules is set with the unsorted list of
modules [1]. Then that variable is sorted before it is used [2]. Could
you confirm that I am understanding this correctly?

If so, I feel that this patch should be the correct way to handle it.
My initial ideas where to sort the modules in main() before the
GLConfig object was created, or to modify the GLConfig class's methods
which are used to set the modules in use. I think the second option is
less likely to be broken by future changes. The changes involve using
a set and sorted() to substitute `sort -u'.

[1] https://git.savannah.gnu.org/cgit/gnulib.git/tree/gnulib-tool#n5085
[2] https://git.savannah.gnu.org/cgit/gnulib.git/tree/gnulib-tool#n5268From 93d4e7698afc92e3b257c2a0b2050540357edc96 Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sat, 24 Feb 2024 19:05:51 -0800
Subject: [PATCH] gnulib-tool.py: Handle the sorting of modules correctly.

See discussion at


* pygnulib/GLConfig.py (GLConfig.addModule, CLConfig.setModules): Use a
sorted set when adding modules to replicate "sort -u" used by
gnulib-tool.
* pygnulib/GLImport.py (GLImport.actioncmd): Don't sort modules when
constructing actioncmd as GLConfig handles it for us.
---
 ChangeLog| 11 +++
 pygnulib/GLConfig.py |  6 --
 pygnulib/GLImport.py |  4 ++--
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1910933eb7..a616db7e09 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-02-24  Collin Funk  
+
+	gnulib-tool.py: Handle the sorting of modules correctly.
+	See discussion at
+	
+	* pygnulib/GLConfig.py (GLConfig.addModule, CLConfig.setModules): Use a
+	sorted set when adding modules to replicate "sort -u" used by
+	gnulib-tool.
+	* pygnulib/GLImport.py (GLImport.actioncmd): Don't sort modules when
+	constructing actioncmd as GLConfig handles it for us.
+
 2024-02-24  Collin Funk  
 
 	gnulib-tool.py: Follow gnulib-tool changes, part 28.
diff --git a/pygnulib/GLConfig.py b/pygnulib/GLConfig.py
index bdebf243cc..03da6f8e3c 100644
--- a/pygnulib/GLConfig.py
+++ b/pygnulib/GLConfig.py
@@ -476,7 +476,7 @@ class GLConfig(object):
 '''Add the module to the modules list.'''
 if type(module) is str:
 if module not in self.table['modules']:
-self.table['modules'] += [module]
+self.table['modules'] = sorted(list(set(self.table['modules'] + [module])))
 else:  # if module has not str type
 raise TypeError('module must be a string, not %s'
 % type(module).__name__)
@@ -499,7 +499,9 @@ class GLConfig(object):
 if type(modules) is list or type(modules) is tuple:
 old_modules = self.table['modules']
 self.table['modules'] = list()
-for module in modules:
+# Canonicalize the list of specified modules.
+# sort -u
+for module in sorted(set(modules)):
 try:  # Try to add each module
 self.addModule(module)
 except TypeError as error:
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index c69a33deb7..9b08e7e4b6 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -438,9 +438,9 @@ class GLImport(object):
 elif vc_files == False:
 actioncmd += ' \\\n#  --no-vc-files'
 if len(avoids) > 0:
-actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in sorted(avoids)])
+actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in avoids])
 if len(modules) > 0:
-actioncmd += ''.join([f" \\\n#  {x}" for x in sorted(modules)])
+actioncmd += ''.join([f" \\\n#  {x}" for x in modules])
 return actioncmd
 
 def relative_to_destdir(self, dir):
-- 
2.39.2

Re: gnulib-tool.py: Follow gnulib-tool changes, part 28.

[Bruno Haible]

Hi Collin,

> The first one fixes an item in the
> gnulib-tool.py.TODO file. Previously the "Generated by gnulib-tool"
> comment at the top of the Makefile would be on one line.

Thanks! Applied.

Only one question on this one:

> +if len(avoids) > 0:
> +actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in 
> sorted(avoids)])
> +if len(modules) > 0:
> +actioncmd += ''.join([f" \\\n#  {x}" for x in sorted(modules)])

The sorted(...) instruction is not present in gnulib-tool lines 5647..5652.
Why is it needed? Can you make a quick test with a gnulib-tool invocation
that has several --avoid=... arguments and several module arguments that
are not in increasing alphabetical order?

Bruno

Re: gnulib-tool.py: Follow gnulib-tool changes, part 28.

[Collin Funk]

Hi Bruno,

Thanks for fixing the typo in the other email. I'll remember to use
'foo' instead of "foo". That is a personal habit of mine but I now
realize that it goes against all of the existing code...

On 2/24/24 3:42 PM, Bruno Haible wrote:
> The sorted(...) instruction is not present in gnulib-tool lines 5647..5652.
> Why is it needed? Can you make a quick test with a gnulib-tool invocation
> that has several --avoid=... arguments and several module arguments that
> are not in increasing alphabetical order?

Sure. I'm using the admin/merge-gnulib script from master to test
this. Here is the diff with the two sorted instructions removed.

diff --git a/gnulib.mk.in.python b/gnulib.mk.in.shell
index a86535fd700..a718c17c0e8 100644
--- a/gnulib.mk.in.python
+++ b/gnulib.mk.in.shell
@@ -1,5 +1,5 @@
 ## DO NOT EDIT! GENERATED AUTOMATICALLY!
-# Copyright (C) 2024 Free Software Foundation, Inc.
+# Copyright (C) 2002-2024 Free Software Foundation, Inc.
 #
 # This file is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -173,8 +173,8 @@
 #  timer-time \
 #  timespec-add \
 #  timespec-sub \
-#  update-copyright \
 #  unlocked-io \
+#  update-copyright \
 #  utimensat \
 #  vla \
 #  warnings \

Now the diff with the two sorted instructions added again:

diff --git a/gnulib.mk.in.python-with-sort b/gnulib.mk.in.shell
index 15d15970051..a718c17c0e8 100644
--- a/gnulib.mk.in.python-with-sort
+++ b/gnulib.mk.in.shell
@@ -1,5 +1,5 @@
 ## DO NOT EDIT! GENERATED AUTOMATICALLY!
-# Copyright (C) 2024 Free Software Foundation, Inc.
+# Copyright (C) 2002-2024 Free Software Foundation, Inc.
 #
 # This file is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by

So that is the original reason I added the sorted() functions.
Unrelated, but gnulib-tool.py does not use a year range for copyright.
I'm not sure if it is intentional or not.

Anyways, upon further inspection not all of the gnulib-modules are
sorted in merge-gnulib. When "unlocked-io" was added to Emacs it was
placed after "update-copyright" [1]. I assume that they are sorted
somewhere before the actioncmd step in gnulib-tool. Let me experiment
with the --avoid modules and I'll reply if I notice anything.

[1] https://git.savannah.gnu.org/cgit/emacs.git/tree/admin/merge-gnulib#n50

Collin

Re: gnulib-tool.py: Follow gnulib-tool changes, part 28.

[Collin Funk]

On 2/24/24 4:47 PM, Collin Funk wrote:
> Anyways, upon further inspection not all of the gnulib-modules are
> sorted in merge-gnulib. When "unlocked-io" was added to Emacs it was
> placed after "update-copyright" [1]. I assume that they are sorted
> somewhere before the actioncmd step in gnulib-tool. Let me experiment
> with the --avoid modules and I'll reply if I notice anything.

I think that I have confirmed this behavior by changing the order of
"GNULIB_MODULES" and "AVOIDED_MODULES" in Emacs admin/merge-gnulib
script. Changing GNULIB_MODULES so the first modules are ordered 1. dup2,
2. alignasof, 3. copy-file-range, 4. alloca-opt, ... and
AVOIDED_MODULES so the first modules are ordered 1. chmod, 2. btowc,
3. access produces the following diff:

diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index 711ddcf1260..f16c018b728 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -34,9 +34,9 @@
 #  --no-libtool \
 #  --macro-prefix=gl \
 #  --no-vc-files \
-#  --avoid=access \
-#  --avoid=btowc \
 #  --avoid=chmod \
+#  --avoid=btowc \
+#  --avoid=access \
 #  --avoid=close \
 #  --avoid=crypto/af_alg \
 #  --avoid=dup \

So the --avoid modules are emitted in the order they are passed to
gnulib-tool, but the actual modules will be alphabetically sorted.
Therefore, I think the correct code would be:

if len(avoids) > 0:
actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in avoids])
if len(modules) > 0:
actioncmd += ''.join([f" \\\n#  {x}" for x in sorted(modules)])

Seems sort of strange but it produces the correct output for that
test.

Collin

Re: gnulib-tool.py: Follow gnulib-tool changes, part 27.

[Dima Pasechnik]

Hi Bruno,

The pythonic way is

mode is not None

rather than 

mode != None

(the reason is None is an object)

Just in case,
Dima

On 24 February 2024 23:25:51 GMT, Bruno Haible  wrote:
>Hi Collin,
>
>> > I.e. you meant to write
>> >   mode != None
>> > not
>> >   modules != None
>> > ?
>> 
>> The second fixes this typo. Thanks for noticing it.
>
>But there's another typo in the same line: The original code
>
>  case $mode,$gnu_make in
>*test*,true)
>  echo "gnulib-tool: --gnu-make not supported when including tests"
>  func_exit 1;;
>  esac
>
>has the intent to match the $mode values
>  create-testdir
>  create-megatestdir
>  test
>  megatest
>But these do not contain the substring "tests".
>
>Also, please can we stick with the syntax 'foo', not "foo", for literal
>strings. Outside of gnulib, both syntaxes seem to be in use. But when I
>search where the literal string 'foo' occurs, I don't want to make 2
>searches (for 'foo' and for "foo"), nor a regex search (for ['"]foo['"]).
>Simple things should remain simple. A certain canonical way to denote
>literal strings is necessary for this. (Btw, JavaScript has the same
>problem.)
>
>
>2024-02-24  Bruno Haible  
>
>   gnulib-tool.py: Further fix last commit.
>   * gnulib-tool.py (main): Make the mode test match for 'create-testdir',
>   'create-megatestdir', 'test', 'megatest'.
>
>diff --git a/gnulib-tool.py b/gnulib-tool.py
>index e168e8fc91..1df790c496 100755
>--- a/gnulib-tool.py
>+++ b/gnulib-tool.py
>@@ -607,7 +607,7 @@ def main():
> if cmdargs.pobase == None and cmdargs.podomain != None:
> message = '%s: warning: --po-domain has no effect without a --po-base 
> option\n' % constants.APP['name']
> sys.stderr.write(message)
>-if  mode != None and "tests" in mode and gnu_make:
>+if mode != None and 'test' in mode and gnu_make:
> message = '%s: --gnu-make not supported when including tests\n' % 
> constants.APP['name']
> sys.stderr.write(message)
> sys.exit(1)
>
>
>
>