Re: Remove MSVC scripts from the tree

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Remove MSVC scripts from the tree
Date: 2023-10-05 07:38:51
Message-ID: d8c7c426-ca50-86be-ce74-d0a8110c40fe@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 02.10.23 09:38, Michael Paquier wrote:
> Attached is a v2 with these adjustments, for now.

General comments:

- I think we can't just delete install-windows.sgml. Some of that
content needs to be moved over to installation.sgml. As a simple
example, install-windows.sgml shows which MSVC versions are supported.
That information should surely be kept.

- Is src/backend/utils/README.Gen_dummy_probes still correct after this?
AFAICT, the Perl-based MSVC build system uses Gen_dummy_probes.pl, but
the meson build uses Gen_dummy_probes.sed even on Windows. Is that
correct, intended?

- src/port/pgstrsignal.c contains a comment that it is not "built in
MSVC builds", but AFAICT, that is only correct for the legacy Perl-based
build system, not for meson. Again, is that correct, intended?

Detail comments:

(Btw., whatever orderfile you used for the diff, I find that confusing.)

* config/perl.m4: This now contains all the required information, but
maybe break the text into paragraphs a bit?

* doc/src/sgml/installation.sgml:

I think this paragraph should just be removed altogether:

<para>
If you are building <productname>PostgreSQL</productname> for Microsoft
- Windows, read this chapter if you intend to build with MinGW or Cygwin;
- but if you intend to build with Microsoft's <productname>Visual
- C++</productname>, see <xref linkend="install-windows"/> instead.
+ Windows, read this chapter if you intend to build with Meson, MinGW or
+ Cygwin.
</para>

Here

<para>
PostgreSQL can be built using Cygwin, a Linux-like environment for
Windows, but that method is inferior to the native Windows build
- <phrase condition="standalone-ignore">(see <xref
linkend="install-windows"/>)</phrase> and
- running a server under Cygwin is no longer recommended.
+ with Meson, and running a server under Cygwin is no longer recommended.
</para>

I think "with Meson" should be removed. The tradeoff is Cygwin vs.
native, it doesn't have anything to do with Meson.

Also, I think this paragraph needs a complete revision, along with
however install-windows.sgml gets integrated:

<para>
- PostgreSQL for Windows can be built using MinGW, a Unix-like build
[...]

* meson.build: I think these comments are unnecessary and can be removed:

-# From Project.pm
+# MSVC flags

+ # Preprocessor definitions.

* src/bin/pgevent/meson.build: After consideration, I think this
comment should just be removed:

-# FIXME: copied from Mkvcbuild.pm, but I don't think that's the right
approach
+# FIXME: this may not not the right approach..

The original site in Mkvcbuild.pm does not contain a comment, so we
should accept that as canonical. It doesn't help much if we carry
around a comment like "this might be wrong" indefinitely without any
further supporting material.

* src/common/Makefile and src/common/meson.build: This change is losing
the period at the end of the first sentence:

# A few files are currently only built for frontend, not server
-# (Mkvcbuild.pm has a copy of this list, too). logging.c is excluded
-# from OBJS_FRONTEND_SHLIB (shared library) as a matter of policy,
-# because it is not appropriate for general purpose libraries such
-# as libpq to report errors directly.
+# logging.c is excluded from OBJS_FRONTEND_SHLIB (shared library) as
+# a matter of policy, because it is not appropriate for general purpose
+# libraries such as libpq to report errors directly.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2023-10-05 08:04:36 Re: make add_paths_to_append_rel aware of startup cost
Previous Message Jim Jones 2023-10-05 07:38:20 Re: [PATCH] Add CANONICAL option to xmlserialize