gnulib-tool.py: Fix reading of 'gl_VC_FILES' in gnulib-cache.m4.

[Collin Funk]

This small patch fixes this diff in the import tests:

$ cat ../output.txt 
$ diff -u ./test-oath-toolkit-1.result/gl/m4/gnulib-cache.m4 
tmp27490-result/gl/m4/gnulib-cache.m4
--- ./test-oath-toolkit-1.result/gl/m4/gnulib-cache.m4  2024-03-29 
02:47:03.843697386 -0700
+++ tmp27490-result/gl/m4/gnulib-cache.m4       2024-03-29 20:02:25.589459017 
-0700
@@ -37,7 +37,6 @@
 #  --no-conditional-dependencies \
 #  --no-libtool \
 #  --macro-prefix=gl \
-#  --no-vc-files \
 #  autobuild \
 #  git-version-gen \
 #  gitlog-to-changelog \
@@ -64,4 +63,3 @@
 gl_MACRO_PREFIX([gl])
 gl_PO_DOMAIN([])
 gl_WITNESS_C_MACRO([])
-gl_VC_FILES([false])

Here is the fix inline because it is useful for explaining things
below:

diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index eb382cadac..81298eeca1 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -202,7 +202,7 @@ class GLImport(object):
                 self.cache.setPoDomain(cleaner(tempdict['gl_PO_DOMAIN']))
             if tempdict['gl_WITNESS_C_MACRO']:
                 
self.cache.setWitnessCMacro(cleaner(tempdict['gl_WITNESS_C_MACRO']))
-            if tempdict['gl_VC_FILES']:
+            if tempdict['gl_VC_FILES'] != '':
                 self.cache.setVCFiles(cleaner(tempdict['gl_VC_FILES']))
 
             # Get cached filelist from gnulib-comp.m4.

The tempdict is populated in these lines, where keys is a list of
accepted gl_* m4 macros:

              result = dict(pattern.findall(data))
              values = cleaner([ result.get(key, '')
                                 for key in keys ])
              tempdict = dict(zip(keys, values))


This creates a dictionary mapping the macro to the value inside of it:

     gl_MACRO([something-here]) -> { 'gl_MACRO' : 'something-here' }

or for a macro that isn't seen, an empty string is set as the value:

   { 'gl_UNSEEN_MACRO' : '' }

The problem occurs with that conditional because of the 'cleaner'
function:

    def cleaner(sequence: str | list[str]) -> str | list[str | bool]:
        '''Clean string or list of strings after using regex.'''
        if type(sequence) is str:
            sequence = sequence.replace('[', '')
            sequence = sequence.replace(']', '')
        elif type(sequence) is list:
            sequence = [ value.replace('[', '').replace(']', '')
                         for value in sequence]
            sequence = [ value.replace('(', '').replace(')', '')
                         for value in sequence]
            sequence = [ False if value == 'false' else value
                         for value in sequence ]
            sequence = [ True if value == 'true' else value
                         for value in sequence ]
            sequence = [ value.strip()
                         if type(value) is str else value
                         for value in sequence ]
        return sequence

So in the case that 'gnulib-comp.m4' does not contain any
'gl_VC_FILES' invocation, in tempdict we have:

    { 'gl_VC_FILES' : '' }

In this case the default value of None in GLConfig is correct, nothing
needs to be done. When we have 'gl_VC_FILES([true])' or
'gl_VC_FILES([false])':

    { 'gl_VC_FILES' : True }
    { 'gl_VC_FILES' : False }

Therefore, with the previous condition, 'gl_VC_FILES([false])' would
not update the GLConfig. It would be left at the default None, which
is incorrect.

I dislike this section of code. I remember previously 'value.strip()'
would be called in cleaner when value was a bool, which would cause it
to crash.

How about we remove that cleaner function and simply make the regular
expression more strict? It should make it behave more similar to
gnulib-tool.sh anyways. Something like this for an (untested) idea:

diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 81298eeca1..55fc87c202 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -121,6 +121,7 @@ class GLImport(object):
 
             # Create regex object and keys.
             pattern = re.compile(r'^(gl_.*?)\((.*?)\)$', re.S | re.M)
+            pattern = re.compile(r'^(gl_.*?)\([\[ ]*([^\]"\$`\\\)]*).*$', 
re.MULTILINE)
             keys = \
                 [
                     'gl_LOCAL_DIR', 'gl_MODULES', 'gl_AVOID', 'gl_SOURCE_BASE',

Collin
From afd9fbe4cc59c8b9b45859c7271f5cb12891453e Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Fri, 29 Mar 2024 20:15:24 -0700
Subject: [PATCH] gnulib-tool.py: Fix reading of 'gl_VC_FILES' in
 gnulib-cache.m4.

* pygnulib/GLImport.py (GLImport.__init__): Check for an empty string
explicitly in conditional so False is not ignored.
---
 ChangeLog            | 6 ++++++
 pygnulib/GLImport.py | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index eab98607a2..6d31a954c7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2024-03-29  Collin Funk  <collin.fu...@gmail.com>
+
+	gnulib-tool.py: Fix reading of 'gl_VC_FILES' in gnulib-cache.m4.
+	* pygnulib/GLImport.py (GLImport.__init__): Check for an empty string
+	explicitly in conditional so False is not ignored.
+
 2024-03-29  Paul Eggert  <egg...@cs.ucla.edu>
 
 	intprops: pacify GCC < 10 -Wsign-compare
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index eb382cadac..81298eeca1 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -202,7 +202,7 @@ class GLImport(object):
                 self.cache.setPoDomain(cleaner(tempdict['gl_PO_DOMAIN']))
             if tempdict['gl_WITNESS_C_MACRO']:
                 self.cache.setWitnessCMacro(cleaner(tempdict['gl_WITNESS_C_MACRO']))
-            if tempdict['gl_VC_FILES']:
+            if tempdict['gl_VC_FILES'] != '':
                 self.cache.setVCFiles(cleaner(tempdict['gl_VC_FILES']))
 
             # Get cached filelist from gnulib-comp.m4.
-- 
2.44.0