conflicting types in windows-spawn

2021-05-06 Thread Markus Mützel
I tried to use the windows-spawn module in a project that defines UNICODE.

Compilation of gnulib failed with the following error:

libtool: compile:  x86_64-w64-mingw32-gcc -DHAVE_CONFIG_H -I. 
-I/home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu -I.. 
-I/home/osboxes/Octave/mxe-octave/usr/x86_64-w64-mingw32/include 
-fvisibility=hidden -g -O2 -MT windows-spawn.lo -MD -MP -MF 
.deps/windows-spawn.Tpo -c 
/home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c
  -DDLL_EXPORT -DPIC -o .libs/windows-spawn.o
/home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c:415:1:
 error: conflicting types for 'compose_handles_block'
  415 | compose_handles_block (const struct inheritable_handles *inh_handles,
  | ^
In file included from 
/home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c:21:
/home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.h:118:12:
 note: previous declaration of 'compose_handles_block' was here
  118 | extern int compose_handles_block (const struct inheritable_handles 
*inh_handles,
  |^
make[5]: *** [Makefile:3299: windows-spawn.lo] Error 1


The following change fixes it for me:

* lib/windows-spawn.h: Use ANSI structure STARTUPINFOA.
---
 lib/windows-spawn.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/windows-spawn.h b/lib/windows-spawn.h
index 1c29d1b17..78f11893c 100644
--- a/lib/windows-spawn.h
+++ b/lib/windows-spawn.h
@@ -25,6 +25,9 @@
 #define WIN32_LEAN_AND_MEAN
 #include 

+/* Don't assume that UNICODE is not defined.  */
+#undef STARTUPINFO
+#define STARTUPINFO STARTUPINFOA

 /* Prepares an argument vector before calling spawn().



Markus




Re: conflicting types in windows-spawn

2021-05-06 Thread Markus Mützel
Am 06. Mai 2021 um 14:28 Uhr schrieb "Markus Mützel":
> I tried to use the windows-spawn module in a project that defines UNICODE.
> 
> Compilation of gnulib failed with the following error:
> 
> libtool: compile:  x86_64-w64-mingw32-gcc -DHAVE_CONFIG_H -I. 
> -I/home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu -I.. 
> -I/home/osboxes/Octave/mxe-octave/usr/x86_64-w64-mingw32/include 
> -fvisibility=hidden -g -O2 -MT windows-spawn.lo -MD -MP -MF 
> .deps/windows-spawn.Tpo -c 
> /home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c
>   -DDLL_EXPORT -DPIC -o .libs/windows-spawn.o
> /home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c:415:1:
>  error: conflicting types for 'compose_handles_block'
>   415 | compose_handles_block (const struct inheritable_handles *inh_handles,
>   | ^
> In file included from 
> /home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c:21:
> /home/osboxes/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.h:118:12:
>  note: previous declaration of 'compose_handles_block' was here
>   118 | extern int compose_handles_block (const struct inheritable_handles 
> *inh_handles,
>   |^
> make[5]: *** [Makefile:3299: windows-spawn.lo] Error 1
> 
> 
> The following change fixes it for me:
> 
> * lib/windows-spawn.h: Use ANSI structure STARTUPINFOA.
> ---
>  lib/windows-spawn.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/windows-spawn.h b/lib/windows-spawn.h
> index 1c29d1b17..78f11893c 100644
> --- a/lib/windows-spawn.h
> +++ b/lib/windows-spawn.h
> @@ -25,6 +25,9 @@
>  #define WIN32_LEAN_AND_MEAN
>  #include 
>  
> +/* Don't assume that UNICODE is not defined.  */
> +#undef STARTUPINFO
> +#define STARTUPINFO STARTUPINFOA
>  
>  /* Prepares an argument vector before calling spawn().
>  

On second thought, the header should probably better not re-define STARTUPINFO.
The following alternative change might be saver:

>From 84df3e82f88799d211bd4f5147473b238937d458 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=BCtzel?= 
Date: Thu, 6 May 2021 15:20:30 +0200
Subject: [PATCH] windows-spawn: Don't assume that UNICODE is not defined

* lib/windows-spawn.h: Use ANSI structure STARTUPINFOA.
---
 lib/windows-spawn.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/windows-spawn.h b/lib/windows-spawn.h
index 1c29d1b17..9bcfb1c82 100644
--- a/lib/windows-spawn.h
+++ b/lib/windows-spawn.h
@@ -116,7 +116,7 @@ extern int init_inheritable_handles (struct 
inheritable_handles *inh_handles,
Returns 0 upon success.  In case of failure, -1 is returned, with errno set.
  */
 extern int compose_handles_block (const struct inheritable_handles 
*inh_handles,
-  STARTUPINFO *sinfo);
+  STARTUPINFOA *sinfo);
 
 /* Frees the memory held by a set of inheritable handles.  */
 extern void free_inheritable_handles (struct inheritable_handles *inh_handles);
-- 
2.31.1.windows.1





Re: conflicting types in windows-spawn

2021-05-14 Thread Markus Mützel
Am 14. Mai 2021 um 12:39 Uhr schrieb "Bruno Haible":
> Yes, I agree. That's the proper way to fix it. I applied and pushed your 
> change.
> Thanks!

Thanks! Very much appreciated.

Markus




Re: bootstrap: improve gnulib git update logic

2020-09-02 Thread Markus Mützel
Am 18.08.2020 um 04:09 schrieb "Kai Torben Ohlhus:
> When GNULIB_SRCDIR is set, this addition should probably work as well.
> Looking at it, maybe it can be fixed by extending your code section [2] by 
> "git fetch":
>
>   if test -d "$GNULIB_SRCDIR"/.git && test -n "$GNULIB_REVISION" \
>  && ! git_modules_config submodule.gnulib.url >/dev/null; then
> (cd "$GNULIB_SRCDIR" && git fetch && git checkout
> "$GNULIB_REVISION") || cleanup_gnulib
>   fi

I didn't receive the original message. Thanks, Kai, for the pointers.

What about only fetching from the remote repository if the selected revision 
doesn't exist locally?

diff --git "a/build-aux/bootstrap" "b/build-aux/bootstrap"
index 8f76d6962..6dfbd907d 100644
--- "a/build-aux/bootstrap"
+++ "b/build-aux/bootstrap"
@@ -704,6 +704,8 @@ if $use_gnulib; then

   if test -d "$GNULIB_SRCDIR"/.git && test -n "$GNULIB_REVISION" \
  && ! git_modules_config submodule.gnulib.url >/dev/null; then
+(cd "$GNULIB_SRCDIR" && ! git cat-file commit "$GNULIB_REVISION" \
+ && git fetch)
 (cd "$GNULIB_SRCDIR" && git checkout "$GNULIB_REVISION") || cleanup_gnulib
   fi


Markus




Modification of environment variables on Windows

2024-04-27 Thread Markus Mützel
Dear gnulib developers,

We recently updated gnulib in GNU Octave to a newer revision 
(d4ec02b3cc70cddaaa5183cc5a45814e0afb2292). (Kudos on the impressive speedup to 
the bootstrap process.)

Since then, we are seeing warnings like the following when building for MinGW:

../../libgnu/tzset.c: In function 'rpl_tzset':
../../libgnu/tzset.c:68:24: warning: initialization of 'char *' from 
incompatible pointer type 'char **' [-Wincompatible-pointer-types]
   68 | for (char *s = env; *s != NULL; s++)
  |^~~
../../libgnu/tzset.c:68:32: warning: comparison between pointer and integer
   68 | for (char *s = env; *s != NULL; s++)
  |^~
../../libgnu/tzset.c:72:28: warning: initialization of 'wchar_t *' {aka 'short 
unsigned int *'} from incompatible pointer type 'wchar_t **' {aka 'short 
unsigned int **'} [-Wincompatible-pointer-types]
   72 | for (wchar_t *ws = wenv; *ws != NULL; ws++)
  |^~~~
../../libgnu/tzset.c:72:38: warning: comparison between pointer and integer
   72 | for (wchar_t *ws = wenv; *ws != NULL; ws++)
 ^~

IIUC, these warnings might be legitimate.

The attached patch avoids those warnings.

I'm hoping this is the correct format to propose changes to gnulib.

Best,
Markus Mützel
From 87b83c757f7d249f983410e85d13ba450d57b417 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=BCtzel?= 
Date: Sat, 27 Apr 2024 13:44:29 +0200
Subject: [PATCH] ctime, localtime, tzset, wcsftime: Check content of
 environment.

* lib/ctime.c (rpl_ctime): Index into content of each environment variable.
* lib/localtime.c (rpl_localtime): Likewise.
* lib/tzset.c (rpl_tzset): Likewise.
* lib/wcsftime.c (rpl_wcsftime): Likewise.
---
 lib/ctime.c | 12 ++--
 lib/localtime.c | 12 ++--
 lib/tzset.c | 12 ++--
 lib/wcsftime.c  | 12 ++--
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/lib/ctime.c b/lib/ctime.c
index 8c54ef463c..e30b7997c8 100644
--- a/lib/ctime.c
+++ b/lib/ctime.c
@@ -63,13 +63,13 @@ rpl_ctime (const time_t *tp)
   char **env = _environ;
   wchar_t **wenv = _wenviron;
   if (env != NULL)
-for (char *s = env; *s != NULL; s++)
-  if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
-s[0] = '$';
+for (char **s = env; *s != NULL; s++)
+  if (*s[0] == 'T' && *s[1] == 'Z' && *s[2] == '=')
+*s[0] = '$';
   if (wenv != NULL)
-for (wchar_t *ws = wenv; *ws != NULL; ws++)
-  if (ws[0] == L'T' && ws[1] == L'Z' && ws[2] == L'=')
-ws[0] = L'$';
+for (wchar_t **ws = wenv; *ws != NULL; ws++)
+  if (*ws[0] == L'T' && *ws[1] == L'Z' && *ws[2] == L'=')
+*ws[0] = L'$';
 }
 #endif

diff --git a/lib/localtime.c b/lib/localtime.c
index f0e91ac647..1c6bb9856e 100644
--- a/lib/localtime.c
+++ b/lib/localtime.c
@@ -63,13 +63,13 @@ rpl_localtime (const time_t *tp)
   char **env = _environ;
   wchar_t **wenv = _wenviron;
   if (env != NULL)
-for (char *s = env; *s != NULL; s++)
-  if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
-s[0] = '$';
+for (char **s = env; *s != NULL; s++)
+  if (*s[0] == 'T' && *s[1] == 'Z' && *s[2] == '=')
+*s[0] = '$';
   if (wenv != NULL)
-for (wchar_t *ws = wenv; *ws != NULL; ws++)
-  if (ws[0] == L'T' && ws[1] == L'Z' && ws[2] == L'=')
-ws[0] = L'$';
+for (wchar_t **ws = wenv; *ws != NULL; ws++)
+  if (*ws[0] == L'T' && *ws[1] == L'Z' && *ws[2] == L'=')
+*ws[0] = L'$';
 }
 #endif

diff --git a/lib/tzset.c b/lib/tzset.c
index f307f0c3d1..c8d48e1afb 100644
--- a/lib/tzset.c
+++ b/lib/tzset.c
@@ -65,13 +65,13 @@ rpl_tzset (void)
   char **env = _environ;
   wchar_t **wenv = _wenviron;
   if (env != NULL)
-for (char *s = env; *s != NULL; s++)
-  if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
-s[0] = '$';
+for (char **s = env; *s != NULL; s++)
+  if (*s[0] == 'T' && *s[1] == 'Z' && *s[2] == '=')
+*s[0] = '$';
   if (wenv != NULL)
-for (wchar_t *ws = wenv; *ws != NULL; ws++)
-  if (ws[0] == L'T' && ws[1] == L&

Re: Modification of environment variables on Windows

2024-04-27 Thread Markus Mützel
Hi Bruno,

> > The attached patch avoids those warnings.
>
> Thanks, but it does not do the right thing: *s[1] accesses the first character
> of the string after s. What was meant was to access the second character of
> the string at s; this needs to be written as (*s)[1].

Oof. You are absolutely right. What a difference some parenthesis make.

> I'm committing this patch instead.

Thank you for spotting the error in the proposed patch and pushing a fix so 
quickly.

Markus



Fetch from existing gnulib Git repository if needed

2024-04-27 Thread Markus Mützel
Dear gnulib developers,

GNU Octave uses Mercurial as the VCS of its main repository.
Developers are using the bootstrap script of gnulib to automatically clone its 
Git repository in a subdirectory of Octave's source tree. The revision that 
we'd like to use is set in the bootstrap.conf script. Currently, that is

: ${GNULIB_REVISION=d4ec02b3cc70cddaaa5183cc5a45814e0afb2292}


This is working perfectly for a fresh clone of Octave's source tree. However, 
when we update GNULIB_REVISION to a newer revision and a user/developer ran the 
bootstrap script before, running the bootstrap script again fails with an error 
like the following:

./bootstrap: Bootstrapping from checked-out octave sources...
fatal: reference is not a tree: d4ec02b3cc70cddaaa5183cc5a45814e0afb2292
program finished with exit code 128


To work around that, a user/developer could manually fetch from the remote 
repository. That is a bit more tedious when it comes to CI installations that 
usually need no manual interaction.

As a workaround we are applying the attached patch to the bootstrap-funclib.sh 
script to automatically fetch from the remote gnulib repository if the 
GNULIB_REVISION isn't found in the local gnulib Git repository.

Would it be possible to make a similar change in gnulib so that updating to a 
newer gnulib revision becomes a bit easier for that configuration?

Markus
Update bootstrap script from upstream gnulib to automatically fetch from 
repository if needed

diff -r c51b07a71421 bootstrap-funclib.sh
--- a/bootstrap-funclib.sh  Fri Apr 26 13:33:37 2024 -0400
+++ b/bootstrap-funclib.sh  Fri Apr 26 20:00:21 2024 +0200
@@ -532,6 +532,10 @@
 # The subdirectory 'gnulib' already exists.
 if test -n "$GNULIB_REVISION"; then
   if test -d "$gnulib_path/.git"; then
+if ! git --git-dir="$gnulib_path"/.git cat-file \
+ commit "$GNULIB_REVISION"; then
+  git --git-dir="$gnulib_path"/.git fetch
+fi
 (cd "$gnulib_path" && git checkout "$GNULIB_REVISION") || exit 1
   else
 die "Error: GNULIB_REVISION is specified in bootstrap.conf," \


Re: Fetch from existing gnulib Git repository if needed

2024-04-28 Thread Markus Mützel
Hi Bruno,

> > As a workaround we are applying the attached patch to the 
> > bootstrap-funclib.sh script to automatically fetch from the remote gnulib 
> > repository if the GNULIB_REVISION isn't found in the local gnulib Git 
> > repository.
>
> Thanks for the patch. But note that GNULIB_REVISION can hold either a commit
> hash or the name of a branch (such as 'stable-202401'). So, we have 4 cases:
>   (a) a commit hash, already present
>   (b) a commit hash, not known
>   (c) a branch, already present
>   (d) a branch, not known
>
> The command 'git cat-file commit $GNULIB_REVISION' returns true in the cases
> (a) and (c). So, your patch would trigger a 'git fetch' in the cases (b) and
> (d). But in case (d), the 'git fetch' is useless:
> 'git cat-file commit $GNULIB_REVISION' would still fail afterwards.
>
> One can distinguish the four cases in more detail using the commands
>   git rev-list --quiet $GNULIB_REVISION --
> which tests for case (a) and
>   git show-ref --verify --quiet refs/heads/$GNULIB_REVISION
> which tests for case (c). This would allow us to do 'git fetch' only in case
> (b).
>
> However, I believe the patch below is simpler and achieves the same goal.

Thank you for looking into this. However, it looks like $GNULIB_SRCDIR is empty 
for us. So, the change doesn't seem to make a difference. Executing bootstrap 
after a revision bump still fails if the bootstrap script was already run 
before.
I like your idea of fetching if the checkout failed though.

The attached diff seems to be working for our use case. Would it be possible to 
apply something like that in gnulib?

Markus
diff -r 0fbf06e4b460 bootstrap-funclib.sh
--- a/bootstrap-funclib.sh  Sat Apr 27 17:33:00 2024 +0200
+++ b/bootstrap-funclib.sh  Sun Apr 28 11:04:52 2024 +0200
@@ -462,7 +462,17 @@
   || die "Error: --gnulib-srcdir or \$GNULIB_SRCDIR is specified," \
  "but does not contain gnulib-tool"
 if test -n "$GNULIB_REVISION" && $use_git; then
-  (cd "$GNULIB_SRCDIR" && git checkout "$GNULIB_REVISION") || exit $?
+  # The 'git checkout "$GNULIB_REVISION"' command succeeds if the
+  # GNULIB_REVISION is a commit hash that exists locally, or if it is
+  # branch name that can be fetched from origin. It fails, however,
+  # if the GNULIB_REVISION is a commit hash that only exists in origin.
+  # In this case, we need a 'git fetch' and then retry
+  # 'git checkout "$GNULIB_REVISION"'.
+  (cd "$GNULIB_SRCDIR" \
+&& { git checkout "$GNULIB_REVISION" 2>/dev/null \
+  || { git fetch origin && git checkout "$GNULIB_REVISION"; }
+}
+  ) || exit $?
 fi
   else
 if ! $use_git; then
@@ -532,7 +542,17 @@
 # The subdirectory 'gnulib' already exists.
 if test -n "$GNULIB_REVISION"; then
   if test -d "$gnulib_path/.git"; then
-(cd "$gnulib_path" && git checkout "$GNULIB_REVISION") || exit 1
+# The 'git checkout "$GNULIB_REVISION"' command succeeds if the
+# GNULIB_REVISION is a commit hash that exists locally, or if it is
+# branch name that can be fetched from origin. It fails, however,
+# if the GNULIB_REVISION is a commit hash that only exists in 
origin.
+# In this case, we need a 'git fetch' and then retry
+# 'git checkout "$GNULIB_REVISION"'.
+(cd "$gnulib_path" \
+  && { git checkout "$GNULIB_REVISION" 2>/dev/null \
+   || { git fetch origin && git checkout "$GNULIB_REVISION"; }
+  }
+) || exit $?
   else
 die "Error: GNULIB_REVISION is specified in bootstrap.conf," \
 "but '$gnulib_path' contains no git history"


Re: Fetch from existing gnulib Git repository if needed

2024-04-28 Thread Markus Mützel
Hi Bruno,

Bruno Haible wrote:
> Markus Mützel wrote:
> > However, it looks like $GNULIB_SRCDIR is empty for us. So, the change 
> > doesn't seem to make a difference. Executing bootstrap after a revision 
> > bump still fails if the bootstrap script was already run before.
> 
> Oh, there were two 'git checkout' commands and I enhanced only one of them...
> 
> Should now be fixed, like this. Thanks for the feedback!

Thank you for the quick fix. It seems to automatically fetch if needed for us 
now.

Markus



Unknown type name 'wint_t' when targeting Cygwin

2024-04-28 Thread Markus Mützel
Dear gnulib maintainers,

We recently updated gnulib to a newer revision in GNU Octave (currently 
92d80242ad1344b5364ca9bd1d995d68c3a73ef7).

Since then we are seeing compilation errors like the following when targeting 
Cygwin:

In file included from /usr/include/sys/reent.h:16,
 from /usr/include/wchar.h:6,
 from ./wchar.h:80,
 from /usr/include/uchar.h:5,
 from ./uchar.h:45,
 from ../../libgnu/btoc32.c:23:
/usr/include/sys/_types.h:167:5: error: unknown type name 'wint_t'
  167 | wint_t __wch;
  | ^~

Is this because something is missing in gnulib? Are we doing something wrong?

I can try to provide logs and more information if needed.

Markus




Re: Unknown type name 'wint_t' when targeting Cygwin

2024-04-29 Thread Markus Mützel
Hi Bruno,

Bruno Haible wrote

> Which Cygwin version, please?

That error occurred, e.g., in a CI run
https://github.com/gnu-octave/octave/actions/runs/8873331621/job/24358996111

The log of that run contains the following line:
Starting cygwin install, version 2.932

Is that the Cygwin version?
All packages should be the latest stable versions that are distributed by the 
Cygwin project.

> Also, what is the gcc command line of this particular compilation unit?
> (`make V=1`)

The CI uses a parallel make. IIUC, the following lines in the log match and 
reproduce one of the errors:

/bin/sh ../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. 
-I../../libgnu -I.. -Wno-cast-qual -Wno-conversion -Wno-float-equal 
-Wno-sign-compare -Wno-undef -Wno-unused-function -Wno-unused-parameter 
-Wno-float-conversion -Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion 
-Wno-type-limits -Wno-unsuffixed-float-constants -g -O2 -MT 
libgnu_la-areadlink.lo -MD -MP -MF .deps/libgnu_la-areadlink.Tpo -c -o 
libgnu_la-areadlink.lo `test -f 'areadlink.c' || echo 
'../../libgnu/'`areadlink.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I../../libgnu -I.. -Wno-cast-qual 
-Wno-conversion -Wno-float-equal -Wno-sign-compare -Wno-undef 
-Wno-unused-function -Wno-unused-parameter -Wno-float-conversion 
-Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion -Wno-type-limits 
-Wno-unsuffixed-float-constants -g -O2 -MT libgnu_la-areadlink.lo -MD -MP -MF 
.deps/libgnu_la-areadlink.Tpo -c ../../libgnu/areadlink.c  -DDLL_EXPORT -DPIC 
-o .libs/libgnu_la-areadlink.o

In file included from /usr/include/sys/reent.h:16,
 from /usr/include/stdlib.h:18,
 from ./stdlib.h:36,
 from ../../libgnu/areadlink.h:21,
 from ../../libgnu/areadlink.c:25:
/usr/include/sys/_types.h:167:5: error: unknown type name 'wint_t'
  167 | wint_t __wch;
  | ^~


Please, let me know if that is the information that you are looking for or if 
you need further details.

Markus





Re: Unknown type name 'wint_t' when targeting Cygwin

2024-05-03 Thread Markus Mützel
Hi Bruno,

> The patch below fixes it for me. It reverts a workaround for gcc 14.0.1
> from a few days ago, for which I'm adding a different workaround later.

Thank you very much. That fixed the build errors also for us. (Tested with 
6213c5bd72d15ca5e1ea9c34122899e02fed448c.)

Markus