Re: gnulib-tool.py: Locate configure.ac correctly when --dir is given.
Hi Collin, > When running ./autogen.sh in gettext gnulib-tool.py fails on the first > invocation. :( > > The command is something like this: > > $GNULIB_TOOL --dir=gettext-runtime ... > > We get the following error message: > ... > FileNotFoundError: [Errno 2] No such file or directory: > 'gettext-runtime/gettext-runtime/configure.ac' > ... > In main we have joinpath(destdir, 'configure.ac') which is then passed > to GLConfig.setAutoconfFile which has > os.path.join(self.table['destdir'], configure_ac). And in particular, main.py tests whether the file actually exists; therefore GLConfig *must* just accept the string as it is, without making modifications to it. Thanks for the patch; applied. Bruno
Re: gnulib-tool.py: Use auxdir as given by AC_CONFIG_AUX_DIR.
Hi Collin, > It seems that gettext catches a lot of issues with gnulib-tool.py... Thanks! I've applied both patches. > Since joinpath() uses os.path.norm(), it will see '$(top_srcdir)/..' > and delete both path components. This is incorrect. So, I'm adding a comment about it: 2024-04-07 Bruno Haible gnulib-tool.py: Add comment. * pygnulib/constants.py (joinpath): Document a limitation. diff --git a/pygnulib/constants.py b/pygnulib/constants.py index 7a88c21c39..a07e473529 100644 --- a/pygnulib/constants.py +++ b/pygnulib/constants.py @@ -269,7 +269,10 @@ def cleaner(sequence: str | list[str]) -> str | list[str | bool]: def joinpath(head: str, *tail: str) -> str: '''Join two or more pathname components, inserting '/' as needed. If any component is an absolute path, all previous path components will be -discarded.''' +discarded. +This function also replaces SUBDIR/../ with empty; therefore it is not +suitable when some of the pathname components use Makefile variables +such as '$(srcdir)'.''' newtail = [] for item in tail: newtail += [item]
Re: gnulib-tool.py: lists vs. sets
Hi Collin, > I was trying to cleanup list comprehensions from this: > > file_table = sorted([ file > for file in table1 > if file not in table2 ]) > > to this: > > file_table = sorted(set(table1).difference(table2)) > > because I find them easier to follow. In my opinion, both are good ways to express the same thing. The second one is more expressive; but we all can store a 3-lines snippet of code in our short-term memory. The interesting question here is: is table2 a possibly long list? If yes, then can we convert it to a set, without impairing the clarity of the code? Bruno
Re: gnulib-tool.py: lists vs. sets
Hi Bruno,
On 4/7/24 4:56 AM, Bruno Haible wrote:
> In my opinion, both are good ways to express the same thing.
> The second one is more expressive; but we all can store a 3-lines
> snippet of code in our short-term memory.
Yeah, it isn't bad. Just personal preference I guess. Two more lines
for me to make typos though. :)
> The interesting question here is: is table2 a possibly long list?
> If yes, then can we convert it to a set, without impairing the
> clarity of the code?
There are probably some places where it would make sense. The test
suite should help catch any divergence from gnulib-tool.sh's sorting
behavior.
Since gnulib-tool.py doesn't have a speed issue, I'll probably focus
more on being a code janitor and cleaning up a bit.
That reminds me, is there a reason why in many places there is
something like this:
newtail = []
for item in tail:
newtail += [item]
instead of this?
newtail = []
for item in tail:
newtail.append(item)
I like the second more, but I can't tell if it is another overly
strong personal preference of mine...
Not that we will notice this, but an interesting implementation
detail:
>>> import timeit
>>> timeit.timeit(stmt='var += ["a"]', setup='var = []', number=1)
6.761068544998125
>>> timeit.timeit(stmt='var.append("a")', setup='var = []', number=1)
1.736760463688
Collin
Re: gnulib-tool.py: Use auxdir as given by AC_CONFIG_AUX_DIR.
On 4/7/24 4:48 AM, Bruno Haible wrote: > So, I'm adding a comment about it: Thanks! I should have remembered to do that, sorry... Collin
Re: Python list micro-benchmarks
Collin Funk wrote:
> That reminds me, is there a reason why in many places there is
> something like this:
>
> newtail = []
> for item in tail:
> newtail += [item]
>
> instead of this?
>
> newtail = []
> for item in tail:
> newtail.append(item)
>
> I like the second more, but I can't tell if it is another overly
> strong personal preference of mine...
I like the first one a little more. So I asked the prior knowledge
summarization engine (ChatGPT):
User
For appending an item to a list in Python, is it better to write `l += [item];`
or `l.append(item);` ?
ChatGPT
Both l += [item] and l.append(item) achieve the goal of appending an item to a
list l, but they have slightly different implications and use cases.
1. l.append(item): This is the standard method for appending an item to a list
in Python. It is clear, concise, and specifically designed for this purpose. It
directly modifies the list in place.
2. l += [item]: This uses the augmented assignment operator (+=) with a list.
It is essentially shorthand for l = l + [item]. While it achieves the same
result, it involves creating a new list (by concatenating the existing list l
with [item]) and then assigning it back to l. Depending on the size of the
list, this can be less efficient in terms of memory usage and potentially
slower, especially for large lists.
In general, if you simply want to append an item to a list, l.append(item) is
preferred because it is more explicit and efficient. However, there might be
scenarios where using += with a list might be appropriate, such as when you
want to concatenate multiple lists together.
So, ChatGPT sides with your preference.
> Not that we will notice this, but an interesting implementation
> detail:
>
> >>> import timeit
> >>> timeit.timeit(stmt='var += ["a"]', setup='var = []', number=1)
> 6.761068544998125
> >>> timeit.timeit(stmt='var.append("a")', setup='var = []', number=1)
> 1.736760463688
Since you show me how to do benchmarks, and since ChatGPT warns about large
lists, I ran the same benchmark with a list of length 100 or 1000. See
attachment.
The result is mysterious: Appending to a list of length 100 or 1000 takes
about the same time either way. In particular, the 'var += ["a"]' is *not*
particularly slow when the list is long. It's impossible to clone a list of
length 1000 and add an item to it, in 41 nanoseconds; that makes no sense.
There's something we must have overlooked. Is this being particularly optimized
by the Python compiler? Or is a reference-counting trick providing the nearly-
constant execution time?
Bruno
>>> import timeit
>>> timeit.timeit(stmt='var += ["a"]', setup='var =
>>> ["0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9"]',
>>> number=1)
3.9719880633056164
>>> timeit.timeit(stmt='var.append("a")', setup='var =
>>> ["0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9"]',
>>> number=1)
2.657867263071239
>>> timeit.timeit(stmt='var += ["a"]', setup='var =
>>> ["0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","3","4","5","6","7","8","9","0","1","2","
sigsegv tests: Avoid a crash on NetBSD 10.0/i386
On NetBSD 10.0/i386, one of the 'sigsegv' tests crashes:
FAIL: test-sigsegv-catch-stackoverflow1
===
FAIL test-sigsegv-catch-stackoverflow1 (exit status: 139)
More precisely, it crashes inside _rtld_bind, when
stackoverflow_handler_continuation invokes longjmp.
The crash is reproducible in a testdir of all gnulib modules,
but not in a testdir of the 'sigsegv' module alone.
I've found two possible workarounds:
- Align the stack pointer before invoking longjmp.
- Ensure that resolving the 'longjmp' symbol is already done
in main(), through
if (setjmp (mainloop) == 0)
longjmp (mainloop, 1);
The former is the more proper fix. So, that is what I'm applying:
2024-04-07 Bruno Haible
sigsegv tests: Avoid a crash on NetBSD 10.0/i386.
* tests/test-sigsegv-catch-stackoverflow1.c
(stackoverflow_handler_continuation): On NetBSD/i386, align the stack
pointer before calling longjmp.
diff --git a/tests/test-sigsegv-catch-stackoverflow1.c
b/tests/test-sigsegv-catch-stackoverflow1.c
index 20915adfd5..46b120686c 100644
--- a/tests/test-sigsegv-catch-stackoverflow1.c
+++ b/tests/test-sigsegv-catch-stackoverflow1.c
@@ -55,7 +55,26 @@ static volatile char *stack_upper_bound;
static void
stackoverflow_handler_continuation (void *arg1, void *arg2, void *arg3)
{
+#if defined __NetBSD__ && defined __i386__
+ /* On NetBSD 10.0/i386, when built as part of a testdir-all (but not as part
+ of a testdir for just the module 'sigsegv'!) this program crashes. The
+ cause is that:
+ - The alternate stack is not aligned (which is intentional, see
+ altstack-util.h) and NetBSD does not align the stack pointer while
+ switching to the alternate stack.
+ - When %esp is not aligned, the dynamic linker crashes in function
+ _rtld_bind while resolving the symbol 'longjmp'.
+ We would around this by aligning the stack pointer, to a multiple of 8.
*/
+ int *argp;
+ __asm__ __volatile__ ("movl %1,%0" : "=r" (argp) : "r" (&arg1));
+ unsigned long sp;
+ __asm__ __volatile__ ("movl %%esp,%0" : "=r" (sp));
+ sp &= ~7UL;
+ __asm__ __volatile__ ("movl %0,%%esp" : : "r" (sp));
+ int arg = *argp;
+#else
int arg = (int) (long) arg1;
+#endif
longjmp (mainloop, arg);
}
Re: serial number format
I've not been a fan of those .m4 serial numbers, which may help to explain why so many .m4 files lack them. Although I still have doubts whether they're worth the trouble, perhaps it'd help if we added a file m4/.dir-locals.el (or something similar) that would cause Emacs to update these serial numbers automatically.
Re: Issue with intprops-internal.h / GCC 8.3
On Sat, 2024-03-30 at 02:17 +0100, Bruno Haible wrote: > The problem with your git repository setup (maintMakefile line 32, > that enables -Werror by default) is that in practice, it's not > only developers who build from git repositories. Namely, many people > whose habits have been shaped by GitHub will look for the git > repository before looking for a release tarball. Well, they are in for some hardship. Building GNU Make from Git requires a significant number of tools to be installed, including automake, autoconf, gettext, texinfo, gnulib, perl 5 (if tests are run), and probably some other things I've forgotten. I do not ever check in any generated files, at all (except, I guess, the gnulib bootstrap script which is generated by gnulib) so all these tools need to be installed by the user to build from Git. Hence we can assume that anyone building GNU Make from Git is sufficiently knowledgeable abut Free Software build environments that they can obtain and configure all the above tools, at sufficient versions (I tend to require the latest versions, pretty quickly after release). However, I'll reconsider whether to preserve -Werror by default, and at least document the proper way to disable it in the README.git file which should be used by those building GNU Make from Git workspaces. What I'd really prefer to do is provide a "nightly source tarball" somewhere which contains the latest Git code, already configured, for those who want to try it. But I don't have a simple way to do that with the current GNU infrastructure AFAIK. It seems like there's little appetite for deleting content from alpha.gnu.org, or creating ephemeral content there. -- Paul D. Smith Find some GNU Make tips at: https://www.gnu.org http://make.mad-scientist.net "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
Re: Issue with intprops-internal.h / GCC 8.3
Hi Paul, Paul Smith wrote: > > ... in practice, it's not > > only developers who build from git repositories. Namely, many people > > whose habits have been shaped by GitHub will look for the git > > repository before looking for a release tarball. > > Well, they are in for some hardship. > > Building GNU Make from Git requires a significant number of tools to be > installed, including automake, autoconf, gettext, texinfo, gnulib, perl > 5 (if tests are run), and probably some other things I've forgotten. Yep, many GNU (and non-GNU) packages are in the same situation: Special and non-so-special tools are needed in order to build from the git repository. > I do not ever check in any generated files, at all (except, I guess, > the gnulib bootstrap script which is generated by gnulib) That's the common way of doing, since otherwise working with git branches becomes a major hassle. One file, though, should be added under version control: doc/fdl.texi. See https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=modules/fdl . Rationale: So that people who check out the git repository _without_ building it still know about the license of doc/*.texi. [0] > What I'd really prefer to do is provide a "nightly source tarball" > somewhere which contains the latest Git code, already configured, for > those who want to try it. But I don't have a simple way to do that > with the current GNU infrastructure AFAIK. It seems like there's > little appetite for deleting content from alpha.gnu.org, or creating > ephemeral content there. Yes, the current GNU infrastructure does not provide this ability. But I have such a "weekly source tarball" infrastructure in place for some GNU packages [1][2][3] on gitlab.com. Other GNU packages do it on sourceware.org [4]. If you want me to set up (and maintain) such a thing for GNU make on gitlab, I can do that. If you want it on sourceware.org, you can contact mjw [5]. Bruno [0] https://www.gnu.org/software/gnulib/manual/html_node/License-Texinfo-sources.html [1] https://gitlab.com/gnu-grep/ci-distcheck [2] https://gitlab.com/gnu-m4/ci-distcheck [3] https://gitlab.com/gnu-gettext/ci-distcheck [4] https://snapshots.sourceware.org/gnupoke/ [5] https://gnu.wildebeest.org/blog/mjw/
Re: serial number format
Paul Eggert wrote: > Although I still have doubts whether they're worth the trouble While I personally have not encountered the benefits of the serial numbers — since I don't use 'aclocal --install' —, other people surely do. > perhaps it'd help if we added a file > m4/.dir-locals.el (or something similar) that would cause Emacs to > update these serial numbers automatically. This sounds useful, yes. Any Emacs Lisp hacker volunteers? Bruno
Re: serial number format
Paul Eggert writes: > I've not been a fan of those .m4 serial numbers, which may help to > explain why so many .m4 files lack them. Although I still have doubts > whether they're worth the trouble, perhaps it'd help if we added a > file m4/.dir-locals.el (or something similar) that would cause Emacs > to update these serial numbers automatically. Not quite it, but autoconf-archive has some tooling around it: * https://github.com/autoconf-archive/autoconf-archive/blob/81e26138e807b9ecb3c19b953e8776ff9e415fdf/macro.py#L112 * https://github.com/autoconf-archive/autoconf-archive/blob/81e26138e807b9ecb3c19b953e8776ff9e415fdf/set-serial.sh#L20 Maintainer mode is integrated with this so it updates serial numbers automatically.
Re: Python list micro-benchmarks
Hi Bruno, On 4/7/24 7:23 AM, Bruno Haible wrote: > I like the first one a little more. So I asked the prior knowledge > summarization engine (ChatGPT): I like that description, "prior knowledge summarization engine". > Since you show me how to do benchmarks, and since ChatGPT warns about large > lists, I ran the same benchmark with a list of length 100 or 1000. See > attachment. The 'timeit' module is a nice way to quickly expirement with things. There are more detailed performance testing tools, but I am not familiar with them [1]. > The result is mysterious: Appending to a list of length 100 or 1000 takes > about the same time either way. In particular, the 'var += ["a"]' is *not* > particularly slow when the list is long. It's impossible to clone a list of > length 1000 and add an item to it, in 41 nanoseconds; that makes no sense. I think that I see the issue here. I believe this is a case of the "prior knowledge summarization engine" being incorrect. Given this statement: a += [var] it seems to think that a new copy of 'a' is being created. This is incorrect. The given statement is equal to: operator.iadd(a, [var]) The Python documentation states that "For mutable targets such as lists and dictionaries, the in-place method will perform the update, so no subsequent assignment is necessary [2]." It seems that 'a += [var]' be the same as 'a.extend([var])'. I think that the difference between 'a.extend([var])' and 'a.append(var)' would better explain the differences in timing that we see with a large number of repetitions. I've found the C source code for those functions if you would like to look [3] [4]. [1] https://github.com/psf/pyperf [2] https://docs.python.org/3/library/operator.html#in-place-operators [3] https://github.com/python/cpython/blob/558b517bb571999ef36123635e0245d083a28305/Objects/listobject.c#L980 [4] https://github.com/python/cpython/blob/558b517bb571999ef36123635e0245d083a28305/Objects/listobject.c#L838 Collin
gnulib-tool.py: Don't remove duplicate avoided modules.
In test-gettext-7.sh there are missing modules in gl_AVOID and the
actioncmd printed on Makefiles. This is because gnulib-tool.py will
remove the duplicates. This patch fixes that.
CollinFrom 4fa6f5231eed42a9935c1f25cdf3c1db3431a8c4 Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Sun, 7 Apr 2024 16:41:18 -0700
Subject: [PATCH] gnulib-tool.py: Don't remove duplicate avoided modules.
* pygnulib/GLConfig.py (GLConfig.addAvoid): Don't remove duplicates from
the modules passed with --avoid.
---
ChangeLog| 6 ++
pygnulib/GLConfig.py | 3 +--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e9054029a7..535dfc04cc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2024-04-07 Collin Funk
+
+ gnulib-tool.py: Don't remove duplicate avoided modules.
+ * pygnulib/GLConfig.py (GLConfig.addAvoid): Don't remove duplicates from
+ the modules passed with --avoid.
+
2024-04-07 Bruno Haible
sigsegv tests: Avoid a crash on NetBSD 10.0/i386.
diff --git a/pygnulib/GLConfig.py b/pygnulib/GLConfig.py
index f5282dbf53..0f9d118a78 100644
--- a/pygnulib/GLConfig.py
+++ b/pygnulib/GLConfig.py
@@ -559,8 +559,7 @@ class GLConfig:
'''Avoid including the given module. Useful if you have code that provides
equivalent functionality.'''
if type(module) is str:
-if module not in self.table['avoids']:
-self.table['avoids'].append(module)
+self.table['avoids'].append(module)
else: # if module has not str type
raise TypeError('avoid must be a string, not %s'
% type(module).__name__)
--
2.44.0
Re: gnulib-tool.py: Don't remove duplicate avoided modules.
Collin Funk wrote: > In test-gettext-7.sh there are missing modules in gl_AVOID and the > actioncmd printed on Makefiles. This is because gnulib-tool.py will > remove the duplicates. This patch fixes that. Thanks! Applied. All test-gettext-*.sh pass now. Cool. Bruno
gnulib-tool.py current status
On 4/7/24 5:20 PM, Bruno Haible wrote: > Thanks! Applied. All test-gettext-*.sh pass now. Cool. Yep, now I can focus on trying to clean up some stuff. By the way, here is a list of packages that I have tested using your method here: https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00018.html These were done sometime in the past ~10 commits, so I would have to go through and double check no breakage occured since then. * Packages successfully tested with gnulib-tool.py bison coreutils cppi cpio diffutils findutils freedink Update AC_PREREQ to 2.64 required. gnutls grep groff gzip inetutils libiconv mailutils patch pspp sed tar wget wget2 * Packages with issues with gnulib-tool.py guile /home/collin/.local/src/gnulib/gnulib-tool.sh: *** patch file gnulib-local/m4/clock_time.m4.diff didn't apply cleanly /home/collin/.local/src/gnulib/gnulib-tool.sh: *** Stop. rcs /home/collin/.local/src/gnulib/gnulib-tool.sh: *** file /home/collin/.local/src/gnulib/build-aux/missing not found /home/collin/.local/src/gnulib/gnulib-tool.sh: *** Stop. The two failures don't seem too bad, especially since they failed with gnulib-tool.sh. I probably ignored the steps in the README... Collin
Re: Python list micro-benchmarks
On 2024-04-07 16:07, Collin Funk wrote: I think that the difference between 'a.extend([var])' and 'a.append(var)' would better explain the differences in timing that we see with a large number of repetitions. a.append should be faster, as it need not cons the singleton.
Re: Python list micro-benchmarks
Hi Paul,
On 4/7/24 5:51 PM, Paul Eggert wrote:
> a.append should be faster, as it need not cons the singleton.
Thanks. Yes, that it is what I was trying to get at. But I didn't know
the correct wording. :)
Here is 'var.extend(["a"])' added to the simple timeit comparison:
>>> import timeit
>>> timeit.timeit(stmt='var += ["a"]', setup='var = []', number=1)
6.978962032000709
>>> timeit.timeit(stmt='var.extend(["a"])', setup='var = []', number=1)
6.58610403899911
>>> timeit.timeit(stmt='var.append("a")', setup='var = []', number=1)
1.8816217469993717
Collin
gnulib-tool.py: Omit some unnecessary list() calls around sorted().
Here is two janitorial work patches.
I remember an extra dict() call causing trouble with GLMakefileTable.
Removing the extra list() calls from sorted() seemed like a good place
to start cleaning up.
The second patch removes this function:
def removeFile(self, file: str) -> None:
'''Remove file from the list of added files.'''
if file in self.added:
self.added.pop(file)
This is incorrect since list.pop() takes an index and not a list
element. This function is unused so I've chosen to remove it. If it is
ever needed in the future the correct method would be list.remove().
CollinFrom d5190037c894a12eb7cbaaee1b89800cbb56615a Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Sun, 7 Apr 2024 18:49:24 -0700
Subject: [PATCH 1/2] gnulib-tool.py: Omit some unnecessary list() calls around
sorted().
* pygnulib/GLEmiter.py (GLEmiter.lib_Makefile_am): Remove the list()
call in the argument to sorted. The sorted() function works on any
iterable and always returns a list.
* pygnulib/GLImport.py (GLImport.rewrite_old_files)
(GLImport.rewrite_new_files): Likewise.
* pygnulib/GLModuleSystem.py (GLModuleTable.transitive_closure)
(GLModuleTable.transitive_closure_separately): Likewise.
* pygnulib/GLTestDir.py (GLTestDir.rewrite_files): Likewise.
---
ChangeLog | 12
pygnulib/GLEmiter.py | 4 ++--
pygnulib/GLImport.py | 6 ++
pygnulib/GLModuleSystem.py | 4 ++--
pygnulib/GLTestDir.py | 3 +--
5 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 535dfc04cc..4fd962ecbb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2024-04-07 Collin Funk
+
+ gnulib-tool.py: Omit some unnecessary list() calls around sorted().
+ * pygnulib/GLEmiter.py (GLEmiter.lib_Makefile_am): Remove the list()
+ call in the argument to sorted. The sorted() function works on any
+ iterable and always returns a list.
+ * pygnulib/GLImport.py (GLImport.rewrite_old_files)
+ (GLImport.rewrite_new_files): Likewise.
+ * pygnulib/GLModuleSystem.py (GLModuleTable.transitive_closure)
+ (GLModuleTable.transitive_closure_separately): Likewise.
+ * pygnulib/GLTestDir.py (GLTestDir.rewrite_files): Likewise.
+
2024-04-07 Collin Funk
gnulib-tool.py: Don't remove duplicate avoided modules.
diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index cb813b3baa..45462c5fe3 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -879,8 +879,8 @@ AC_DEFUN([%V1%_LIBSOURCES], [
capture_output=True)
if result.returncode == 0:
# sort -u
-emit += lines_to_multiline(sorted(list(set(x.strip()
- for x in result.stdout.decode(encoding='utf-8').splitlines()
+emit += lines_to_multiline(sorted({ x.strip()
+for x in result.stdout.decode(encoding='utf-8').splitlines() }))
else:
emit += '== gnulib-tool GNU Make output failed as follows ==\n'
emit += ['# stderr: ' + x + '\n' for x in
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 2776f2c964..08e83c5c9a 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -336,8 +336,7 @@ class GLImport:
else: # file is not a special file
path = file
result += [os.path.normpath(path)]
-result = sorted(set(result))
-return list(result)
+return sorted(set(result))
def rewrite_new_files(self, files: list[str]) -> list[str]:
'''Replace auxdir, docbase, sourcebase, m4base and testsbase from
@@ -373,8 +372,7 @@ class GLImport:
else: # file is not a special file
path = file
result += [os.path.normpath(path)]
-result = sorted(set(result))
-return list(result)
+return sorted(set(result))
def actioncmd(self) -> str:
'''Return command-line invocation comment.'''
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index f8ff71383a..3997668538 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -935,7 +935,7 @@ class GLModuleTable:
inc_all_tests = self.inc_all_indirect_tests
modules = sorted(set(outmodules))
self.modules = modules
-return list(modules)
+return modules
def transitive_closure_separately(self, basemodules: list[GLModule],
finalmodules: list[GLModule]) -> tuple[list[GLModule], list[GLModule]]:
@@ -978,7 +978,7 @@ class GLModuleTable:
# + [ m
# for m in main_modules
# if m.getApplicability() != 'main' ]
-tests_modules = sorted(list(set(tests_modules)))
+tests_modules = sorted(set(tests_modules))
# If testsrelated_modules consists only of modules with applicability 'all
Re: gnulib-tool.py: Omit some unnecessary list() calls around sorted().
Hi Bruno,
I was having a look at GLModuleSystem.py and noticed some duplicate
checks that an key is valid. Patch 0003 addresses that.
Patch 0004 adds a missing None return type hint to
GLModuleTable.getCondition(). This is used to indicate that the module
is not a conditional dependency. I missed this in the type hint patch.
This was also one of the functions you mentioned here:
https://lists.gnu.org/archive/html/bug-gnulib/2024-03/msg00360.html
> * In GLModuleSystem.py lines 784, 821:
> A condition can be a string or True. But
> str | bool
>makes it look like False was also a valid value. Is possible to
>write
> str | True
>in some way?
The correct way to do this is Literal[True] after the import:
from typing import Literal
but I wasn't sure if it was compatible with Python 3.7. I ended up
trying it and it is compatible with Python 3.7. The documentation says
Python 3.8 which I've confirmed with my installation [1].
Here is the thread we chose Python 3.7:
https://lists.gnu.org/archive/html/bug-gnulib/2024-03/msg4.html
It appears it is EOL but I'm not sure how RHEL / CentOS versions and
packages work [2]. Does the next version default to 3.7? It looks like
they ship a few and allow you to use 'alternatives --set', but I am
unsure which is default.
[1] https://docs.python.org/3/library/typing.html#typing.Literal
[2] https://devguide.python.org/versions
CollinFrom 415efd4e122ec5377d890f3e44a992eb7b06986d Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Sun, 7 Apr 2024 22:00:48 -0700
Subject: [PATCH 3/4] gnulib-tool.py: Remove unnecessary conditional.
* pygnulib/GLModuleSystem.py (GLModuleTable.__getitem__): Don't check if
the key is valid twice.
---
ChangeLog | 6 ++
pygnulib/GLModuleSystem.py | 21 ++---
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index cb563b3830..2bae9994dd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2024-04-07 Collin Funk
+
+ gnulib-tool.py: Remove unnecessary conditional.
+ * pygnulib/GLModuleSystem.py (GLModuleTable.__getitem__): Don't check if
+ the key is valid twice.
+
2024-04-07 Collin Funk
gnulib-tool.py: Remove a unused and incorrect function.
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index 3997668538..d3f38c2b66 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -763,17 +763,16 @@ class GLModuleTable:
def __getitem__(self, y: str) -> list[GLModule]:
'''x.__getitem__(y) <==> x[y]'''
-if y in ['base', 'final', 'main', 'tests', 'avoids']:
-if y == 'base':
-return self.getBaseModules()
-elif y == 'final':
-return self.getFinalModules()
-elif y == 'main':
-return self.getMainModules()
-elif y == 'tests':
-return self.getTestsModules()
-else: # if y == 'avoids'
-return self.getAvoids()
+if y == 'base':
+return self.getBaseModules()
+elif y == 'final':
+return self.getFinalModules()
+elif y == 'main':
+return self.getMainModules()
+elif y == 'tests':
+return self.getTestsModules()
+elif y == 'avoids':
+return self.getAvoids()
else: # if y is not in list
raise KeyError('GLModuleTable does not contain key: %s' % repr(y))
--
2.44.0
From 61f1bf56f271581555e60aa3d20edbc448565eb9 Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Sun, 7 Apr 2024 22:22:49 -0700
Subject: [PATCH 4/4] gnulib-tool.py: Fix incomplete type hint.
* pygnulib/GLModuleSystem.py (GLModuleTable.getCondition): Add None to
the return type hint. This is the return value when the module is not a
conditional dependency.
---
ChangeLog | 7 +++
pygnulib/GLModuleSystem.py | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 2bae9994dd..0f0aedd2d2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-04-07 Collin Funk
+
+ gnulib-tool.py: Fix incomplete type hint.
+ * pygnulib/GLModuleSystem.py (GLModuleTable.getCondition): Add None to
+ the return type hint. This is the return value when the module is not a
+ conditional dependency.
+
2024-04-07 Collin Funk
gnulib-tool.py: Remove unnecessary conditional.
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index d3f38c2b66..761c43c2e0 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -813,7 +813,7 @@ class GLModuleTable:
result = str(module) in self.dependers
return result
-def getCondition(self, parent: GLModule, module: GLModule) -> str | bool:
+def getCondition(self, parent: GLModule, module: GLModule) -> str | bool | None:
'''Return condition from parent to module. Condition can be string or True.
If module is
Re: gnulib-tool.py: Omit some unnecessary list() calls around sorted().
On 4/7/24 10:57 PM, Collin Funk wrote: > it is compatible with Python 3.7 It is *not* compatible with Python 3.7. It only works with Python 3.8+. My bad... Collin
