On Fri, Nov 17, 2023 at 6:31 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Wed, Nov 15, 2023 at 05:07:03PM -0800, Andres Freund wrote: > > On 2023-11-15 13:49:06 +0900, Michael Paquier wrote: > > Where "Windows" actually seems to solely describe visual studio? That seems > > confusing. > > Yeah, switch that to Visual. > > > 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 ... > > Yes, I was wondering if there could be an argument for simplifying > some code here by pushing more logic into this wrapper, but I'm > finding that a bit unappealing, and building it under Visual has no > actual consequence: it seems that we never call pg_strsignal() under > WIN32. > > >> -# 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. > > Agreed that there is a good argument for removing it at some point, > with a separate investigation. I've just added a XXX comment for now. > > >> @@ -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. > > I was a bit uncomfortable with removing these references, but I > suspect that you are right and that they're outdated artifacts of the > past. So I'm OK to remove the cl and gcc parts as the flags come from > $PERL. > > >> +# 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? > > Indeed, that's now gone. > > >> + <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... > > Nah, this is a reference outdated for ages. 495ed0ef2d72 has even > bumped _WIN32_WINNT to require Windows 10 as the minimal runtime > version supported, so this needs to be updated and backpatched. The > first two sentences can be simplified like that: > - 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). > + The native Windows port requires a 32 or 64-bit version of Windows > + 10 or later. Earlier operating systems do not have sufficient > + infrastructure. > > Even the second sentence could be entirely removed, I don't see much > advantage in keeping it. Would you be OK with that, as a separate > patch? I've updated the refernce in the attached. > > >> + <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 > > Okay. I couldn't really make it work, FWIW. Perhaps this is just > something that could be tweaked in a different patch. What you are > mentioning requires quite a few steps, and I am not sure if this is > the safest and/or the easiest way to achieve that, TBH. I'd keep that > as a separate investigation for now. > > >> + <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". > > Hmm. But if this is a section only for Visual, it doesn't make sense > to me to mention MinGW here? I am not sure to follow how this is in > line with the previous comments. > > > Continuing to recommend ActiveState perl seems dubious, but I guess that's > > material for another patch. > > I want to see this reference entirely gone at the end with more > stuff trimmed. For now I'm focusing on a simpler restructure. > > >> + <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 > > And I've been using chocolatey to fetch some dependencies. I think > that trimming this stuff should be discussed in a separate patch. > > > Except for openssl, where the link is somewhat valuable, the rest don't > > really > > seem to be specific to windows. > > Yeah, these are historic. Still they can be useful for the Visual > builds in some cases, I guess? I am not sure if it's worth pushing > these dependencies to the main meson page, somewhat polluting it for > references that most people don't really care about. Anyway, I'm > tempted to be less ambitious in a first step and just move that in the > compatibility section. > > >> + <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 > > ? > > How it that? Mixing 32b and 64b libraries is not related to the > minimal runtime version. This is just telling to not mix both. > -- > Patch is not applying. Please share the Rebased Version. Please find the error:
D:\Project\Postgres>git am D:\Project\Patch\v5-0001-Remove-MSVC-scripts.patch error: patch failed: doc/src/sgml/filelist.sgml:38 error: doc/src/sgml/filelist.sgml: patch does not apply error: patch failed: src/tools/msvc/Mkvcbuild.pm:1 error: src/tools/msvc/Mkvcbuild.pm: patch does not apply error: patch failed: src/tools/msvc/Solution.pm:1 error: src/tools/msvc/Solution.pm: patch does not apply hint: Use 'git am --show-current-patch=diff' to see the failed patch Applying: Remove MSVC scripts Patch failed at 0001 Remove MSVC scripts When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Thanks and Regards, Shubham Khanna.
