Re: gnulib-tool.py: Don't fail when given a bad module name.
Hi Bruno, On 4/21/24 4:37 AM, Bruno Haible wrote: > The former makes an implicit statement that the function is searching > for the file. The latter does not; it does not say or hint to as what > function internally does. Thus it is better to use the latter wording. Agreed, thanks for the change and advice. My documentation skills are a bit lacking... > It says "Stop." twice but does not actually stop :) Fixed as follows. Nice! Running this passes for me now: $ env GNULIB_TOOL_IMPL=sh+py gnulib-tool --create-testdir --dir aaa abc Collin
Re: gnulib-tool.py: Don't fail when given a bad module name.
Hi Collin, > When using --create-testdir with a bad module name: > > $ gnulib-tool.py --create-testdir --dir aaa bad-module-name > gnulib-tool: warning: module bad-module-name doesn't exist > Traceback (most recent call last): > File "/home/collin/.local/src/gnulib/.gnulib-tool.py", line 30, in > main.main_with_exception_handling() > File "/home/collin/.local/src/gnulib/pygnulib/main.py", line 1382, in > main_with_exception_handling > main() > File "/home/collin/.local/src/gnulib/pygnulib/main.py", line 1075, in main > testdir.execute() > File "/home/collin/.local/src/gnulib/pygnulib/GLTestDir.py", line 209, in > execute > requested_licence = requested_module.getLicense() > ^^^ > AttributeError: 'NoneType' object has no attribute 'getLicense' > > This patch removes None from the list of GLModule objects before using > them. Good point. Thanks, patch applied with one tweak: > I also documented the meaning of a None return in > GLModuleSystem.find() since this problem occurred in GLImport > previously. When documenting a function or program, it is sometimes possible to state the same condition with and without referring to the internals of the function / program. It is better to document it without reference to the internals. In this case, one can say if the module description file could not be found or if the module description file does not exist The former makes an implicit statement that the function is searching for the file. The latter does not; it does not say or hint to as what function internally does. Thus it is better to use the latter wording. I also tried the behaviour on gnulib-tool.sh, and got this confusing output: $ ./gnulib-tool.sh --create-testdir --dir=../testdir2 --single-configure foobar signbit ./gnulib-tool.sh: *** file /GNULIB/gnulib-git/./modules/foobar not found ./gnulib-tool.sh: *** Stop. gnulib-tool: warning: module foobar lacks a License ./gnulib-tool.sh: *** file /GNULIB/gnulib-git/./modules/foobar not found ./gnulib-tool.sh: *** Stop. gnulib-tool: warning: module foobar doesn't exist gnulib-tool: warning: module foobar doesn't exist Module list with included dependencies (indented): absolute-header c99 extern-inline ... It says "Stop." twice but does not actually stop :) Fixed as follows. 2024-04-21 Bruno Haible gnulib-tool.sh: In --create-testdir, just warn about a bad module name. * gnulib-tool.sh (func_create_testdir): Validate the modules list. diff --git a/gnulib-tool.sh b/gnulib-tool.sh index 5f0c6f530e..b486e99b1e 100755 --- a/gnulib-tool.sh +++ b/gnulib-tool.sh @@ -6504,6 +6504,9 @@ func_create_testdir () # Except lib-ignore, which leads to link errors when Sun C++ is used. FIXME. modules=`func_all_modules` modules=`for m in $modules; do case $m in config-h | non-recursive-gnulib-prefix-hack | timevar | mountlist | lib-ignore) ;; *) echo $m;; esac; done` + else +# Validate the list of specified modules. +modules=`for module in $modules; do func_verify_module; if test -n "$module"; then echo "$module"; fi; done` fi specified_modules="$modules"
gnulib-tool.py: Don't fail when given a bad module name.
When using --create-testdir with a bad module name:
$ gnulib-tool.py --create-testdir --dir aaa bad-module-name
gnulib-tool: warning: module bad-module-name doesn't exist
Traceback (most recent call last):
File "/home/collin/.local/src/gnulib/.gnulib-tool.py", line 30, in
main.main_with_exception_handling()
File "/home/collin/.local/src/gnulib/pygnulib/main.py", line 1382, in
main_with_exception_handling
main()
File "/home/collin/.local/src/gnulib/pygnulib/main.py", line 1075, in main
testdir.execute()
File "/home/collin/.local/src/gnulib/pygnulib/GLTestDir.py", line 209, in
execute
requested_licence = requested_module.getLicense()
^^^
AttributeError: 'NoneType' object has no attribute 'getLicense'
This patch removes None from the list of GLModule objects before using
them.
I also documented the meaning of a None return in
GLModuleSystem.find() since this problem occurred in GLImport
previously.
CollinFrom f9ff066e21adcd12de167ceea5a4c64a1f012a6b Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Sat, 20 Apr 2024 20:29:44 -0700
Subject: [PATCH] gnulib-tool.py: Don't fail when given a bad module name.
* pygnulib/GLTestDir.py (GLTestDir.execute): Don't include None in the
list of GLModule objects.
* pygnulib/GLModuleSystem.py (GLModuleSystem.find): Document the meaning
of the None return.
---
ChangeLog | 8
pygnulib/GLModuleSystem.py | 4 +++-
pygnulib/GLTestDir.py | 9 ++---
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 2ede5219a3..6523c89b87 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2024-04-20 Collin Funk
+
+ gnulib-tool.py: Don't fail when given a bad module name.
+ * pygnulib/GLTestDir.py (GLTestDir.execute): Don't include None in the
+ list of GLModule objects.
+ * pygnulib/GLModuleSystem.py (GLModuleSystem.find): Document the meaning
+ of the None return.
+
2024-04-20 Collin Funk
gnulib-tool.py: Update type hints and docstring.
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index 301592a79d..daee7b403e 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -88,7 +88,9 @@ def exists(self, module: str) -> bool:
return result
def find(self, module: str) -> GLModule | None:
-'''Find the given module.'''
+'''Return the GLModule object given the module name. If the module
+description file could not be found None is returned.
+- module, The name of the module.'''
if type(module) is not str:
raise TypeError('module must be a string, not %s'
% type(module).__name__)
diff --git a/pygnulib/GLTestDir.py b/pygnulib/GLTestDir.py
index 8bbef2a311..9673a5bd26 100644
--- a/pygnulib/GLTestDir.py
+++ b/pygnulib/GLTestDir.py
@@ -185,9 +185,12 @@ def execute(self) -> None:
'mountlist', 'lib-ignore']]
# Canonicalize the list of specified modules.
-specified_modules = sorted(set(specified_modules))
-specified_modules = [ self.modulesystem.find(m)
- for m in specified_modules ]
+modules = set()
+for name in specified_modules:
+module = self.modulesystem.find(name)
+if module is not None:
+modules.add(module)
+specified_modules = sorted(modules)
# Test modules which invoke AC_CONFIG_FILES cannot be used with
# --with-tests --single-configure. Avoid them.
--
2.44.0
