Problem building today's snapshot with MinGW

Problem building today's snapshot with MinGW

[Eli Zaretskii]

At Joel's request, I've built today's snapshot of the GDB master
branch using mingw.org's MinGW, and found a problem when running the
produced gdb.exe on versions of Windows older than 8.0 (in my case it
was XP).  The details are described in my report to the Gnulib folks:

  https://lists.gnu.org/archive/html/bug-gnulib/2020-09/index.html

The provisional patch I used to solve this problem is this:

--- gnulib/import/stat-w32.c~	2020-09-10 04:47:11.000000000 +0300
+++ gnulib/import/stat-w32.c	2020-09-10 16:05:46.789125000 +0300
@@ -20,10 +20,14 @@
 
 #if defined _WIN32 && ! defined __CYGWIN__
 
+#if _GL_WINDOWS_STAT_INODES == 2
 /* Ensure that <windows.h> defines FILE_ID_INFO.  */
-#if !defined _WIN32_WINNT || (_WIN32_WINNT < _WIN32_WINNT_WIN8)
-# undef _WIN32_WINNT
-# define _WIN32_WINNT _WIN32_WINNT_WIN8
+# if !defined _WIN32_WINNT || (_WIN32_WINNT < _WIN32_WINNT_WIN8)
+#  undef _WIN32_WINNT
+#  define _WIN32_WINNT _WIN32_WINNT_WIN8
+# endif
+#elif !defined VOLUME_NAME_NONE
+# define VOLUME_NAME_NONE 0x4
 #endif
 
 #include <sys/types.h>

Re: Problem building today's snapshot with MinGW

[Joel Brobecker]

> At Joel's request, I've built today's snapshot of the GDB master
> branch using mingw.org's MinGW, and found a problem when running the
> produced gdb.exe on versions of Windows older than 8.0 (in my case it
> was XP).  The details are described in my report to the Gnulib folks:
> 
>   https://lists.gnu.org/archive/html/bug-gnulib/2020-09/index.html

FTR, the direct link to the actual email is:
https://lists.gnu.org/archive/html/bug-gnulib/2020-09/msg00027.html

> The provisional patch I used to solve this problem is this:

I created the following gdb/build PR:

    https://sourceware.org/bugzilla/show_bug.cgi?id=26607

> --- gnulib/import/stat-w32.c~	2020-09-10 04:47:11.000000000 +0300
> +++ gnulib/import/stat-w32.c	2020-09-10 16:05:46.789125000 +0300
> @@ -20,10 +20,14 @@
>  
>  #if defined _WIN32 && ! defined __CYGWIN__
>  
> +#if _GL_WINDOWS_STAT_INODES == 2
>  /* Ensure that <windows.h> defines FILE_ID_INFO.  */
> -#if !defined _WIN32_WINNT || (_WIN32_WINNT < _WIN32_WINNT_WIN8)
> -# undef _WIN32_WINNT
> -# define _WIN32_WINNT _WIN32_WINNT_WIN8
> +# if !defined _WIN32_WINNT || (_WIN32_WINNT < _WIN32_WINNT_WIN8)
> +#  undef _WIN32_WINNT
> +#  define _WIN32_WINNT _WIN32_WINNT_WIN8
> +# endif
> +#elif !defined VOLUME_NAME_NONE
> +# define VOLUME_NAME_NONE 0x4
>  #endif

From what I understood of the analysis, gnulib is doing something
pretty questionable (forcing _WIN32_WINNT to the wrong value) in
order to get FILE_ID_INFO being defined; and looking at your patch
and the explanation you gave in the report to gnulib, you found
that the APIs in question are only needed if _GL_WINDOWS_STAT_INODES
and so fixed this by only enabling the redefined based on
this variable. So for me, this is more of a workaround than an actual
fix. Between this and the fact that all versions of Windows older
than 8.1 are no longer supported by Microsoft, I can anticipate
some push back from the gnulib community.

The question we then want to ask ourselves, is whether we want to
continue supporting older versions of Windows. I'm personally not
opposed, but it has to be at a reasonable cost, meaning that it can
only be acceptable if changes made to support those versions don't
increase the maintenance burden. Unfortunately, I don't think
the patch above meets those requirements.

I'm not opposed to having some kind of fix to be discussed in
the upcoming gdb-10-branch, though, because there we know the extra
burden would only last as long as the branch itself.

Thoughts?

-- 
Joel

Re: Problem building today's snapshot with MinGW

[Eli Zaretskii]

> Date: Sat, 12 Sep 2020 09:33:46 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> From what I understood of the analysis, gnulib is doing something
> pretty questionable (forcing _WIN32_WINNT to the wrong value) in
> order to get FILE_ID_INFO being defined; and looking at your patch
> and the explanation you gave in the report to gnulib, you found
> that the APIs in question are only needed if _GL_WINDOWS_STAT_INODES
> and so fixed this by only enabling the redefined based on
> this variable. So for me, this is more of a workaround than an actual
> fix.

It's indeed a workaround, because I actually believe the code is
wrong, and the entire part that redefines _WIN32_WINNT should be
dropped, as the rest of the code is well equipped to handle any
reasonable value of _WIN32_WINNT.

I used the workaround because I'm still not 100% sure I'm not missing
something, which could explain why the Gnulib folks have this code
there.  It is also the reason why I didn't post the patch to the
Gnulib list, but instead asked them why that part of the code was at
all necessary.

Re: Problem building today's snapshot with MinGW

[Eli Zaretskii]

> Date: Thu, 10 Sep 2020 18:35:34 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> At Joel's request, I've built today's snapshot of the GDB master
> branch using mingw.org's MinGW, and found a problem when running the
> produced gdb.exe on versions of Windows older than 8.0 (in my case it
> was XP).  The details are described in my report to the Gnulib folks:
> 
>   https://lists.gnu.org/archive/html/bug-gnulib/2020-09/index.html

The Gnulib folks confirmed my report and provided a patch, see

  https://lists.gnu.org/archive/html/bug-gnulib/2020-09/msg00075.html

How should we proceed?  Do we import the updated Gnulib, or do we
patch our copy?

Re: Problem building today's snapshot with MinGW

[Joel Brobecker]

> > At Joel's request, I've built today's snapshot of the GDB master
> > branch using mingw.org's MinGW, and found a problem when running the
> > produced gdb.exe on versions of Windows older than 8.0 (in my case it
> > was XP).  The details are described in my report to the Gnulib folks:
> > 
> >   https://lists.gnu.org/archive/html/bug-gnulib/2020-09/index.html
> 
> The Gnulib folks confirmed my report and provided a patch, see
> 
>   https://lists.gnu.org/archive/html/bug-gnulib/2020-09/msg00075.html
> 
> How should we proceed?  Do we import the updated Gnulib, or do we
> patch our copy?

I think we should take different approaches for master and branch.

  - On master, we should attempt a gnulib update, so as to keep
    out list of patches down to the absolute bare minimum.

    Alternatively, if there is an unforseen complication with doing
    that, we could include those two changes as local patches
    specifically, but this would only create more work for the next
    person who wants to do an update. That's why I think we should
    first try the regular update.

  - On the branch, I think we should go with Bruno's patches,
    after review and testing. gnulib updates are always tricky,
    so outside of some particular circumstances, I think they are
    in general too risky to do on a release branch.

-- 
Joel