Hi Collin,
> I was not a fan of adding another value to the left side of this
> assignment:
>
> emit, uses_subdirs = self.emitter.lib_Makefile_am(basename, ...)
Why not? This is perfectly normal functional programming style.
Lisp has it ("multiple values"), ISO C has it ("struct" as return value),
Python has it ("tuplie"), ...
> I've attached an updated patch.
Looks better, but I think it has a mistake: The loop is
- using an upwards iteration (from 0 to count()-1),
- removing elements from the list in the loop,
- incrementing current_edit at each round in the loop.
For example, here: If current_edit is 1, and we remove the element
at index 1:
>>> l = ['a','b','c']
>>> l.pop(1)
'b'
>>> l
['a', 'c']
we need to continue at index 1, not index 2. Otherwise we miss the 'c'.
Also, the
if dictionary['var']:
lines that were meant to detect already-handled entries can now be dropped.
But I kept wondering why the original code is not working. The reason is that
each makefiletable[index] access creates a clone of the dict object, and thus
dictionary.pop('var')
has an effect on the dictionary variable but not on the makefiletable.
Once I fix this, I get a KeyError from
if dictionary['var']:
indicating that this code never worked.
I'm applying my minimal fix. If you feel that marking an entry as removed by
removing the 'var' key is not so good, and your way of doing it by actually
removing the entry from the table is better, please provide a correct patch
to do so.
2024-03-29 Bruno Haible <br...@clisp.org>
gnulib-tool.py: Don't print Makefile.am edits that are already done.
* pygnulib/GLMakefileTable.py (GLMakefileTable): Improve comments.
(GLMakefileTable.__getitem__): Do not clone the result.
* pygnulib/GLEmiter.py (GLEmiter.lib_Makefile_am, tests_Makefile_am):
Avoid a KeyError when testing for 'var'.
Use 'del' to remove a dictionary entry.
* pygnulib/GLImport.py (GLImport.execute): Avoid a KeyError when
testing for 'var'. Simplify loop over makefiletable.
diff --git a/pygnulib/GLMakefileTable.py b/pygnulib/GLMakefileTable.py
index 17a35ee935..ba63edb6cb 100644
--- a/pygnulib/GLMakefileTable.py
+++ b/pygnulib/GLMakefileTable.py
@@ -44,7 +44,11 @@ joinpath = constants.joinpath
#===============================================================================
class GLMakefileTable(object):
'''This class is used to edit Makefile and store edits as table.
- When user creates Makefile, he may need to use this class.'''
+ When user creates Makefile.am, he may need to use this class.
+ The internal representation consists of a list of edits.
+ Each edit is a dictionary with keys 'dir', 'var', 'val', 'dotfirst'.
+ An edit may be removed; this is done by removing its 'var' key but
+ keeping it in the list. Removed edits must be ignored.'''
def __init__(self, config: GLConfig) -> None:
'''Create GLMakefileTable instance.'''
@@ -60,7 +64,9 @@ class GLMakefileTable(object):
raise TypeError('indices must be integers, not %s'
% type(y).__name__)
result = self.table[y]
- return dict(result)
+ # Do *not* clone the result here. We want GLEmiter to be able to make
+ # side effects on the result.
+ return result
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
@@ -107,5 +113,5 @@ class GLMakefileTable(object):
self.editor(dir1, 'EXTRA_DIST', joinpath(dir2, 'gnulib-cache.m4'))
def count(self) -> int:
- '''Count number of edits which were applied.'''
+ '''Count number of edits which are stored, including the removed
ones.'''
return len(self.table)
diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index 0bfbfe10e3..7211e24cbe 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -895,7 +895,7 @@ AC_DEFUN([%V1%_LIBSOURCES], [
# Execute edits that apply to the Makefile.am being generated.
for current_edit in range(0, makefiletable.count()):
dictionary = makefiletable[current_edit]
- if dictionary['var']:
+ if 'var' in dictionary:
if destfile == joinpath(dictionary['dir'], 'Makefile.am'):
val = dictionary['val']
if dictionary['var'] == 'SUBDIRS' and
dictionary['dotfirst']:
@@ -903,7 +903,7 @@ 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')
+ del dictionary['var']
# Define two parts of cppflags variable.
cppflags_part1 = ''
@@ -1198,7 +1198,7 @@ AC_DEFUN([%V1%_LIBSOURCES], [
# Execute edits that apply to the Makefile.am being generated.
for current_edit in range(0, makefiletable.count()):
dictionary = makefiletable[current_edit]
- if dictionary['var']:
+ if 'var' in dictionary:
if destfile == joinpath(dictionary['dir'], 'Makefile.am'):
val = dictionary['val']
if dictionary['var'] == 'SUBDIRS' and
dictionary['dotfirst']:
@@ -1206,7 +1206,7 @@ AC_DEFUN([%V1%_LIBSOURCES], [
# But we have '.' among SUBDIRS already, so do nothing.
pass
emit += '%s += %s\n' % (dictionary['var'], val)
- dictionary.pop('var')
+ del dictionary['var']
emit += '\n'
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 5137f07e37..afe751dbd3 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -1466,11 +1466,9 @@ in <library>_a_LDFLAGS or <library>_la_LDFLAGS when
linking a library.''')
else: # if makefile_am != 'Makefile.am'
print(' - "include %s" from within "%s/Makefile.am",' %
(tests_makefile_am, testsbase))
# Print makefile edits.
- current_edit = 0
- makefile_am_edits = self.makefiletable.count()
- while current_edit != makefile_am_edits:
+ for current_edit in range(0, self.makefiletable.count()):
dictionary = self.makefiletable[current_edit]
- if dictionary['var']:
+ if 'var' in dictionary:
if dictionary['var'] == 'ACLOCAL_AMFLAGS':
print(' - mention "-I %s" in %s in %s'
% (dictionary['val'], dictionary['var'],
joinpath(dictionary['dir'], 'Makefile.am')))
@@ -1479,7 +1477,6 @@ in <library>_a_LDFLAGS or <library>_la_LDFLAGS when
linking a library.''')
else:
print(' - mention "%s" in %s in %s,'
% (dictionary['val'], dictionary['var'],
joinpath(dictionary['dir'], 'Makefile.am')))
- current_edit += 1
# Detect position_early_after.
with codecs.open(configure_ac, 'rb', 'UTF-8') as file:
