Hi Bruno,
On 4/9/24 12:04 PM, Bruno Haible wrote:
> A better fix would be revisit this 'verifier'. Either an enum with 3 possible
> values, or (better) a function GLModule -> bool. And call it 'moduleFilter'
> rather than 'verifier'.
How does this patch look?
This method seems much clearer to me than passing 0, 1, or 2 which
have no meaning unless you have previously read the docstring.
I've type hinted it and double checked that it works with Python 3.7
and Python 3.12 on my machine. The type hinting looks something like
this [1]:
from collections.abc import Callable
def autoconfSnippets(self, ..., module_filter: Callable[[GLModule], bool])
-> str:
Meaning that 'module_filter' is a function that accepts a GLModule
object and returns a bool.
This removes a lot of repetitive lines which is nice. The conditionals
can be refactored to remove a level of nesting in a few places. I
didn't do that now since it would make the patch harder to follow.
I've just used lambda functions since these functions are just
wrappers around GLModule methods. I'm not sure if it is better to
define them in GLModuleSystem.py or something. Since you are the
lisp expert, I will trust your judgment here.
Also, now I see an annoying warning:
lambda module: True # warning under 'module'
because pyright thinks it is unused [2]. I tried omitting the 'module'
in hopes that Python would just deal with it. But it gives this:
Traceback (most recent call last):
[...]
File "/home/collin/.local/src/gnulib/pygnulib/GLEmiter.py", line 311, in
autoconfSnippets
if module_filter(module):
^^^^^^^^^^^^^^^^^^^^^
TypeError: GLImport.gnulib_comp.<locals>.<lambda>() takes 0 positional
arguments but 1 was given
Maybe I will use fixing that as an excuse to add to the HACKING file.
I meant to add information there earlier but got lazy...
[1] https://docs.python.org/3/library/typing.html#annotating-callable-objects
[2] https://github.com/microsoft/pyright
CollinFrom 45a750586980579c44c14e5d95ac4434f2d61313 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Wed, 10 Apr 2024 05:05:52 -0700
Subject: [PATCH] gnulib-tool.py: Use function arguments instead of magic
numbers.
* pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Remove the
'verifier' integer flag argument. Add the 'module_filter' function
argument. Use it to determine if Autoconf snippets should be printed for
each module.
* pygnulib/GLImport.py (GLImport.gnulib_comp): Update call to use a
lambda function.
* pygnulib/GLTestDir.py (GLTestDir.execute): Likewise.
---
ChangeLog | 11 ++++++++
pygnulib/GLEmiter.py | 62 +++++++------------------------------------
pygnulib/GLImport.py | 4 +--
pygnulib/GLTestDir.py | 21 ++++++++-------
4 files changed, 35 insertions(+), 63 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 033eb5dc92..f6aaab6b05 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-04-10 Collin Funk <collin.fu...@gmail.com>
+
+ gnulib-tool.py: Use function arguments instead of magic numbers.
+ * pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Remove the
+ 'verifier' integer flag argument. Add the 'module_filter' function
+ argument. Use it to determine if Autoconf snippets should be printed for
+ each module.
+ * pygnulib/GLImport.py (GLImport.gnulib_comp): Update call to use a
+ lambda function.
+ * pygnulib/GLTestDir.py (GLTestDir.execute): Likewise.
+
2024-04-09 Collin Funk <collin.fu...@gmail.com>
gnulib-tool.py: Change the avoid list to a set for lookups.
diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index e6143645be..247e791e6b 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -22,6 +22,7 @@ import os
import re
import codecs
import subprocess as sp
+from collections.abc import Callable
from . import constants
from .GLInfo import GLInfo
from .GLConfig import GLConfig
@@ -257,7 +258,7 @@ class GLEmiter:
return emit
def autoconfSnippets(self, modules: list[GLModule], referenceable_modules: list[GLModule],
- moduletable: GLModuleTable, verifier: int, toplevel: bool,
+ moduletable: GLModuleTable, module_filter: Callable[[GLModule], bool], toplevel: bool,
disable_libtool: bool, disable_gettext: bool, replace_auxdir: bool) -> str:
'''Collect and emit the autoconf snippets of a set of modules.
GLConfig: conddeps.
@@ -271,10 +272,8 @@ class GLEmiter:
dependencies.
moduletable is a GLModuleTable instance, which contains necessary
information about dependencies of the modules.
- verifier is an integer, which can be 0, 1 or 2.
- if verifier == 0, then process every module;
- if verifier == 1, then process only non-tests modules;
- if verifier == 2, then process only tests modules.
+ module_filter is a function that accepts a GLModule and returns a bool describing
+ whether or not Autoconf snippets should be emitted for it.
toplevel is a bool variable, False means a subordinate use of pygnulib.
disable_libtool is a bool variable; it tells whether to disable libtool
handling even if it has been specified through the GLConfig class.
@@ -291,11 +290,6 @@ class GLEmiter:
if type(moduletable) is not GLModuleTable:
raise TypeError('moduletable must be a GLModuleTable, not %s'
% type(moduletable).__name__)
- if type(verifier) is not int:
- raise TypeError('verifier must be an int, not %s'
- % type(verifier).__name__)
- if not (0 <= verifier <= 2):
- raise ValueError('verifier must be 0, 1 or 2, not %d' % verifier)
if type(toplevel) is not bool:
raise TypeError('toplevel must be a bool, not %s'
% type(toplevel).__name__)
@@ -314,37 +308,19 @@ class GLEmiter:
if not conddeps:
# Ignore the conditions, and enable all modules unconditionally.
for module in modules:
- if verifier == 0:
- solution = True
- elif verifier == 1:
- solution = module.isNonTests()
- elif verifier == 2:
- solution = module.isTests()
- if solution:
+ if module_filter(module):
emit += self.autoconfSnippet(module, toplevel,
disable_libtool, disable_gettext, replace_auxdir, ' ')
else: # if conddeps
# Emit the autoconf code for the unconditional modules.
for module in modules:
- if verifier == 0:
- solution = True
- elif verifier == 1:
- solution = module.isNonTests()
- elif verifier == 2:
- solution = module.isTests()
- if solution:
+ if module_filter(module):
if not moduletable.isConditional(module):
emit += self.autoconfSnippet(module, toplevel,
disable_libtool, disable_gettext, replace_auxdir, ' ')
# Initialize the shell variables indicating that the modules are enabled.
for module in modules:
- if verifier == 0:
- solution = True
- elif verifier == 1:
- solution = module.isNonTests()
- elif verifier == 2:
- solution = module.isTests()
- if solution:
+ if module_filter(module):
if moduletable.isConditional(module):
shellvar = module.getShellVar()
emit += ' %s=false\n' % module.getShellVar()
@@ -352,13 +328,7 @@ class GLEmiter:
# function. This makes it possible to support cycles among conditional
# modules.
for module in modules:
- if verifier == 0:
- solution = True
- elif verifier == 1:
- solution = module.isNonTests()
- elif verifier == 2:
- solution = module.isTests()
- if solution:
+ if module_filter(module):
if moduletable.isConditional(module):
shellfunc = module.getShellFunc()
shellvar = module.getShellVar()
@@ -390,13 +360,7 @@ class GLEmiter:
emit += ' }\n'
# Emit the dependencies from the unconditional to the conditional modules.
for module in modules:
- if verifier == 0:
- solution = True
- elif verifier == 1:
- solution = module.isNonTests()
- elif verifier == 2:
- solution = module.isTests()
- if solution:
+ if module_filter(module):
if not moduletable.isConditional(module):
depmodules = module.getDependenciesWithoutConditions()
# Intersect dependencies with the modules list.
@@ -419,13 +383,7 @@ class GLEmiter:
# Define the Automake conditionals.
emit += ' m4_pattern_allow([^%s_GNULIB_ENABLED_])\n' % macro_prefix
for module in modules:
- if verifier == 0:
- solution = True
- elif verifier == 1:
- solution = module.isNonTests()
- elif verifier == 2:
- solution = module.isTests()
- if solution:
+ if module_filter(module):
if moduletable.isConditional(module):
condname = module.getConditionalName()
shellvar = module.getShellVar()
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 4a6f7c48b8..66d8eb922c 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -678,7 +678,7 @@ AC_DEFUN([%s_INIT],
emit += ' m4_pushdef([gl_MODULE_INDICATOR_CONDITION], [%s])\n' % witness_c_macro
# Emit main autoconf snippets.
emit += self.emitter.autoconfSnippets(moduletable.getMainModules(), moduletable.getMainModules(),
- moduletable, 0, True, False, True, replace_auxdir)
+ moduletable, lambda module: True, True, False, True, replace_auxdir)
if witness_c_macro:
emit += ' m4_popdef([gl_MODULE_INDICATOR_CONDITION])\n'
emit += ' # End of code from modules\n'
@@ -702,7 +702,7 @@ AC_DEFUN([%s_INIT],
# Emit tests autoconf snippets.
emit += self.emitter.autoconfSnippets(moduletable.getTestsModules(),
moduletable.getMainModules() + moduletable.getTestsModules(),
- moduletable, 0, True, True, True, replace_auxdir)
+ moduletable, lambda module: True, True, True, True, replace_auxdir)
emit += ' m4_popdef([gl_MODULE_INDICATOR_CONDITION])\n'
emit += self.emitter.initmacro_end('%stests' % macro_prefix, gentests)
emit += ' AC_REQUIRE([gl_CC_GNULIB_WARNINGS])\n'
diff --git a/pygnulib/GLTestDir.py b/pygnulib/GLTestDir.py
index e8c8f63faf..f6ab4f9d2e 100644
--- a/pygnulib/GLTestDir.py
+++ b/pygnulib/GLTestDir.py
@@ -497,13 +497,13 @@ class GLTestDir:
# autoconf snippets. It's cleanest to put those of the library before
# those of the tests.
emit += self.emitter.shellvars_init(True, f'../{sourcebase}')
- emit += self.emitter.autoconfSnippets(modules, modules,
- moduletable, 1, False, False, False,
- replace_auxdir)
+ emit += self.emitter.autoconfSnippets(modules, modules, moduletable,
+ lambda module: module.isNonTests(),
+ False, False, False, replace_auxdir)
emit += self.emitter.shellvars_init(True, '.')
- emit += self.emitter.autoconfSnippets(modules, modules,
- moduletable, 2, False, False, False,
- replace_auxdir)
+ emit += self.emitter.autoconfSnippets(modules, modules, moduletable,
+ lambda module: module.isTests(),
+ False, False, False, replace_auxdir)
emit += self.emitter.initmacro_end(macro_prefix, True)
# _LIBDEPS and _LTLIBDEPS variables are not needed if this library is
# created using libtool, because libtool already handles the
@@ -613,10 +613,12 @@ class GLTestDir:
emit += self.emitter.shellvars_init(False, sourcebase)
if single_configure:
emit += self.emitter.autoconfSnippets(main_modules, main_modules, moduletable,
- 0, True, False, False, replace_auxdir)
+ lambda module: True,
+ True, False, False, replace_auxdir)
else: # if not single_configure
emit += self.emitter.autoconfSnippets(modules, modules, moduletable,
- 1, True, False, False, replace_auxdir)
+ lambda module: module.isNonTests(),
+ True, False, False, replace_auxdir)
emit += self.emitter.initmacro_end(macro_prefix, False)
if single_configure:
emit += ' gltests_libdeps=\n'
@@ -630,7 +632,8 @@ class GLTestDir:
emit += ' m4_pushdef([gl_MODULE_INDICATOR_CONDITION], '
emit += '[$gl_module_indicator_condition])\n'
snippets = self.emitter.autoconfSnippets(tests_modules, main_modules + tests_modules,
- moduletable, 0, True, False, False, replace_auxdir)
+ moduletable, lambda module: True,
+ True, False, False, replace_auxdir)
emit += snippets
emit += ' m4_popdef([gl_MODULE_INDICATOR_CONDITION])\n'
emit += self.emitter.initmacro_end('%stests' % macro_prefix, True)
--
2.44.0
