Re: Remove MSVC scripts from the tree

Hi,

On 2023-11-15 13:49:06 +0900, Michael Paquier wrote:
> The latest patch I have sent is close to that, actually.  Instead of
> creating a new section, I have integrated the contents of
> install-windows.sgml into the existing section dedicated to MinGW and
> native Windows because some parts apply to both of them, like the
> crash reporting facility.  So this gave the following structure:
> - sect2 MinGW/Native Windows
> -- sect3 Requirements
> -- sect3 MinGW
> -- sect3 Visual Studio
> -- sect3 Special Considerations for 64-Bit Windows
> -- sect3 Collecting Crash Dumps on Windows

It doesn't seem like your patch has it quite that way? I see

  <sect2 id="installation-notes-mingw">
   <title>MinGW</title>
...
  <sect2 id="installation-notes-windows">
   <title>Windows</title>

Where "Windows" actually seems to solely describe visual studio? That seems
confusing.


> diff --git a/src/port/pgstrsignal.c b/src/port/pgstrsignal.c
> index 7d76d1cca9..8c10a760c6 100644
> --- a/src/port/pgstrsignal.c
> +++ b/src/port/pgstrsignal.c
> @@ -6,9 +6,6 @@
>   * On platforms compliant with modern POSIX, this just wraps strsignal(3).
>   * Elsewhere, we do the best we can.
>   *
> - * This file is not currently built in MSVC builds, since it's useless
> - * on non-Unix platforms.
> - *
>   * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
>   * Portions Copyright (c) 1994, Regents of the University of California
>   *

Huh, so this was wrong since the code was added? For a moment I thought I'd
unintentionally promoted it to be built by default, but ...


> index eca930ae47..14c9905b60 100644
> --- a/src/bin/pgevent/meson.build
> +++ b/src/bin/pgevent/meson.build
> @@ -14,7 +14,6 @@ pgevent_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
>
>  pgevent_sources += windows.compile_resources('pgmsgevent.rc')
>
> -# FIXME: copied from Mkvcbuild.pm, but I don't think that's the right approach
>  pgevent_link_args = []
>  if cc.get_id() == 'msvc'
>    pgevent_link_args += '/ignore:4104'

I think it's worth leaving a trail indicating that adding this
warning-suppression is dubious at best.  It seems to pretty obviously paper
over us exporting the symbols the wrong way:
https://learn.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-warning-lnk4104?view=msvc-170

Which pretty clearly explains that pgevent.def is wrong.

I just can't really test it, nor does it have test. Otherwise I might have
fixed it.


> @@ -53,10 +53,25 @@ AC_DEFUN([PGAC_CHECK_PERL_CONFIGS],
>  # would be fatal to try to compile PL/Perl to a different libc ABI than core
>  # Postgres uses.  The available information says that most symbols that affect
>  # Perl's own ABI begin with letters, so it's almost sufficient to adopt -D
> -# switches for symbols not beginning with underscore.  Some exceptions are the
> -# Windows-specific -D_USE_32BIT_TIME_T and -D__MINGW_USE_VC2005_COMPAT; see
> -# Mkvcbuild.pm for details.  We absorb the former when Perl reports it.  Perl
> -# never reports the latter, and we don't attempt to deduce when it's needed.
> +# switches for symbols not beginning with underscore.
> +
> +# Some exceptions are the Windows-specific -D_USE_32BIT_TIME_T and
> +# -D__MINGW_USE_VC2005_COMPAT. To be exact, Windows offers several 32-bit ABIs.
> +# Perl is sensitive to sizeof(time_t), one of the ABI dimensions.  To get
> +# 32-bit time_t, use "cl -D_USE_32BIT_TIME_T" or plain "gcc".  For 64-bit
> +# time_t, use "gcc -D__MINGW_USE_VC2005_COMPAT" or plain "cl".  Before MSVC
> +# 2005, plain "cl" chose 32-bit time_t.  PostgreSQL doesn't support building
> +# with pre-MSVC-2005 compilers, but it does support linking to Perl built with
> +# such a compiler.  MSVC-built Perl 5.13.4 and later report -D_USE_32BIT_TIME_T
> +# in $Config{ccflags} if applicable, but MinGW-built Perl never reports
> +# -D_USE_32BIT_TIME_T despite typically needing it.

Hm, it's pretty odd to have comments about cl.exe here, given that it can't
even be used with msvc.

My impression from testing this is that absorbing the flag from perl suffices
with strawberry perl and mingw perl, both when building with mingw and msvc.


> +# Ignore the $Config{ccflags} opinion about -D_USE_32BIT_TIME_T, and use a
> +# runtime test to deduce the ABI Perl expects.  Specifically, test use of
> +# PL_modglobal, which maps to a PerlInterpreter field whose position depends
> +# on sizeof(time_t).  We absorb the former when Perl reports it.  Perl never
> +# reports the latter, and we don't attempt to deduce when it's needed.

I don't think this is implemented anywhere now?


> +   <para>
> +    PostgreSQL for Windows can be built using meson, as described
> +    in <xref linkend="install-meson"/>.
> +    The native Windows port requires a 32 or 64-bit version of Windows
> +    2000 or later. Earlier operating systems do
> +    not have sufficient infrastructure (but Cygwin may be used on
> +    those).
> +   </para>

Is this actually true? I don't think we build on win2k...


> +   <para>
> +    Native builds of <application>psql</application> don't support command
> +    line editing. The <productname>Cygwin</productname> build does support
> +    command line editing, so it should be used where psql is needed for
> +    interactive use on <productname>Windows</productname>.
> +   </para>

FWIW, the last time I tested it, readline worked.

https://postgr.es/m/20221124023251.k4dnbmxuxmqzq7w3%40awork3.anarazel.de


> +   <para>
> +    PostgreSQL can be built using the Visual C++ compiler suite from Microsoft.
> +    These compilers can be either from <productname>Visual Studio</productname>,
> +    <productname>Visual Studio Express</productname> or some versions of the
> +    <productname>Microsoft Windows SDK</productname>. If you do not already have a
> +    <productname>Visual Studio</productname> environment set up, the easiest
> +    ways are to use the compilers from
> +    <productname>Visual Studio 2022</productname> or those in the
> +    <productname>Windows SDK 10</productname>, which are both free downloads
> +    from Microsoft.
> +   </para>

I think we need a reference to mingw somewhere around here. I don't think
everybody can be expected to just know that they should not have navigated to
"Windows" but "MinGW".



> +     <variablelist>
> +      <varlistentry>
> +       <term><productname>ActiveState Perl</productname></term>
> +       <listitem><para>
> +        ActiveState Perl is required to run the build generation scripts. MinGW
> +        or Cygwin Perl will not work. It must also be present in the PATH.
> +        Binaries can be downloaded from
> +        <ulink url="https://www.activestate.com"></ulink>
> +        (Note: version 5.14 or later is required,
> +        the free Standard Distribution is sufficient).
> +       </para></listitem>
> +      </varlistentry>

Continuing to recommend ActiveState perl seems dubious, but I guess that's
material for another patch.


> +      <varlistentry>
> +       <term><productname>Bison</productname> and
> +        <productname>Flex</productname></term>
> +       <listitem>
> +       <para>
> +        <productname>Bison</productname> and <productname>Flex</productname> are
> +        required to build from Git, but not required when building from a release
> +        file. Only <productname>Bison</productname> versions 2.3 and later
> +        will work. <productname>Flex</productname> must be version 2.5.35 or later.
> +       </para>
> +
> +       <para>
> +        Both <productname>Bison</productname> and <productname>Flex</productname>
> +        are included in the <productname>msys</productname> tool suite, available
> +        from <ulink url="http://www.mingw.org/wiki/MSYS"></ulink> as part of the
> +        <productname>MinGW</productname> compiler suite.
> +       </para>
> +
> +       <para>
> +        You will need to add the directory containing
> +        <filename>flex.exe</filename> and <filename>bison.exe</filename> to the
> +        PATH environment variable. In the case of MinGW, the directory is the
> +        <filename>\msys\1.0\bin</filename> subdirectory of your MinGW
> +        installation directory.
> +       </para>

I found it a lot easier to use https://github.com/lexxmark/winflexbison



> +      <varlistentry>
> +       <term><productname>MIT Kerberos</productname></term>
> +       <listitem><para>
> +        Required for GSSAPI authentication support. MIT Kerberos can be
> +        downloaded from
> +        <ulink url="https://web.mit.edu/Kerberos/dist/index.html"></ulink>.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>libxml2</productname> and
> +        <productname>libxslt</productname></term>
> +       <listitem><para>
> +        Required for XML support. Binaries can be downloaded from
> +        <ulink url="https://zlatkovic.com/pub/libxml"></ulink> or source from
> +        <ulink url="http://xmlsoft.org"></ulink>. Note that libxml2 requires iconv,
> +        which is available from the same download location.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>LZ4</productname></term>
> +       <listitem><para>
> +        Required for supporting <productname>LZ4</productname> compression.
> +        Binaries and source can be downloaded from
> +        <ulink url="https://github.com/lz4/lz4/releases"></ulink>.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>Zstandard</productname></term>
> +       <listitem><para>
> +        Required for supporting <productname>Zstandard</productname> compression.
> +        Binaries and source can be downloaded from
> +        <ulink url="https://github.com/facebook/zstd/releases"></ulink>.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>OpenSSL</productname></term>
> +       <listitem><para>
> +        Required for SSL support. Binaries can be downloaded from
> +        <ulink url="https://slproweb.com/products/Win32OpenSSL.html"></ulink>
> +        or source from <ulink url="https://www.openssl.org"></ulink>.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>ossp-uuid</productname></term>
> +       <listitem><para>
> +        Required for UUID-OSSP support (contrib only). Source can be
> +        downloaded from
> +        <ulink url="http://www.ossp.org/pkg/lib/uuid/"></ulink>.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>Python</productname></term>
> +       <listitem><para>
> +        Required for building <application>PL/Python</application>. Binaries can
> +        be downloaded from <ulink url="https://www.python.org"></ulink>.
> +       </para></listitem>
> +      </varlistentry>
> +
> +      <varlistentry>
> +       <term><productname>zlib</productname></term>
> +       <listitem><para>
> +        Required for compression support in <application>pg_dump</application>
> +        and <application>pg_restore</application>. Binaries can be downloaded
> +        from <ulink url="https://www.zlib.net"></ulink>.
> +       </para></listitem>
> +      </varlistentry>


Except for openssl, where the link is somewhat valuable, the rest don't really
seem to be specific to windows.


> +   <sect3 id="install-windows-full-64-bit">
> +    <title>Special Considerations for 64-Bit Windows</title>
> +    <para>
> +     PostgreSQL will only build for the x64 architecture on 64-bit Windows.
> +    </para>
> +    <para>
> +     Mixing 32- and 64-bit versions in the same build tree is not supported.
> +     The build system will automatically detect if it's running in a 32- or
> +     64-bit environment, and build PostgreSQL accordingly. For this reason, it
> +     is important to start the correct command prompt before building.
> +    </para>

Isn't this directly contradicting the earlier
> +    The native Windows port requires a 32 or 64-bit version of Windows
> +    2000 or later. Earlier operating systems do
?

> +    <para>
> +     To use a server-side third party library such as <productname>Python</productname> or
> +     <productname>OpenSSL</productname>, this library <emphasis>must</emphasis> also be
> +     64-bit. There is no support for loading a 32-bit library in a 64-bit
> +     server. Several of the third party libraries that PostgreSQL supports may
> +     only be available in 32-bit versions, in which case they cannot be used with
> +     64-bit PostgreSQL.
> +    </para>
> +   </sect3>

I.e. cannot be used with postgres at all.


Thank you for working on this!


- Andres