Re: gnulib-tool.py current status
Hi Collin, > By the way, here is a list of packages that I have tested using your > method here: > > https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00018.html Good progress! > These were done sometime in the past ~10 commits, so I would have to > go through and double check no breakage occured since then. This is not needed. For protecting against regressions, we have the test suite. If the test suite shows no failures, it is unlikely that there are regressions in the various packages. > * Packages successfully tested with gnulib-tool.py > > bison > coreutils > cppi > cpio > diffutils > findutils > freedink > Update AC_PREREQ to 2.64 required. > gnutls > grep > groff > gzip > inetutils > libiconv > mailutils > patch > pspp > sed > tar > wget > wget2 Good; I'm testing some more of them. > * Packages with issues with gnulib-tool.py > guile > /home/collin/.local/src/gnulib/gnulib-tool.sh: *** patch file > gnulib-local/m4/clock_time.m4.diff didn't apply cleanly > /home/collin/.local/src/gnulib/gnulib-tool.sh: *** Stop. This only means that this .diff file is out-of-date. I would just remove it and continue: $ git rm gnulib-local/m4/clock_time.m4.diff $ git commit gnulib-local/m4/clock_time.m4.diff $ ./autogen.sh > rcs > /home/collin/.local/src/gnulib/gnulib-tool.sh: *** file > /home/collin/.local/src/gnulib/build-aux/missing not found > /home/collin/.local/src/gnulib/gnulib-tool.sh: *** Stop. This is a problem with the Automake version of invocation; gnulib-tool does not do anything w.r.t. build-aux/missing. You can ignore it. Bruno
gnulib-tool.py in gettext
Hi Collin, Can you please look into this one? $ export GNULIB_TOOL_IMPL=sh+py $ git clone https://git.savannah.gnu.org/git/gettext.git $ sh -x ./autogen.sh ... + .../gnulib-tool --copy-file tests/init.sh gettext-tools .../gnulib-tool: *** gnulib-tool.py produced different files than gnulib-tool.sh! Compare .../gettext and .../glpybdZoLD. .../gnulib-tool: *** Stop. The Python implementation has created a tests/ directory instead of a gettext-tools/tests/ directory. Bruno
gnulib-tool.py in wget
Hi Collin, Another package that shows a difference. Can you please handle it? $ export GNULIB_TOOL_IMPL=sh+py $ git clone https://git.savannah.gnu.org/git/wget.git $ cd wget $ ./bootstrap --no-git --gnulib-srcdir=$GNULIB_SRCDIR ... ./bootstrap: .../gnulib-tool --no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=lib/ --tests-base=lib/tests --local-dir=gl --makefile-name=gnulib.mk --po-base=gnulib_po --po-domain=wget --import ... .../gnulib-tool: *** gnulib-tool.py produced different files than gnulib-tool.sh! Compare .../wget and .../glpyjuwsDS. .../gnulib-tool: *** Stop. ./bootstrap: gnulib-tool failed The reason is a different order of the languages in LINGUAS. gnulib-tool.sh uses LC_ALL=C ls -1 *.po | sed -e 's,\.po$,,' which produces sorted output. Whereas GLImport.py uses for file in os.listdir(joinpath(destdir, pobase)) which lacks the sorting. Bruno
Re: autoreconf --force seemingly does not forcibly update everything
On 4/1/24 11:54 PM, Bruno Haible wrote: > Last month, I spent 2 days on prerelease testing of coreutils. If, after > downloading the carefully prepared tarball from ftp.gnu.org, the first > thing a distro does is to throw away the *.m4 files and regenerate the > configure script with their own one, >* It shows [...] FWIW: especially the downstream builds of the 'coreutils' package have been using 'autoreconf -fi' for a long time, because the upstream tools do not have full I18N support, and the large I18N patch is in use e.g. at Fedora, openSUSE and Debian probably since >15 years. Nowadays, the coreutils-i18n.patch [1][2] gets smaller and smaller because upstream is adding I18N step by step in one tool or another. But we're still not there yet, unfortunately. [1] https://src.fedoraproject.org/rpms/coreutils/blob/main/f/coreutils-i18n.patch [2] https://build.opensuse.org/projects/openSUSE:Factory/packages/coreutils/files/coreutils-i18n.patch?expand=1 Have a nice day, Berny
Re: gnulib-tool.py: Fix some unbound variables.
Hi Bruno, On 4/9/24 12:04 PM, Bruno Haible wrote: > A better fix would be revisit this 'verifier'. Either an enum with 3 possible > values, or (better) a function GLModule -> bool. And call it 'moduleFilter' > rather than 'verifier'. How does this patch look? This method seems much clearer to me than passing 0, 1, or 2 which have no meaning unless you have previously read the docstring. I've type hinted it and double checked that it works with Python 3.7 and Python 3.12 on my machine. The type hinting looks something like this [1]: from collections.abc import Callable def autoconfSnippets(self, ..., module_filter: Callable[[GLModule], bool]) -> str: Meaning that 'module_filter' is a function that accepts a GLModule object and returns a bool. This removes a lot of repetitive lines which is nice. The conditionals can be refactored to remove a level of nesting in a few places. I didn't do that now since it would make the patch harder to follow. I've just used lambda functions since these functions are just wrappers around GLModule methods. I'm not sure if it is better to define them in GLModuleSystem.py or something. Since you are the lisp expert, I will trust your judgment here. Also, now I see an annoying warning: lambda module: True # warning under 'module' because pyright thinks it is unused [2]. I tried omitting the 'module' in hopes that Python would just deal with it. But it gives this: Traceback (most recent call last): [...] File "/home/collin/.local/src/gnulib/pygnulib/GLEmiter.py", line 311, in autoconfSnippets if module_filter(module): ^ TypeError: GLImport.gnulib_comp..() takes 0 positional arguments but 1 was given Maybe I will use fixing that as an excuse to add to the HACKING file. I meant to add information there earlier but got lazy... [1] https://docs.python.org/3/library/typing.html#annotating-callable-objects [2] https://github.com/microsoft/pyright CollinFrom 45a750586980579c44c14e5d95ac4434f2d61313 Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Wed, 10 Apr 2024 05:05:52 -0700 Subject: [PATCH] gnulib-tool.py: Use function arguments instead of magic numbers. * pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Remove the 'verifier' integer flag argument. Add the 'module_filter' function argument. Use it to determine if Autoconf snippets should be printed for each module. * pygnulib/GLImport.py (GLImport.gnulib_comp): Update call to use a lambda function. * pygnulib/GLTestDir.py (GLTestDir.execute): Likewise. --- ChangeLog | 11 pygnulib/GLEmiter.py | 62 +++ pygnulib/GLImport.py | 4 +-- pygnulib/GLTestDir.py | 21 --- 4 files changed, 35 insertions(+), 63 deletions(-) diff --git a/ChangeLog b/ChangeLog index 033eb5dc92..f6aaab6b05 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2024-04-10 Collin Funk + + gnulib-tool.py: Use function arguments instead of magic numbers. + * pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Remove the + 'verifier' integer flag argument. Add the 'module_filter' function + argument. Use it to determine if Autoconf snippets should be printed for + each module. + * pygnulib/GLImport.py (GLImport.gnulib_comp): Update call to use a + lambda function. + * pygnulib/GLTestDir.py (GLTestDir.execute): Likewise. + 2024-04-09 Collin Funk gnulib-tool.py: Change the avoid list to a set for lookups. diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py index e6143645be..247e791e6b 100644 --- a/pygnulib/GLEmiter.py +++ b/pygnulib/GLEmiter.py @@ -22,6 +22,7 @@ import os import re import codecs import subprocess as sp +from collections.abc import Callable from . import constants from .GLInfo import GLInfo from .GLConfig import GLConfig @@ -257,7 +258,7 @@ class GLEmiter: return emit def autoconfSnippets(self, modules: list[GLModule], referenceable_modules: list[GLModule], - moduletable: GLModuleTable, verifier: int, toplevel: bool, + moduletable: GLModuleTable, module_filter: Callable[[GLModule], bool], toplevel: bool, disable_libtool: bool, disable_gettext: bool, replace_auxdir: bool) -> str: '''Collect and emit the autoconf snippets of a set of modules. GLConfig: conddeps. @@ -271,10 +272,8 @@ class GLEmiter: dependencies. moduletable is a GLModuleTable instance, which contains necessary information about dependencies of the modules. -verifier is an integer, which can be 0, 1 or 2. - if verifier == 0, then process every module; - if verifier == 1, then process only non-tests modules; - if verifier == 2, then process only tests modules. +module_filter is a function that accepts a GLModule and returns a bool describing + whether or not Autoconf snippets should be emitted for it. toplevel is a bool variab
Re: gnulib-tool.py in gettext
Hi Bruno, On 4/10/24 4:26 AM, Bruno Haible wrote: > The Python implementation has created a tests/ directory instead of a > gettext-tools/tests/ directory. Ah, I see. Sounds like something simple. I don't think I have even looked at that section of the code... I'll try to get around to fixing that and the wget problem later today. That one also seems unsuprising to me. If I remember correctly the LINGUAS file is the thing that gave me trouble with Bison. While taking a glance at --copy-file I noticed a single character typo copied from gnulib-tool.sh. :) Patch attached. CollinFrom bdfd181f77bcc8f81f34fb81c3c356effc2add7a Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Wed, 10 Apr 2024 05:56:34 -0700 Subject: [PATCH] gnulib-tool: Fix a typo. * gnulib-tool.sh: Fix a typo. * pygnulib/main.py (main): Likewise. --- ChangeLog| 6 ++ gnulib-tool.sh | 2 +- pygnulib/main.py | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f6aaab6b05..8516402a69 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2024-04-10 Collin Funk + + gnulib-tool: Fix a typo. + * gnulib-tool.sh: Fix a typo. + * pygnulib/main.py (main): Likewise. + 2024-04-10 Collin Funk gnulib-tool.py: Use function arguments instead of magic numbers. diff --git a/gnulib-tool.sh b/gnulib-tool.sh index 57620ddf36..a8075d991f 100755 --- a/gnulib-tool.sh +++ b/gnulib-tool.sh @@ -7646,7 +7646,7 @@ s/\([.*$]\)/[\1]/g' # Verify the file exists. func_lookup_file "$f" -# The second argument is the destination; either a directory ot a file. +# The second argument is the destination; either a directory or a file. # It defaults to the current directory. dest="$2" test -n "$dest" || dest='.' diff --git a/pygnulib/main.py b/pygnulib/main.py index 530b22e6f2..f8d082705d 100644 --- a/pygnulib/main.py +++ b/pygnulib/main.py @@ -1288,7 +1288,7 @@ def main() -> None: elif mode == 'copy-file': srcpath = files[0] -# The second argument is the destination; either a directory ot a file. +# The second argument is the destination; either a directory or a file. # It defaults to the current directory. if len(files) == 2: dest = files[1] -- 2.44.0
Re: gnulib-tool.py: Fix some unbound variables.
Hi Collin, > > A better fix would be revisit this 'verifier'. Either an enum with 3 > > possible > > values, or (better) a function GLModule -> bool. And call it 'moduleFilter' > > rather than 'verifier'. > > How does this patch look? Pretty good. The only thing that's missing is the type check for module_filter. Is it module_filter is functionNope! module_filter is CallableNope! module_filter is callableNope! The previous knowledge summarization engine told me how to write it: callable(module_filter) > The conditionals > can be refactored to remove a level of nesting in a few places. I > didn't do that now since it would make the patch harder to follow. It's OK like this. Similar enough to gnulib-tool.sh. > I've just used lambda functions since these functions are just > wrappers around GLModule methods. I'm not sure if it is better to > define them in GLModuleSystem.py or something. Since you are the > lisp expert, I will trust your judgment here. What I considered is write 3 constants: ALL_MODULES_FILTER = lambda module: True NON_TESTS_MODULES_FILTER = lambda module: module.isNonTests() TESTS_MODULES_FILTER = lambda module: module.isTests() but that's probably not worth it either. > Also, now I see an annoying warning: > > lambda module: True # warning under 'module' > > because pyright thinks it is unused [2]. This is a problem with gcc's warnings as well. Such tools don't make the distinction between a function that is only meant to be called directly and a function which needs to adhere to a certain signature. You can open a bug report with that tool (with a trimmed-down test case); then it might get fixed in a year or two. Bruno
Re: autoreconf --force seemingly does not forcibly update everything
Bernhard Voelker wrote: > > Last month, I spent 2 days on prerelease testing of coreutils. If, after > > downloading the carefully prepared tarball from ftp.gnu.org, the first > > thing a distro does is to throw away the *.m4 files and regenerate the > > configure script with their own one, > >* It shows [...] > > FWIW: especially the downstream builds of the 'coreutils' package have been > using 'autoreconf -fi' for a long time, because the upstream tools do not > have full I18N support, and the large I18N patch is in use e.g. at Fedora, > openSUSE and Debian probably since >15 years. Sure, if downstream applies a patch that modifies bootstrap.conf, they need to rerun 'bootstrap'. That goes without saying. I hope downstream then also runs "make check". Bruno
Re: gnulib-tool.py in gettext
> Patch attached. Typo patch applied.
Re: autoreconf --force seemingly does not forcibly update everything
On 4/10/24 4:22 PM, Bruno Haible wrote: Sure, if downstream applies a patch that modifies bootstrap.conf, they need to rerun 'bootstrap'. That goes without saying. I hope downstream then also runs "make check". Sure, full automated QA is in action on all supported platforms. Have a nice day, Berny
Re: gnulib-tool.py: Fix some unbound variables.
On 4/10/24 7:17 AM, Bruno Haible wrote:
> Pretty good. The only thing that's missing is the type check for
> module_filter.
> Is it
> module_filter is functionNope!
> module_filter is CallableNope!
> module_filter is callableNope!
Yes, I tried to add it but was confused since:
>>> print(type(lambda x: True).__name__)
function
But 'is function' does not work.
> The previous knowledge summarization engine told me how to write it:
> callable(module_filter)
Ah, it was correct this time. I guess that makes sense because you can
create classes with the '__call__' method. Then use objects in a
similar way to functions.
> What I considered is write 3 constants:
> ALL_MODULES_FILTER = lambda module: True
> NON_TESTS_MODULES_FILTER = lambda module: module.isNonTests()
> TESTS_MODULES_FILTER = lambda module: module.isTests()
> but that's probably not worth it either.
Yeah, now that I think about it the lambda's are fine. That way we
don't have to deal with importing those across multiple files.
> This is a problem with gcc's warnings as well. Such tools don't make
> the distinction between a function that is only meant to be called
> directly and a function which needs to adhere to a certain signature.
True. Sometimes I forget about those and use -Wall + -Wextra and get
greeted with the unused variable/function spam.
I attached the patch with the callable(module_filter) check.
CollinFrom ff1164711c1397f0f76f6919b395ff132312864f Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Wed, 10 Apr 2024 08:24:58 -0700
Subject: [PATCH] gnulib-tool.py: Use function arguments instead of magic
numbers.
* pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Remove the
'verifier' integer flag argument. Add the 'module_filter' function
argument. Use it to determine if Autoconf snippets should be printed for
each module.
* pygnulib/GLImport.py (GLImport.gnulib_comp): Update call to use a
lambda function.
* pygnulib/GLTestDir.py (GLTestDir.execute): Likewise.
---
ChangeLog | 11
pygnulib/GLEmiter.py | 65 +--
pygnulib/GLImport.py | 4 +--
pygnulib/GLTestDir.py | 21 --
4 files changed, 38 insertions(+), 63 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 7de60dfead..f8fcd38de7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-04-10 Collin Funk
+
+ gnulib-tool.py: Use function arguments instead of magic numbers.
+ * pygnulib/GLEmiter.py (GLEmiter.autoconfSnippets): Remove the
+ 'verifier' integer flag argument. Add the 'module_filter' function
+ argument. Use it to determine if Autoconf snippets should be printed for
+ each module.
+ * pygnulib/GLImport.py (GLImport.gnulib_comp): Update call to use a
+ lambda function.
+ * pygnulib/GLTestDir.py (GLTestDir.execute): Likewise.
+
2024-04-10 Collin Funk
gnulib-tool: Fix a typo.
diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index e6143645be..6d82657e7c 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -22,6 +22,7 @@ import os
import re
import codecs
import subprocess as sp
+from collections.abc import Callable
from . import constants
from .GLInfo import GLInfo
from .GLConfig import GLConfig
@@ -257,7 +258,7 @@ class GLEmiter:
return emit
def autoconfSnippets(self, modules: list[GLModule], referenceable_modules: list[GLModule],
- moduletable: GLModuleTable, verifier: int, toplevel: bool,
+ moduletable: GLModuleTable, module_filter: Callable[[GLModule], bool], toplevel: bool,
disable_libtool: bool, disable_gettext: bool, replace_auxdir: bool) -> str:
'''Collect and emit the autoconf snippets of a set of modules.
GLConfig: conddeps.
@@ -271,10 +272,8 @@ class GLEmiter:
dependencies.
moduletable is a GLModuleTable instance, which contains necessary
information about dependencies of the modules.
-verifier is an integer, which can be 0, 1 or 2.
- if verifier == 0, then process every module;
- if verifier == 1, then process only non-tests modules;
- if verifier == 2, then process only tests modules.
+module_filter is a function that accepts a GLModule and returns a bool describing
+ whether or not Autoconf snippets should be emitted for it.
toplevel is a bool variable, False means a subordinate use of pygnulib.
disable_libtool is a bool variable; it tells whether to disable libtool
handling even if it has been specified through the GLConfig class.
@@ -291,11 +290,9 @@ class GLEmiter:
if type(moduletable) is not GLModuleTable:
raise TypeError('moduletable must be a GLModuleTable, not %s'
% type(moduletable).__name__)
-if type(verifier) is not int:
-raise TypeError('verifie
Re: autoreconf --force seemingly does not forcibly update everything
Bruno Haible writes: > Bernhard Voelker wrote: >> > Last month, I spent 2 days on prerelease testing of coreutils. If, after >> > downloading the carefully prepared tarball from ftp.gnu.org, the first >> > thing a distro does is to throw away the *.m4 files and regenerate the >> > configure script with their own one, >> >* It shows [...] >> >> FWIW: especially the downstream builds of the 'coreutils' package have been >> using 'autoreconf -fi' for a long time, because the upstream tools do not >> have full I18N support, and the large I18N patch is in use e.g. at Fedora, >> openSUSE and Debian probably since >15 years. > > Sure, if downstream applies a patch that modifies bootstrap.conf, they need > to rerun 'bootstrap'. Is bootstrap intended to be reliable from within a tarball? I thought the bootstrap script was not included in tarballs because it wasn't designed to be ran that way, and the way it is designed may not give expected results. Has this changed, so we should recommend maintainers to 'EXTRA_DIST = bootstrap bootstrap-funclib.sh bootstrap.conf' so this is even possible? I recall some project already added something to that effect, but I'm not sure if that is something gnulib supports? /Simon signature.asc Description: PGP signature
Re: gnulib-tool.py: Fix some unbound variables.
Collin Funk wrote: > I attached the patch with the callable(module_filter) check. Thanks! Applied. This eliminates one more item from the TODO list. Bruno
./bootstrap --gnulib-srcdir and GNULIB_REVISION
Hi I'm trying to get ./bootstrap from a minimal source-only archive generated via 'git archive' that has GNULIB_REVISION set in bootstrap.conf, expecting this to work: ./bootstrap --gnulib-srcdir=/home/jas/src/gnulib Bug #1: it seems GNULIB_REVISION in bootstrap.conf has no effect, and this code is the reason (quoting bootstrap-funclib.sh): # XXX Should this be done if $use_git is false? if test -d "$GNULIB_SRCDIR"/.git && test -n "$GNULIB_REVISION" \ && ! git_modules_config submodule.gnulib.url >/dev/null; then (cd "$GNULIB_SRCDIR" && git checkout "$GNULIB_REVISION") || cleanup_gnulib fi The reason is that the tarball has .gitmodules looking like this: [submodule "gnulib"] path = gnulib url = https://git.savannah.gnu.org/git/gnulib.git Which trigger the '! git_modules_config submodules.gnulib.url'. The result is that GNULIB_REVISION is not respected, and I get whatever gnulib code happens to be checked out in --gnulib-srcdir. What's the reason for that check? The logic here isn't that clear. How about simply using: if test -d "$GNULIB_SRCDIR"/.git && test -n "$GNULIB_REVISION"; then (cd "$GNULIB_SRCDIR" && git checkout "$GNULIB_REVISION") || cleanup_gnulib fi At least it seems like a bug that GNULIB_REVISION is not respected, the --help output suggests this should work, which doesn't say anything about got submodules affecting behaviour: * If the environment variable GNULIB_SRCDIR is set (either as an environment variable or via the --gnulib-srcdir option), then sources are fetched from that local directory. If it is a git repository and the configuration variable GNULIB_REVISION is set in bootstrap.conf, then that revision is checked out. I can work around bug#1 with the following: rm .gitmodules ./bootstrap --gnulib-srcdir=/home/jas/src/gnulib That result in the correct gnulib commit being used, and all is fine. Bug #2: ./bootstrap writes to the path indicated by --gnulib-srcdir with the 'git checkout' command, and leaves the --gnulib-srcdir path at that commit after ./bootstrap is finished. This happens to work in my example since I pointed it to a writable work tree, but I think altering that path is unexpected and not documented. Imagine pointing this to a system-wide gnulib .git store like --gnulib-srcdir=/usr/share/src/gnulib or similar read-only place. Or imagine multiple ./bootstrap running at the same time for different projects, both pointing to the same gnulib .git work tree. I think the path indicated by --gnulib-srcdir should be read-only. Should the 'git checkout' code be replaced with something like git clone --reference "$GNULIB_SRCDIR" "$gnulib_path" \ && git checkout -C "$gnulib_path" $GNULIB_REVISION GNULIB_SRCDIR="$gnulib_path" Discussion before suggesting patches would be useful, to establish some agreement on how we want this to behave. /Simon signature.asc Description: PGP signature
Re: autoreconf --force seemingly does not forcibly update everything
On 4/10/24 13:36, Simon Josefsson via Gnulib discussion list wrote: Is bootstrap intended to be reliable from within a tarball? I thought the bootstrap script was not included in tarballs because it wasn't designed to be ran that way, and the way it is designed may not give expected results. It's pretty routinely distributed, I expect under the theory that we're being transparent about what sources we use to generate the tarball. Whether it works from a tarball depends on one's definition of "works". Certainly more expertise and tools are needed to bootstrap than merely to configure + make.
Re: ./bootstrap --gnulib-srcdir and GNULIB_REVISION
My quick reaction is that you've identified two bugs. --gnulib-srcdir should only read from that directory, and GNULIB_REVISION should work in bootstrap.conf.
Re: autoreconf --force seemingly does not forcibly update everything
On Wed, Apr 10, 2024 at 5:44 PM Paul Eggert wrote: > > On 4/10/24 13:36, Simon Josefsson via Gnulib discussion list wrote: > > Is bootstrap intended to be reliable from within a tarball? I thought > > the bootstrap script was not included in tarballs because it wasn't > > designed to be ran that way, and the way it is designed may not give > > expected results. > > It's pretty routinely distributed, I expect under the theory that we're > being transparent about what sources we use to generate the tarball. > > Whether it works from a tarball depends on one's definition of "works". > Certainly more expertise and tools are needed to bootstrap than merely > to configure + make. Maybe the tarball should include a shell script that can achieve a reproducible build, similar to what bootstrap does. Jeff
gnulib-tool.py in barcode, gcal, gengetopt, myserver
A couple of packages (barcode, gcal, gengetopt, myserver) have an old
gnulib configuration that still uses a module 'free', 'malloc', or 'getopt'.
These modules have been renamed in the mean time.
gnulib-tool.sh prints warnings about missing modules or mistyped module names.
gnulib-tool.py currently
- fails,
- prints a Python stack trace, which is not helpful to the user.
For example, in the 'barcode' package:
$ export GNULIB_TOOL_IMPL=sh+py
$ git clone https://git.savannah.gnu.org/git/barcode.git
$ cd barcode
$ ./bootstrap --no-git --gnulib-srcdir=$GNULIB_SRCDIR
...
./bootstrap: .../gnulib-tool --import --no-changelog --aux-dir build-aux
--doc-base doc --lib libgnu --m4-base m4/ --source-base lib/ --tests-base tests
--local-dir gl --libtool --import ...
+ .../gnulib-tool --import --no-changelog --aux-dir build-aux --doc-base doc
--lib libgnu --m4-base m4/ --source-base lib/ --tests-base tests --local-dir gl
--libtool --import calloc-gnu close error float fopen free gettext-h
git-version-gen malloc-gnu memcpy memset open rint search sigpipe snprintf
stdio strdup-posix strerror string time verify write
.../gnulib-tool: *** gnulib-tool.sh succeeded but gnulib-tool.py failed!
Inspect .../glpyB2dMjm/ and .../glpyB2dMjm-py-err.
.../gnulib-tool: *** Stop.
gnulib-tool.sh stderr:
gnulib-tool: warning: module free doesn't exist
gnulib-tool: warning: module free doesn't exist
gnulib-tool.py stderr:
gnulib-tool: warning: file free does not exist
Traceback (most recent call last):
File ".../.gnulib-tool.py", line 30, in
main.main_with_exception_handling()
File ".../pygnulib/main.py", line 1384, in main_with_exception_handling
main()
File ".../pygnulib/main.py", line 956, in main
filetable, transformers = importer.prepare()
File ".../pygnulib/GLImport.py", line 815, in prepare
final_modules = self.moduletable.transitive_closure(base_modules)
File ".../pygnulib/GLModuleSystem.py", line 825, in transitive_closure
raise TypeError('each module must be a GLModule instance')
TypeError: each module must be a GLModule instance
This patch aligns the behaviour of gnulib-tool.py with the one of
gnulib-tool.sh, except that it prints the warning only once instead of twice.
2024-04-10 Bruno Haible
gnulib-tool.py: Skip nonexistent modules instead of failing.
* pygnulib/GLModuleSystem.py (GLModuleSystem.find): Use the same warning
wording as gnulib-tool.sh.
* pygnulib/GLImport.py (GLImport.gnulib_cache): Print the specified
modules, not the base modules.
(GLImport.prepare): Don't put None elements into base_modules.
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 66d8eb922c..fdca884a08 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -523,8 +523,7 @@ class GLImport:
podomain = self.config['podomain']
witness_c_macro = self.config['witness_c_macro']
vc_files = self.config['vc_files']
-modules = [ str(module)
-for module in moduletable.getBaseModules() ]
+modules = self.config['modules']
avoids = self.config['avoids']
emit += self.emitter.copyright_notice()
emit += '''#
@@ -808,8 +807,12 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
m4base = self.config['m4base']
lgpl = self.config['lgpl']
verbose = self.config['verbosity']
-base_modules = sorted({ self.modulesystem.find(m)
-for m in modules })
+base_modules = set()
+for name in modules:
+module = self.modulesystem.find(name)
+if module is not None:
+base_modules.add(module)
+base_modules = sorted(base_modules)
# Perform transitive closure.
final_modules = self.moduletable.transitive_closure(base_modules)
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index ed96b9846c..01378259d9 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -110,7 +110,7 @@ class GLModuleSystem:
raise GLError(3, module)
else: # if not self.config['errors']
sys.stderr.write('gnulib-tool: warning: ')
-sys.stderr.write('file %s does not exist\n' % str(module))
+sys.stderr.write("module %s doesn't exist\n" % str(module))
def file_is_module(self, filename: str) -> bool:
'''Given the name of a file in the modules/ directory, return true
gnulib-tool.sh in time
In the 'time' package, a .gitignore file does not end with a newline. And
while gnulib-tool.py adds a newline before adding more lines, gnulib.tool.sh
does not.
How to reproduce:
$ git clone https://git.savannah.gnu.org/git/time.git
$ cd time
Remove the duplicate /build-aux line from .gitignore.
$ GNULIB_TOOL_IMPL=sh ./bootstrap --no-git --gnulib-srcdir=$GNULIB_SRCDIR
Look at the last lines of .gitignore.
This patch fixes it.
2024-04-10 Bruno Haible
gnulib-tool.sh: Handle .gitignore files that do not end in a newline.
* gnulib-tool.sh (func_import): If the .gitignore file ends with a
character other than a newline, add a newline before adding more lines.
diff --git a/gnulib-tool.sh b/gnulib-tool.sh
index a8075d991f..e99b8fff79 100755
--- a/gnulib-tool.sh
+++ b/gnulib-tool.sh
@@ -6263,6 +6263,9 @@ s,//*$,/,'
if test -n "$anchor"; then sed -e 's,/,\\/,g' -e
"s,^,/^${doubly_escaped_anchor}," -e 's,$,$/d,' < "$tmp"/ignore-removed; fi
} > "$tmp"/sed-ignore-removed
{ cat "$destdir/$dir$ignore"~
+ # Add a newline if the original $dir$ignore file ended
+ # with a character other than a newline.
+ if test `tail -c 1 < "$destdir/$dir$ignore"~ | tr -d '\n' |
wc -c` = 1; then echo; fi
sed -e "s|^|$anchor|" < "$tmp"/ignore-added
} | sed -f "$tmp"/sed-ignore-removed \
> "$destdir/$dir$ignore"
Re: autoreconf --force seemingly does not forcibly update everything
On 2024-04-10 16:36, Simon Josefsson wrote: > Is bootstrap intended to be reliable from within a tarball? I thought > the bootstrap script was not included in tarballs because it wasn't > designed to be ran that way, and the way it is designed may not give > expected results. Has this changed, so we should recommend maintainers > to 'EXTRA_DIST = bootstrap bootstrap-funclib.sh bootstrap.conf' so this > is even possible? Not including the scripts used to build configure in a source tarball is a mistake, particularly for a GPL-licensed package. The configure script itself is clearly object code, and the GPL defines corresponding source to include any "scripts to control [its generation]". If you cannot successfully regenerate configure from a source tarball, it is probably missing some of the source code for the configure script, which is a problem if it was supposed to be a free software package. Cheers, Nick
Re: ./bootstrap --gnulib-srcdir and GNULIB_REVISION
Hi Simon, > Bug #2: ./bootstrap writes to the path indicated by --gnulib-srcdir with > the 'git checkout' command, and leaves the --gnulib-srcdir path at that > commit after ./bootstrap is finished. This happens to work in my > example since I pointed it to a writable work tree, but I think altering > that path is unexpected and not documented. Imagine pointing this to a > system-wide gnulib .git store like --gnulib-srcdir=/usr/share/src/gnulib > or similar read-only place. Or imagine multiple ./bootstrap running at > the same time for different projects, both pointing to the same gnulib > .git work tree. I think the path indicated by --gnulib-srcdir should be > read-only. > > Should the 'git checkout' code be replaced with something like > > git clone --reference "$GNULIB_SRCDIR" "$gnulib_path" \ > && git checkout -C "$gnulib_path" $GNULIB_REVISION > GNULIB_SRCDIR="$gnulib_path" > > Discussion before suggesting patches would be useful, to establish some > agreement on how we want this to behave. You're right, --gnulib-srcdir and the $GNULIB_SRCDIR variable denote "the local directory where gnulib sources reside. Use this if you already have gnulib sources on your machine, and you want to use these sources." (I introduced the distinction between GNULIB_SRCDIR and GNULIB_REFDIR in commit 2122284380cc0d1b3b6f11d92c04652616da79c7.) Thus the behaviour you observed is a bug. Even worse, 'bootstrap' does it even when the option --no-git is given! How to reproduce: $ git clone git://git.savannah.gnu.org/make.git $ cd make $ ./bootstrap --no-git --gnulib-srcdir=$GNULIB_SRCDIR I think the use of --gnulib-srcdir when GNULIB_REVISION is specified in bootstrap.conf is a classical example of conflicting requests. Which one should take precedence? - IMO --gnulib-srcdir is documented in such a way that it takes precedence. - But one may also argue that it should produce an error, to make the user aware of the conflict. Something like "The option --gnulib-srcdir cannot be honored together because the package specifies a GNULIB_REVISION." The user should be able to resolve the conflict either way, by choosing different command-line options. Bruno
gnulib-tool.py in jugtail
Another package that exhibits a difference: $ export GNULIB_TOOL_IMPL=sh+py $ cvs -z3 -d:pserver:anonym...@cvs.savannah.nongnu.org:/sources/jugtail co jugtail $ cd jugtail $ $GNULIB_SRCDIR/gnulib-tool --update .../gnulib-tool: *** gnulib-tool.sh failed but gnulib-tool.py succeeded! Inspect .../glpyFfCgil-sh-err and .../glpyFfCgil-py-err. .../gnulib-tool: *** Stop. $ cat ../glpyFfCgil-sh-err .../gnulib-tool.sh: *** missing --doc-base option. --doc-base has been introduced on 2006-07-11; if your last invocation of 'gnulib-tool --import' is before that date, you need to run 'gnulib-tool --import' once, with a --doc-base option. .../gnulib-tool.sh: *** Stop. $ cat ../glpyFfCgil-py-err gnulib-tool: warning: module malloc doesn't exist Bruno
Re: autoreconf --force seemingly does not forcibly update everything
Nick Bowler wrote: > Not including the scripts used to build configure in a source tarball > is a mistake, particularly for a GPL-licensed package. The configure > script itself is clearly object code, and the GPL defines corresponding > source to include any "scripts to control [its generation]". But the GPL does not require that the corresponding sources be included in the *same tarball*. When a distro distributes binary packages, they don't include the source code in the same binary .rpm or binary .tar.xz. All they do is to provide the source code in a location that is straightforward to find. The same rights hold for upstream package maintainers: When they produce tarballs, that include a "binary" configure and generated .c files, it is sufficient that the source code (*.m4 files and .y and .l files) are in the git repository or submodules, and that the git repository is straight- forward to find. Bruno
Re: ./bootstrap --gnulib-srcdir and GNULIB_REVISION
Bruno Haible writes: > Hi Simon, > >> Bug #2: ./bootstrap writes to the path indicated by --gnulib-srcdir with >> the 'git checkout' command, and leaves the --gnulib-srcdir path at that >> commit after ./bootstrap is finished. This happens to work in my >> example since I pointed it to a writable work tree, but I think altering >> that path is unexpected and not documented. Imagine pointing this to a >> system-wide gnulib .git store like --gnulib-srcdir=/usr/share/src/gnulib >> or similar read-only place. Or imagine multiple ./bootstrap running at >> the same time for different projects, both pointing to the same gnulib >> .git work tree. I think the path indicated by --gnulib-srcdir should be >> read-only. >> >> Should the 'git checkout' code be replaced with something like >> >> git clone --reference "$GNULIB_SRCDIR" "$gnulib_path" \ >> && git checkout -C "$gnulib_path" $GNULIB_REVISION >> GNULIB_SRCDIR="$gnulib_path" >> >> Discussion before suggesting patches would be useful, to establish some >> agreement on how we want this to behave. > > You're right, --gnulib-srcdir and the $GNULIB_SRCDIR variable denote > "the local directory where gnulib >sources reside. Use this if you already >have gnulib sources on your machine, and >you want to use these sources." > (I introduced the distinction between GNULIB_SRCDIR and GNULIB_REFDIR > in commit 2122284380cc0d1b3b6f11d92c04652616da79c7.) > > Thus the behaviour you observed is a bug. Even worse, 'bootstrap' does > it even when the option --no-git is given! > > How to reproduce: > $ git clone git://git.savannah.gnu.org/make.git > $ cd make > $ ./bootstrap --no-git --gnulib-srcdir=$GNULIB_SRCDIR > > I think the use of --gnulib-srcdir when GNULIB_REVISION is specified > in bootstrap.conf is a classical example of conflicting requests. > Which one should take precedence? > - IMO --gnulib-srcdir is documented in such a way that it takes > precedence. > - But one may also argue that it should produce an error, to make > the user aware of the conflict. Something like > "The option --gnulib-srcdir cannot be honored together because the > package specifies a GNULIB_REVISION." > The user should be able to resolve the conflict either way, > by choosing different command-line options. My reaction was initially exactly the same as yours, until I found this piece of --help documentation, which actually is the first (and presumably highest priority) rule: * If the environment variable GNULIB_SRCDIR is set (either as an environment variable or via the --gnulib-srcdir option), then sources are fetched from that local directory. If it is a git repository and the configuration variable GNULIB_REVISION is set in bootstrap.conf, then that revision is checked out. So I think this combination is intended to be supported, it is just not working when a .gitmodules file is present in $CWD -- something that is not mentioned as a requirement. I would certainly agree that trying to understand the interaction between: 1) --gnulib-srcdir 2) --gnulib-refdir 3) GNULIB_REVISION 4) --no-git 4) building from a tarball with .gitmodules file 5) building from a tarball without .gitmodules file 6) building from a git clone with a .git sub-directory 7) building from a git clone with an indirect .git file 8) building with GNULIB_REVISION provided as an environment variable outside of bootstrap.conf and maybe other factors is really complicated, and I have had to read both --help and source code to feel close to understanding things. Alas, GNULIB_REVISION is not documented in doc/gnulib.texi. My impression is that the ./bootstrap script has gained a lot of complexity that has evolved organically. For some projects this complexity is unwanted -- e.g., guile-gnutls is used early in the bootstrapping of Guix and we eventually resolved to putting all needed gnulib files in .git and used a naive ./bootstrap script: https://gitlab.com/gnutls/guile/-/blob/master/bootstrap?ref_type=heads /Simon signature.asc Description: PGP signature
Re: autoreconf --force seemingly does not forcibly update everything
Paul Eggert writes: > On 4/10/24 13:36, Simon Josefsson via Gnulib discussion list wrote: >> Is bootstrap intended to be reliable from within a tarball? I thought >> the bootstrap script was not included in tarballs because it wasn't >> designed to be ran that way, and the way it is designed may not give >> expected results. > > It's pretty routinely distributed, I expect under the theory that > we're being transparent about what sources we use to generate the > tarball. > > Whether it works from a tarball depends on one's definition of > "works". Certainly more expertise and tools are needed to bootstrap > than merely to configure + make. The definition for "works" seems fairly permissive: running ./bootstrap from, e.g., the coreutils 9.5 tarball dies instantly due to this: if test -n "$checkout_only_file" && test ! -r "$checkout_only_file"; then die "Running this script from a non-checked-out distribution is risky." fi I see that some projects (including coreutils) add bootstrap to EXTRA_DIST, but I can't find any recommendation in the gnulib manual to do that so I had assumed it is not something we recommend generally. I haven't added it to inetutils, libidn2, gsasl, etc. /Simon signature.asc Description: PGP signature
Re: ./bootstrap --gnulib-srcdir and GNULIB_REVISION
Simon Josefsson via Gnulib discussion list writes: > My reaction was initially exactly the same as yours, until I found this > piece of --help documentation, which actually is the first (and > presumably highest priority) rule: > > * If the environment variable GNULIB_SRCDIR is set (either as an >environment variable or via the --gnulib-srcdir option), then sources >are fetched from that local directory. If it is a git repository and >the configuration variable GNULIB_REVISION is set in bootstrap.conf, >then that revision is checked out. > > So I think this combination is intended to be supported, it is just not > working when a .gitmodules file is present in $CWD -- something that is > not mentioned as a requirement. Sorry, I found this piece of --help contradict the last sentence I wrote: If you maintain a package and want to pin a particular revision of the Gnulib sources that has been tested with your package, then there are two possible approaches: either configure a 'gnulib' submodule with the appropriate revision, or set GNULIB_REVISION (and if necessary GNULIB_URL) in bootstrap.conf. So it seems having a git submodule for gnulib and using GNULIB_REVISION at the same time is not supported. FWIW, I'm going to experiment in libntlm and use GNULIB_REVISION in bootstrap.conf instead of a git submodule, to allow a downloaded tarball of git HEAD to be a supported way of building the package from source. This hasn't worked historically for reasons discussed in this thread, but given the xz backdoor I think there is value in supporting this approach. /Simon signature.asc Description: PGP signature
