This patch fixes the extra line printed by gnulib-tool.py:

$ diff  -u test-oath-toolkit-4.out tmp758920-out
--- test-oath-toolkit-4.out     2024-03-28 13:17:54.441485990 -0700
+++ tmp758920-out       2024-03-28 14:36:28.290120583 -0700
@@ -194,5 +194,6 @@
   - mention "gl" in SUBDIRS in Makefile.am,
   - mention "-I gl/m4" in ACLOCAL_AMFLAGS in Makefile.am
     or add an AC_CONFIG_MACRO_DIRS([gl/m4]) invocation in ./configure.ac,
+  - mention "m4/gnulib-cache.m4" in EXTRA_DIST in gl/Makefile.am,
   - invoke gl_EARLY in ./configure.ac, right after AC_PROG_CC,
   - invoke gl_INIT in ./configure.ac.

The removal of variables from the GLMakefileTable was incorrect. The
'.pop()' call was on a local variable dictionary. I fixed it by adding
a pop function taking the index to GLMakefileTable. This is just a
wrapper function around list.pop().

Since we need the GLMakefileTable between function calls, I've added
it as an instance variable to GLEmiter. This also means removing it
from other classes which passed it as an argument.

It has been a while since I've used OOP, but this relationship seems
more correct to me anyways.

Collin
From 3ec8e0f47a0e0e59986c0fccef4216f26d763b2e Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Thu, 28 Mar 2024 14:33:27 -0700
Subject: [PATCH] gnulib-tool.py: Fix removal of variables from
 GLMakefileTable.

* pygnulib/GLEmiter.py (GLEmiter.__init__): Add GLMakefileTable as a
instance variable so it can be modified and accessed between function
calls.
(GLEmiter.lib_Makefile_am): Remove makefiletable argument and use the
class instance. Remove variables from the makefiletable once emitted.
(GLEmiter.tests_Makefile_am): Likewise.
* pygnulib/GLMakefileTable.py (GLMakefileTable.pop): New function.
* pygnulib/GLImport.py (GLImport.__init__): Remove GLMakefileTable
instance variable.
(GLImport.execute): Update calls to GLEmiter functions.
* pygnulib/GLTestDir.py (GLTestDir.__init__, GLMegaTestDir.__init__):
Remove GLMakefileTable instance variable.
(GLTestDir.execute): Update calls to GLEmiter functions.
---
 ChangeLog                   | 17 +++++++++++++++++
 pygnulib/GLEmiter.py        | 29 +++++++++++++----------------
 pygnulib/GLImport.py        | 19 +++++++++----------
 pygnulib/GLMakefileTable.py |  6 ++++++
 pygnulib/GLTestDir.py       | 10 ++++------
 5 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 797dd77c62..e7db6905bc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2024-03-28  Collin Funk  <collin.fu...@gmail.com>
+
+	gnulib-tool.py: Fix removal of variables from GLMakefileTable.
+	* pygnulib/GLEmiter.py (GLEmiter.__init__): Add GLMakefileTable as a
+	instance variable so it can be modified and accessed between function
+	calls.
+	(GLEmiter.lib_Makefile_am): Remove makefiletable argument and use the
+	class instance. Remove variables from the makefiletable once emitted.
+	(GLEmiter.tests_Makefile_am): Likewise.
+	* pygnulib/GLMakefileTable.py (GLMakefileTable.pop): New function.
+	* pygnulib/GLImport.py (GLImport.__init__): Remove GLMakefileTable
+	instance variable.
+	(GLImport.execute): Update calls to GLEmiter functions.
+	* pygnulib/GLTestDir.py (GLTestDir.__init__, GLMegaTestDir.__init__):
+	Remove GLMakefileTable instance variable.
+	(GLTestDir.execute): Update calls to GLEmiter functions.
+
 2024-03-28  Bruno Haible  <br...@clisp.org>
 
 	havelib: Recognize ELF platform despite SunPRO C on Linux.
diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index efde78823d..56ccdf66ae 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -118,6 +118,7 @@ class GLEmiter(object):
             raise TypeError('config must be a GLConfig, not %s'
                             % type(config).__name__)
         self.config = config
+        self.makefiletable = GLMakefileTable(self.config)
 
     def __repr__(self) -> str:
         '''x.__repr__() <==> repr(x)'''
@@ -698,7 +699,7 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         return emit
 
     def lib_Makefile_am(self, destfile: str, modules: list[GLModule], moduletable: GLModuleTable,
-                        makefiletable: GLMakefileTable, actioncmd: str, for_test: bool) -> tuple[str, bool]:
+                        actioncmd: str, for_test: bool) -> tuple[str, bool]:
         '''Emit the contents of the library Makefile. Returns str and a bool
         variable which shows if subdirectories are used.
         GLConfig: localpath, sourcebase, libname, pobase, auxdir, makefile_name, libtool,
@@ -707,7 +708,6 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         destfile is a filename relative to destdir of Makefile being generated.
         modules is a list of GLModule instances.
         moduletable is a GLModuleTable instance.
-        makefiletable is a GLMakefileTable instance.
         actioncmd is a string variable, which represents the actioncmd; it can be
           an empty string e.g. when user wants to generate files for GLTestDir.
         for_test is a bool variable; it must be set to True if creating a package
@@ -721,9 +721,6 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         if type(moduletable) is not GLModuleTable:
             raise TypeError('moduletable must be a GLModuleTable, not %s'
                             % type(moduletable).__name__)
-        if type(makefiletable) is not GLMakefileTable:
-            raise TypeError('makefiletable must be a GLMakefileTable, not %s'
-                            % type(makefiletable).__name__)
         if type(actioncmd) is not str:
             raise TypeError('actioncmd must be a string, not %s'
                             % type(actioncmd).__name__)
@@ -902,8 +899,9 @@ AC_DEFUN([%V1%_LIBSOURCES], [
             emit += '# No GNU Make output.\n'
 
         # Execute edits that apply to the Makefile.am being generated.
-        for current_edit in range(0, makefiletable.count()):
-            dictionary = makefiletable[current_edit]
+        current_edit = 0
+        while current_edit < self.makefiletable.count():
+            dictionary = self.makefiletable[current_edit]
             if dictionary['var']:
                 if destfile == joinpath(dictionary['dir'], 'Makefile.am'):
                     val = dictionary['val']
@@ -912,7 +910,8 @@ AC_DEFUN([%V1%_LIBSOURCES], [
                         # Since we don't have '.' among SUBDIRS so far, add it now.
                         val = f'. {val}'
                     emit += '%s += %s\n' % (dictionary['var'], val)
-                    dictionary.pop('var')
+                    self.makefiletable.pop(current_edit)
+            current_edit += 1
 
         # Define two parts of cppflags variable.
         cppflags_part1 = ''
@@ -1007,7 +1006,7 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         return result
 
     def tests_Makefile_am(self, destfile: str, modules: list[GLModule], moduletable: GLModuleTable,
-                          makefiletable: GLMakefileTable, witness_macro: str, for_test: bool) -> tuple[str, bool]:
+                          witness_macro: str, for_test: bool) -> tuple[str, bool]:
         '''Emit the contents of the tests Makefile. Returns str and a bool variable
         which shows if subdirectories are used.
         GLConfig: localpath, modules, libname, auxdir, makefile_name, libtool,
@@ -1018,7 +1017,6 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         witness_macro is a string which represents witness_c_macro with the suffix.
         modules is a list of GLModule instances.
         moduletable is a GLModuleTable instance.
-        makefiletable is a GLMakefileTable instance.
         actioncmd is a string variable, which represents the actioncmd; it can be
           an empty string e.g. when user wants to generate files for GLTestDir.
         for_test is a bool variable; it must be set to True if creating a package
@@ -1029,9 +1027,6 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         for module in modules:
             if type(module) is not GLModule:
                 raise TypeError('each module must be a GLModule instance')
-        if type(makefiletable) is not GLMakefileTable:
-            raise TypeError('makefiletable must be a GLMakefileTable, not %s'
-                            % type(makefiletable).__name__)
         if type(witness_macro) is not str:
             raise TypeError('witness_macro must be a string, not %s'
                             % type(witness_macro).__name__)
@@ -1217,8 +1212,9 @@ AC_DEFUN([%V1%_LIBSOURCES], [
         emit += 'MAINTAINERCLEANFILES =\n'
 
         # Execute edits that apply to the Makefile.am being generated.
-        for current_edit in range(0, makefiletable.count()):
-            dictionary = makefiletable[current_edit]
+        current_edit = 0
+        while current_edit < self.makefiletable.count():
+            dictionary = self.makefiletable[current_edit]
             if dictionary['var']:
                 if destfile == joinpath(dictionary['dir'], 'Makefile.am'):
                     val = dictionary['val']
@@ -1227,7 +1223,8 @@ AC_DEFUN([%V1%_LIBSOURCES], [
                         # But we have '.' among SUBDIRS already, so do nothing.
                         pass
                     emit += '%s += %s\n' % (dictionary['var'], val)
-                    dictionary.pop('var')
+                    self.makefiletable.pop(current_edit)
+            current_edit += 1
 
         emit += '\n'
 
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 07be6742a1..1a04285bfb 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -294,7 +294,6 @@ class GLImport(object):
         self.moduletable = GLModuleTable(self.config,
                                          self.config.checkInclTestCategory(TESTS['all-tests']),
                                          self.config.checkInclTestCategory(TESTS['all-tests']))
-        self.makefiletable = GLMakefileTable(self.config)
 
     def __repr__(self) -> str:
         '''x.__repr__ <==> repr(x)'''
@@ -1158,18 +1157,18 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
         if source_makefile_am == 'Makefile.am':
             sourcebase_dir = os.path.dirname(sourcebase)
             sourcebase_base = os.path.basename(sourcebase)
-            self.makefiletable.editor(sourcebase_dir, 'SUBDIRS', sourcebase_base)
+            self.emitter.makefiletable.editor(sourcebase_dir, 'SUBDIRS', sourcebase_base)
         if pobase:
             pobase_dir = os.path.dirname(pobase)
             pobase_base = os.path.basename(pobase)
-            self.makefiletable.editor(pobase_dir, 'SUBDIRS', pobase_base)
+            self.emitter.makefiletable.editor(pobase_dir, 'SUBDIRS', pobase_base)
         if self.config.checkInclTestCategory(TESTS['tests']):
             if tests_makefile_am == 'Makefile.am':
                 testsbase_dir = os.path.dirname(testsbase)
                 testsbase_base = os.path.basename(testsbase)
-                self.makefiletable.editor(testsbase_dir, 'SUBDIRS', testsbase_base, True)
-        self.makefiletable.editor('', 'ACLOCAL_AMFLAGS', m4base)
-        self.makefiletable.parent(gentests, source_makefile_am, tests_makefile_am)
+                self.emitter.makefiletable.editor(testsbase_dir, 'SUBDIRS', testsbase_base, True)
+        self.emitter.makefiletable.editor('', 'ACLOCAL_AMFLAGS', m4base)
+        self.emitter.makefiletable.parent(gentests, source_makefile_am, tests_makefile_am)
 
         # Create po/ directory.
         filesystem = GLFileSystem(self.config)
@@ -1331,7 +1330,7 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
         basename = joinpath(sourcebase, source_makefile_am)
         tmpfile = self.assistant.tmpfilename(basename)
         emit, uses_subdirs = self.emitter.lib_Makefile_am(basename,
-                                                          self.moduletable['main'], self.moduletable, self.makefiletable,
+                                                          self.moduletable['main'], self.moduletable,
                                                           actioncmd, for_test)
         if automake_subdir:
             emit = sp.run([joinpath(DIRS['root'], 'build-aux/prefix-gnulib-mk'), '--from-gnulib-tool',
@@ -1359,7 +1358,7 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
             basename = joinpath(testsbase, tests_makefile_am)
             tmpfile = self.assistant.tmpfilename(basename)
             emit, uses_subdirs = self.emitter.tests_Makefile_am(basename,
-                                                                self.moduletable['tests'], self.moduletable, self.makefiletable,
+                                                                self.moduletable['tests'], self.moduletable,
                                                                 '%stests_WITNESS' % macro_prefix, for_test)
             with codecs.open(tmpfile, 'wb', 'UTF-8') as file:
                 file.write(emit)
@@ -1477,9 +1476,9 @@ in <library>_a_LDFLAGS or <library>_la_LDFLAGS when linking a library.''')
                 print('  - "include %s" from within "%s/Makefile.am",' % (tests_makefile_am, testsbase))
         # Print makefile edits.
         current_edit = 0
-        makefile_am_edits = self.makefiletable.count()
+        makefile_am_edits = self.emitter.makefiletable.count()
         while current_edit != makefile_am_edits:
-            dictionary = self.makefiletable[current_edit]
+            dictionary = self.emitter.makefiletable[current_edit]
             if dictionary['var']:
                 if dictionary['var'] == 'ACLOCAL_AMFLAGS':
                     print('  - mention "-I %s" in %s in %s'
diff --git a/pygnulib/GLMakefileTable.py b/pygnulib/GLMakefileTable.py
index 17a35ee935..48c3267962 100644
--- a/pygnulib/GLMakefileTable.py
+++ b/pygnulib/GLMakefileTable.py
@@ -62,6 +62,12 @@ class GLMakefileTable(object):
         result = self.table[y]
         return dict(result)
 
+    def pop(self, index: int) -> dict[str, bool]:
+        '''Remove the given index from the Makefile table. The removed element is returned.'''
+        if type(index) is not int:
+            raise TypeError(f'indices must be integers, not {type(index).__name__}')
+        return self.table.pop(index)
+
     def editor(self, dir: str, var: str, val: str, dotfirst: bool = False) -> None:
         '''This method is used to remember that ${dir}Makefile.am needs to be edited
         to that ${var} mentions ${val}.
diff --git a/pygnulib/GLTestDir.py b/pygnulib/GLTestDir.py
index aa9a6b82ea..932f846c96 100644
--- a/pygnulib/GLTestDir.py
+++ b/pygnulib/GLTestDir.py
@@ -117,7 +117,6 @@ class GLTestDir(object):
         self.filesystem = GLFileSystem(self.config)
         self.modulesystem = GLModuleSystem(self.config)
         self.assistant = GLFileAssistant(self.config)
-        self.makefiletable = GLMakefileTable(self.config)
 
         # Subdirectory names.
         self.config.setSourceBase('gllib')
@@ -397,10 +396,10 @@ class GLTestDir(object):
         destfile = joinpath(directory, 'Makefile.am')
         if single_configure:
             emit, uses_subdirs = self.emitter.lib_Makefile_am(destfile, main_modules,
-                                                              moduletable, self.makefiletable, '', for_test)
+                                                              moduletable, '', for_test)
         else:  # if not single_configure
             emit, uses_subdirs = self.emitter.lib_Makefile_am(destfile, modules,
-                                                              moduletable, self.makefiletable, '', for_test)
+                                                              moduletable, '', for_test)
         with codecs.open(destfile, 'wb', 'UTF-8') as file:
             file.write(emit)
         any_uses_subdirs = uses_subdirs
@@ -434,7 +433,7 @@ class GLTestDir(object):
                 destfile = joinpath(directory, 'Makefile.am')
                 witness_macro = '%stests_WITNESS' % macro_prefix
                 emit, uses_subdirs = self.emitter.tests_Makefile_am(destfile, tests_modules, moduletable,
-                                                                    self.makefiletable, witness_macro, for_test)
+                                                                    witness_macro, for_test)
                 with codecs.open(destfile, 'wb', 'UTF-8') as file:
                     file.write(emit)
             else:  # if not single_configure
@@ -443,7 +442,7 @@ class GLTestDir(object):
                 libtests = False
                 self.config.setLibtests(False)
                 emit, uses_subdirs = self.emitter.tests_Makefile_am(destfile, modules, moduletable,
-                                                                    self.makefiletable, '', for_test)
+                                                                    '', for_test)
                 with codecs.open(destfile, 'wb', 'UTF-8') as file:
                     file.write(emit)
                 # Viewed from the $testsbase subdirectory, $auxdir is different.
@@ -922,7 +921,6 @@ class GLMegaTestDir(object):
         self.filesystem = GLFileSystem(self.config)
         self.modulesystem = GLModuleSystem(self.config)
         self.assistant = GLFileAssistant(self.config)
-        self.makefiletable = GLMakefileTable(self.config)
 
     def execute(self) -> None:
         '''Create a mega scratch package with the given modules one by one
-- 
2.44.0

Reply via email to