[PATCH] gnulib-tool.py: Adjust construction of configure.ac path.

[Collin Funk]

When using gnulib-tool.py this comment in gnulib-comp.m4 is different
(using Emacs merge-gnulib):

-# This macro should be invoked from ./configure.ac, in the section
+# This macro should be invoked from configure.ac, in the section

In gnulib-tool the $configure_ac is set to [1]:

   configure_ac="$destdir/configure.ac"

but gnulib-tool.py uses relativize which modifies the path. The
GLConfig.resetAutoconfFile is unused (based on a quick grep search)
and uses joinpath. This does path normalization so it also gives the
incorrect result.

This patch just changes GLConfig.setAutoconfFile to use os.path.join
which seems most similar and fixes the output.

I think that this may _still_ give the incorrect output in some
situations. This test program:

import os
print(os.path.join('.//', 'configure.ac'))

prints (on GNU/Linux and most likely anything but Windows):

.//configure.ac

where gnulib-tool would give (assuming unsanitized input):

.///configure.ac

Feel free to propose a better solution if you think of one. I'm not
too sure how $destdir is sanitized or if we assume it doesn't contain
path separators.

[1] https://git.savannah.gnu.org/cgit/gnulib.git/tree/gnulib-tool#n1597

CollinFrom 0343d44ada1c1b0d50ad4b90efa10d596f03c5b0 Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sun, 3 Mar 2024 16:00:57 -0800
Subject: [PATCH] gnulib-tool.py: Adjust construction of configure.ac path.

* pygnulib/GLConfig.py (GLConfig.setAutoconfFile): Join destdir and
configure.ac instead of using relativize.
---
 ChangeLog| 6 ++
 pygnulib/GLConfig.py | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 098868de10..75925373ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2024-03-03  Collin Funk  
+
+	gnulib-tool.py: Adjust construction of configure.ac path.
+	* pygnulib/GLConfig.py (GLConfig.setAutoconfFile): Join destdir and
+	configure.ac instead of using relativize.
+
 2024-03-03  Collin Funk  
 Bruno Haible  
 
diff --git a/pygnulib/GLConfig.py b/pygnulib/GLConfig.py
index 4284c73ad9..1689e867b0 100644
--- a/pygnulib/GLConfig.py
+++ b/pygnulib/GLConfig.py
@@ -927,7 +927,7 @@ class GLConfig(object):
 if type(configure_ac) is str:
 if configure_ac:
 self.table['configure_ac'] = \
-relpath(self.table['destdir'], configure_ac)
+os.path.join(self.table['destdir'], configure_ac)
 else:  # if type of configure_ac is not str
 raise TypeError('configure_ac must be a string, not %s'
 % type(configure_ac).__name__)
-- 
2.44.0

Re: --create-testdir speed

[Tim Rice]

On Sun, 3 Mar 2024, Bruno Haible wrote:

> +@example
> +gnulib-tool --create-megatestdir --with-tests --dir=... autobuild MODULES
> +@end example
> +
> +@noindent
> +(You can't do this will all of Gnulib at once: @code{gnulib-tool} would run
  
I assume the brain wanted to type with but the fingers did not comply.

> +for a week and produce a directory that takes more than 100 GB, maybe even 1 
> TB,
> +of disk space.)
>  
>  @item Transfer gnulib directory
>  

-- 
Tim RiceMultitalents
t...@multitalents.net

Re: [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 39.

[Collin Funk]

On 3/3/24 4:30 AM, Pádraig Brady wrote:
> Right, join without options is essentially set intersection.
> See https://www.pixelbeat.org/cmdline.html#sets

Thanks for the examples. Bookmarked it since it will probably come
handy as I work on gnulib-tool. :)

Collin

Re: [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 39.

[Collin Funk]

On 3/3/24 4:30 AM, Bruno Haible wrote:
> In the original commit, I removed the error message
> "option --conditional-dependencies is not supported with --with-tests"
> only at one place (the command-line option checks), but left it in 
> func_import.
> You are removing it in both places (mode == 'import' as well as mode != 
> 'import).

Oops, I just noticed the ChangeLog says (Options) next to "Don't
reject the combination of --conditional-dependencies with
--with-tests." For some reason my brain ignored that part. Thanks for
the catch.

Collin

Re: [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 39.

[Bruno Haible]

Hello Collin,

> -for m in $modules; do echo $m; done | LC_ALL=C sort -u > "$tmp"/modules
> +for m in $referenceable_modules; do echo $m; done | LC_ALL=C sort -u > 
> "$tmp"/modules
> 
> Which I didn't understand until seeing this line [1]:
> 
> deps=`for m in $deps; do echo $m; done | LC_ALL=C sort -u | LC_ALL=C join - 
> "$tmp"/modules`
> 
> This just means that join takes stdin as its first file which is the
> piped output of sort. Then the second file is the sorted
> referenceable_modules stored in "$tmp"/modules right?

Correct.

> If I am interpreting that correctly then the lines I used would be
> correct I think:
> 
> depmodules = sorted(set(depmodules).intersection(referenceable_modules))

Yes, correct.

> + Follow gnulib-tool change
> + 2017-12-28  Bruno Haible  
> + gnulib-tool: Make --conditional-dependencies work better.
> -commit 589e96475f8f2d21a83405ab0672ce95091b80e5

In the original commit, I removed the error message
"option --conditional-dependencies is not supported with --with-tests"
only at one place (the command-line option checks), but left it in func_import.
You are removing it in both places (mode == 'import' as well as mode != 
'import).

The rest of the patch looks fine.

Bruno

Re: [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 39.

[Pádraig Brady]

On 03/03/2024 05:33, Collin Funk wrote:

On 3/2/24 7:02 PM, Collin Funk wrote:

When working on the one of the TODO entries I noticed that
the gnulib-comp.m4 output was incorrect.


Here is the item I was working on. I _think_ I understood it correctly
but you guys are much more talented at shell than me. :)

Specifically, I was a bit confused at first by the
func_emit_autoconf_snippets function. The diff showed the following
change:

-for m in $modules; do echo $m; done | LC_ALL=C sort -u > "$tmp"/modules
+for m in $referenceable_modules; do echo $m; done | LC_ALL=C sort -u > 
"$tmp"/modules

Which I didn't understand until seeing this line [1]:

deps=`for m in $deps; do echo $m; done | LC_ALL=C sort -u | LC_ALL=C join - 
"$tmp"/modules`

This just means that join takes stdin as its first file which is the
piped output of sort. Then the second file is the sorted
referenceable_modules stored in "$tmp"/modules right?

If I am interpreting that correctly then the lines I used would be
correct I think:

depmodules = sorted(set(depmodules).intersection(referenceable_modules))

The merge-gnulib script output seems more correct than usual, but I
could be missing something.

[1] 
https://git.savannah.gnu.org/cgit/gnulib.git/tree/gnulib-tool?id=589e96475f8f2d21a83405ab0672ce95091b80e5#n4296

Collin


Right, join without options is essentially set intersection.
See https://www.pixelbeat.org/cmdline.html#sets

cheers,
Pádraig

Re: [PATCH] gnulib-tool.py: Fix output of gnulib-comp.m4.

[Collin Funk]

Hi Bruno,

On 3/3/24 2:34 AM, Bruno Haible wrote:
> There are no problems with writing "condition != True". Remember the rule?
> 'condition' as a variable can hold the values None, True, or a string.
> Thus it has to be compared with == or !=.

Yes, I was overthinking it. :)

> "condition != True" is more future-proof than "type(condition) is str",
> because
>   - It expresses the intent more precisely. We are making an optimization
> that consists in simplifying
> 
>   if true; then
> STATEMENT
>   fi
> 
> to
> 
>   STATEMENT
> 
>   - We may want to allow more complicated conditions in the future, that
> may be represented as, say, list of strings instead of a simple string.
> 
> I've therefore applied a modified patch:

Good point. Thanks for the explanation and fix.

Collin

Re: [PATCH] gnulib-tool.py: Fix output of gnulib-comp.m4.

[Bruno Haible]

Hi Collin,

> When working on the one of the TODO entries I noticed that
> the gnulib-comp.m4 output was incorrect. The indentation was slightly
> off and the output also had "if True;" conditionals. Here is a diff
> from Emacs merge-gnulib to show the difference:
> 
> @@ -784,8 +785,12 @@ AC_DEFUN
>  if $gl_gnulib_enabled_8444034ea779b88768865bb60b4fb8c9; then :; else
>AC_PROG_MKDIR_P
>gl_gnulib_enabled_8444034ea779b88768865bb60b4fb8c9=true
> -  func_gl_gnulib_m4code_ef455225c00f5049c808c2eda3e76866
> -  func_gl_gnulib_m4code_61bcaca76b3e6f9ae55d57a1c3193bc4
> +  if True; then
> +func_gl_gnulib_m4code_ef455225c00f5049c808c2eda3e76866
> +  fi
> +  if True; then
> +func_gl_gnulib_m4code_61bcaca76b3e6f9ae55d57a1c3193bc4
> +  fi
>  fi
>}
> 
> This seems to be caused by GLModuleTable.addConditional [1] allowing
> string values or boolean True values. When outputting the
> conditionals in GLEmiter.autoconfSnippets, the condition is printed as
> long as the return value is not None [2]. Therefore, when True is
> returned it is printed. The shell script does not print the
> conditional in these cases.

I see; good point.

> I've opted to use "type(condition) is str" and not "condition != True"
> to avoid any problems comparing a string to a bool. I doubt it would
> cause problems, but the type check in addConditional would ensure it
> isn't anything else weird.

There are no problems with writing "condition != True". Remember the rule?
'condition' as a variable can hold the values None, True, or a string.
Thus it has to be compared with == or !=.

"condition != True" is more future-proof than "type(condition) is str",
because
  - It expresses the intent more precisely. We are making an optimization
that consists in simplifying

  if true; then
STATEMENT
  fi

to

  STATEMENT

  - We may want to allow more complicated conditions in the future, that
may be represented as, say, list of strings instead of a simple string.

I've therefore applied a modified patch:


2024-03-03  Collin Funk  
Bruno Haible  

gnulib-tool.py: Fix output of gnulib-comp.m4.
* pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Fix indentation.
Don't print nonstring values into gnulib-comp.m4.

diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index 2a8ede9335..8939917a0a 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -297,12 +297,12 @@ class GLEmiter(object):
 if moduletable.isConditional(depmodule):
 shellfunc = depmodule.getShellFunc()
 condition = moduletable.getCondition(module, 
depmodule)
-if condition != None:
-emit += '  if %s; then\n' % condition
-emit += '%s\n' % shellfunc
-emit += '  fi\n'
-else:  # if condition == None
-emit += '  %s\n' % shellfunc
+if condition != None and condition != True:
+emit += '  if %s; then\n' % condition
+emit += '%s\n' % shellfunc
+emit += '  fi\n'
+else:  # if condition == None or condition == 
True
+emit += '  %s\n' % shellfunc
 # if not moduletable.isConditional(depmodule)
 else:
 # The autoconf code for $dep has already been 
emitted above and
@@ -329,11 +329,11 @@ class GLEmiter(object):
 if moduletable.isConditional(depmodule):
 shellfunc = depmodule.getShellFunc()
 condition = moduletable.getCondition(module, 
depmodule)
-if condition != None:
+if condition != None and condition != True:
 emit += '  if %s; then\n' % condition
 emit += '%s\n' % shellfunc
 emit += '  fi\n'
-else:  # if condition == None
+else:  # if condition == None or condition == 
True
 emit += '  %s\n' % shellfunc
 # if not moduletable.isConditional(depmodule)
 else:

Re: Python list remove-duplicates

[Bruno Haible]

Collin Funk wrote:
> > The output needs to be deterministic; the natural order from the module
> > description is perfectly fine. Therefore the sorting in the Python code 
> > should
> > go away.
> 
> I've attached a patch

Thanks; applied!

> Since sets are unordered, before I found out duplicates weren't
> removed, I was wondering what the fastest way to remove duplicates
> from a list in Python would be while preserving order. Turns out an
> implementation detail in Python 3.7+ allows for this to be done
> quickly in one line. Might be come in handy later.
> 
> [1] https://stackoverflow.com/a/17016257

Nice trick; thanks for sharing.

Bruno

Re: [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 38.

[Bruno Haible]

Hi Collin,

> This patch pretty much copy and pastes a lot of code used from
> GLEmiter.lib_Makefile_am into GLEmiter.tests_Makefile_am.

Thanks, I am applying it, together with a fix: When doing copy&paste,
one needs to do it _mutatis mutandis_.


2024-03-03  Bruno Haible  

gnulib-tool.py: Fix last commit.
* pygnulib/GLEmiter.py (GLEmiter.tests_Makefile_am): Ignore libname and
libext here.

diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index 7115dfe115..2a8ede9335 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -887,8 +887,8 @@ AC_DEFUN([%V1%_LIBSOURCES], [
 
 def tests_Makefile_am(self, destfile, modules, moduletable,
   makefiletable, witness_macro, for_test):
-'''GLEmiter.tests_Makefile_am(destfile, modules, makefiletable,
- witness_c_macro, for_test) -> tuple of string and bool
+'''GLEmiter.tests_Makefile_am(destfile, modules, moduletable,
+ makefiletable, witness_c_macro, for_test) -> tuple of string and 
bool
 
 Emit the contents of the tests Makefile. Returns str and a bool 
variable
 which shows if subdirectories are used.
@@ -995,14 +995,13 @@ AC_DEFUN([%V1%_LIBSOURCES], [
 amsnippet1 += 'libtests_a_LIBADD += @ALLOCA@\n'
 amsnippet1 += 'libtests_a_DEPENDENCIES += @ALLOCA@\n'
 
-amsnippet1 = 
constants.combine_lines_matching(re.compile('%s_%s_SOURCES' % (libname, 
libext)),
+amsnippet1 = 
constants.combine_lines_matching(re.compile('libtests_a_SOURCES'),
   amsnippet1)
 
 # Get unconditional snippet, edit it and save to amsnippet2.
 amsnippet2 = module.getAutomakeSnippet_Unconditional()
 pattern = re.compile('lib_([A-Z][A-Z]*)', re.M)
-amsnippet2 = pattern.sub('%s_%s_\\1' % (libname, libext),
- amsnippet2)
+amsnippet2 = pattern.sub('libtests_a_\\1', amsnippet2)
 amsnippet2 = amsnippet2.replace('$(GNULIB_',
 '$(' + module_indicator_prefix 
+ '_GNULIB_')
 # Skip the contents if it's entirely empty.

Re: --create-testdir speed

[Bruno Haible]

Collin Funk wrote:
> > --create-megatestdir with all modules is not realistic: it would consume
> > hundreds of GB of disk space and maybe a week of run time.
> > 
> > --create-megatestdir is useful for a small set of modules, in particular
> > for 2 modules, e.g. fnmatch-posix and fnmatch-gnu.
> 
> I see, thanks. I think I tried --create-megatestdir a while ago and
> gave up because it took along time. I assume there were much less
> modules when this was written:
> 
>   
> https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/build-automation.texi#n34

Yes. When this text was written (on 2007-03-12), gnulib had 659 modules.
Now it has 3214 modules.

I'm updating the doc:


2024-03-03  Bruno Haible  

doc: Update regarding --create-megatestdir.
Reported by Alexei Sholomitskiy  in

and by Collin Funk  in
.
* doc/build-automation.texi (Building gnulib): Discourage the use of
--create-megatestdir with all modules.

diff --git a/doc/build-automation.texi b/doc/build-automation.texi
index bff5daf283..250723e717 100644
--- a/doc/build-automation.texi
+++ b/doc/build-automation.texi
@@ -28,10 +28,19 @@
 git checkout, use
 
 @example
-gnulib-tool --create-megatestdir --with-tests --dir=...
+gnulib-tool --create-testdir --with-tests --dir=...
 @end example
 
-Note: The created directory uses ca. 512 MB on disk.
+Alternatively, pick a small set of modules and run
+
+@example
+gnulib-tool --create-megatestdir --with-tests --dir=... autobuild MODULES
+@end example
+
+@noindent
+(You can't do this will all of Gnulib at once: @code{gnulib-tool} would run
+for a week and produce a directory that takes more than 100 GB, maybe even 1 
TB,
+of disk space.)
 
 @item Transfer gnulib directory