Обсуждение: When to drop src/tools/msvc support

Hi,

I'd planned to write this soon anyway, but it was just brought up in [1].

Originally we had planned to drop src/tools/msvc support shortly after meson
went in. Unfortunately, it took a bit longer than originally hoped for to
merge meson support and then longer than hoped to add buildfarm support. I
don't think there's been any buildfarm client release with meson support yet -
but we do have windows buildfarm animals using it.

Do we want to drop src/tools/msvc support in 16 (i.e. now), or do it early in
17?

I do have a set of patches removing src/tools/msvc. There are a few loose ends
that I know of (my eyes glaze over every time I try to reconcile the
src/tools/perl.m4 comments with the referenced comments in Mkvcbuild.pm), and
probably more small references that grep terms didn't find.

Greetings,

Andres Freund

[1] https://postgr.es/m/3598664.1680976414%40sss.pgh.pa.us

On 4/8/23 3:10 PM, Andres Freund wrote:
> Hi,
> 
> I'd planned to write this soon anyway, but it was just brought up in [1].
> 
> Originally we had planned to drop src/tools/msvc support shortly after meson
> went in. Unfortunately, it took a bit longer than originally hoped for to
> merge meson support and then longer than hoped to add buildfarm support. I
> don't think there's been any buildfarm client release with meson support yet -
> but we do have windows buildfarm animals using it.
> 
> Do we want to drop src/tools/msvc support in 16 (i.e. now), or do it early in
> 17?
> 
> I do have a set of patches removing src/tools/msvc. There are a few loose ends
> that I know of (my eyes glaze over every time I try to reconcile the
> src/tools/perl.m4 comments with the referenced comments in Mkvcbuild.pm), and
> probably more small references that grep terms didn't find.

(reads [1])

Can we treat this as an "open item" for completing the transition to 
meson for builds as part of v16?

With my personal hat on, it seems silly to wait until v17 to do this, 
and I don't see why we would want to wait. If there's limited risk in 
doing this and it'll make our builds both more stable + faster, it seems 
like we should just do it.

Thanks,

Jonathan

Andres Freund <andres@anarazel.de> writes:
> Do we want to drop src/tools/msvc support in 16 (i.e. now), or do it early in
> 17?

On the one hand, it feels like something we shouldn't do after
feature freeze.  On the other hand, continuing to maintain three
build systems is a real drag (although you could argue that there
shouldn't be much churn there until the tree opens for 17).

We clearly can't consider it in any case until the buildfarm
is prepared, with all the Windows animals updated to a compatible
client script.  I don't know what timeline Andrew has in mind
for that.

I guess I'd vote for pulling the trigger in v16 if we can get that
done by the end of April.  Once we're close to beta I think it
must wait for v17 to open.

            regards, tom lane

On Sat, Apr 8, 2023 at 3:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I guess I'd vote for pulling the trigger in v16 if we can get that
> done by the end of April.  Once we're close to beta I think it
> must wait for v17 to open.

I think that sounds reasonable. It would be to the project's advantage
not to have to maintain three build systems for an extra year, but we
can't still be whacking things around right up until the moment we
expect to ship a beta.

However, if this is the direction we're going, we probably need to
give pgsql-packagers a heads up ASAP, because anybody who is still
relying on the MSVC system to build Windows binaries is presumably
going to need some time to adjust. If we rip out the build system
somebody is using a couple of weeks before beta, that might make it
difficult for that person to get the beta out promptly. And I think
there's probably more than just EDB who would be in that situation.

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:
> However, if this is the direction we're going, we probably need to
> give pgsql-packagers a heads up ASAP, because anybody who is still
> relying on the MSVC system to build Windows binaries is presumably
> going to need some time to adjust. If we rip out the build system
> somebody is using a couple of weeks before beta, that might make it
> difficult for that person to get the beta out promptly. And I think
> there's probably more than just EDB who would be in that situation.

Oh ... that's a good point.  Is there anyone besides EDB shipping
MSVC-built executables?  Would it even be practical to switch to
meson with a month-or-so notice?  Seems kind of tight, and it's
not like the packagers volunteered to make this switch.

Maybe we have to bite the bullet and keep MSVC for v16.
If we drop it as soon as v17 opens, there's probably not that
much incremental work involved compared to dropping for v16.

            regards, tom lane

On Mon, Apr 10, 2023 at 12:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > However, if this is the direction we're going, we probably need to
> > give pgsql-packagers a heads up ASAP, because anybody who is still
> > relying on the MSVC system to build Windows binaries is presumably
> > going to need some time to adjust. If we rip out the build system
> > somebody is using a couple of weeks before beta, that might make it
> > difficult for that person to get the beta out promptly. And I think
> > there's probably more than just EDB who would be in that situation.
>
> Oh ... that's a good point.  Is there anyone besides EDB shipping
> MSVC-built executables?  Would it even be practical to switch to
> meson with a month-or-so notice?  Seems kind of tight, and it's
> not like the packagers volunteered to make this switch.

I can't really speak to those questions with confidence.

Perhaps instead of telling pgsql-packagers what we're doing, we could
instead ask them if it would work for them if we did XYZ. Then we
could use that information to inform our decision-making.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, Apr 10, 2023 at 6:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Robert Haas <robertmhaas@gmail.com> writes:
> > However, if this is the direction we're going, we probably need to
> > give pgsql-packagers a heads up ASAP, because anybody who is still
> > relying on the MSVC system to build Windows binaries is presumably
> > going to need some time to adjust. If we rip out the build system
> > somebody is using a couple of weeks before beta, that might make it
> > difficult for that person to get the beta out promptly. And I think
> > there's probably more than just EDB who would be in that situation.
>
> Oh ... that's a good point.  Is there anyone besides EDB shipping
> MSVC-built executables?  Would it even be practical to switch to
> meson with a month-or-so notice?  Seems kind of tight, and it's
> not like the packagers volunteered to make this switch.
>
> Maybe we have to bite the bullet and keep MSVC for v16.
> If we drop it as soon as v17 opens, there's probably not that
> much incremental work involved compared to dropping for v16.

Not involved with any such build tasks anymore, but I think we can
definitely assume there are others than EDB who do that. It's also
used by people who build add-on modules to be loaded in the
EDB-installer-installed systems, I'm sure.

It seems a bit aggressive to those to drop the entire build system out
just before beta.

Thus, +1 on actually keeping it up and dropping it immediately as v17
opens, giving them a year of advantage. And probably updating the docs
(if anybody were to read them.. but at least then we tried) stating
that it's deprecated and will be removed in v17.

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

Magnus Hagander <magnus@hagander.net> writes:
> Thus, +1 on actually keeping it up and dropping it immediately as v17
> opens, giving them a year of advantage. And probably updating the docs
> (if anybody were to read them.. but at least then we tried) stating
> that it's deprecated and will be removed in v17.

Yeah, I think that's the only feasible answer at this point.
Maybe a month or two back we could have done differently,
but there's not a lot of runway now.

Once we do drop src/tools/msvc from HEAD, we should make a point
of reminding -packagers about it, in hopes that they'll work on
the transition sooner than next May.

            regards, tom lane

Hi,

On 2023-04-10 19:55:35 +0100, Dave Page wrote:
> Projects other than the EDB installers use the MSVC build system - e.g.
> pgAdmin uses it’s own builds of libpq and other tools (psql, pg_dump etc)
> that are pretty heavily baked into a fully automated build system (even the
> build servers and all their requirements are baked into Ansible).
> 
> Changing that lot would be non-trivial, though certainly possible, and I
> suspect we’re not the only ones doing that sort of thing.

Do you have a link to the code for that, if it's open? Just to get an
impression for how hard it'd be to switch over?

Greetings,

Andres Freund

Hi,

On 2023-04-10 16:50:20 -0400, Tom Lane wrote:
> Yeah, I think that's the only feasible answer at this point.
> Maybe a month or two back we could have done differently,
> but there's not a lot of runway now.
> 
> Once we do drop src/tools/msvc from HEAD, we should make a point
> of reminding -packagers about it, in hopes that they'll work on
> the transition sooner than next May.

So the plan is:

- add note to docs in HEAD that the src/tools/msvc style of build is
  deprecated
- give -packagers a HEADS up, once the deprecation notice has been added to
  the docs
- have a patch ready to drop src/tools/msvc from HEAD once 16 has branched off
  (i.e. a polished version of what I posted upthread)

On IM Thomas made some point about CI - I wonder if we should add building 16
with src/tools/msvc as an optional CI task? We can't enable it by default
(yet), because we'd not have enough resources to also run that for cfbot. Once
16 forked, we then could set to run automatically in the 16 branch, as cfbot
won't run those.

Greetings,

Andres Freund

On Mon, Apr 10, 2023 at 03:32:19PM -0700, Andres Freund wrote:
> On IM Thomas made some point about CI - I wonder if we should add building 16
> with src/tools/msvc as an optional CI task? We can't enable it by default
> (yet), because we'd not have enough resources to also run that for cfbot. Once
> 16 forked, we then could set to run automatically in the 16 branch, as cfbot
> won't run those.

Getting a CI job able to do some validation for MSVC would be indeed
nice.  What's the plan in the buildfarm with this coverage?  Would all
the animals switch to meson (Chocolatey + StrawberryPerl, I assume)
for the job or will there still be some coverage with MSVC for v16
there?
--
Michael

On 4/10/23 4:50 PM, Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Thus, +1 on actually keeping it up and dropping it immediately as v17
>> opens, giving them a year of advantage. And probably updating the docs
>> (if anybody were to read them.. but at least then we tried) stating
>> that it's deprecated and will be removed in v17.
> 
> Yeah, I think that's the only feasible answer at this point.
> Maybe a month or two back we could have done differently,
> but there's not a lot of runway now.
> 
> Once we do drop src/tools/msvc from HEAD, we should make a point
> of reminding -packagers about it, in hopes that they'll work on
> the transition sooner than next May.

[personal opinion, not RMT]

The last point would be my reasoning for "why not now" given deadlines 
are a pretty good motivator to get things done.

That said, if the plan is to do this "shortly thereafter" for v17 and it 
makes the transition easier, I'm all for that.

Jonathan

On Tue, Apr 11, 2023 at 12:27 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2023-04-10 19:55:35 +0100, Dave Page wrote:
> > Projects other than the EDB installers use the MSVC build system - e.g.
> > pgAdmin uses it’s own builds of libpq and other tools (psql, pg_dump etc)
> > that are pretty heavily baked into a fully automated build system (even the
> > build servers and all their requirements are baked into Ansible).
> >
> > Changing that lot would be non-trivial, though certainly possible, and I
> > suspect we’re not the only ones doing that sort of thing.
>
> Do you have a link to the code for that, if it's open? Just to get an
> impression for how hard it'd be to switch over?


The pgadmin docs/readme refers to
https://github.com/pgadmin-org/pgadmin4/tree/master/pkg/win32

It clearly doesn't have the full automation stuff, but appears to have
the parts about building the postgres dependency.

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On Tue, 11 Apr 2023 at 08:09, Magnus Hagander <magnus@hagander.net> wrote:

On Tue, Apr 11, 2023 at 12:27 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2023-04-10 19:55:35 +0100, Dave Page wrote:
> > Projects other than the EDB installers use the MSVC build system - e.g.
> > pgAdmin uses it’s own builds of libpq and other tools (psql, pg_dump etc)
> > that are pretty heavily baked into a fully automated build system (even the
> > build servers and all their requirements are baked into Ansible).
> >
> > Changing that lot would be non-trivial, though certainly possible, and I
> > suspect we’re not the only ones doing that sort of thing.
>
> Do you have a link to the code for that, if it's open? Just to get an
> impression for how hard it'd be to switch over?


The pgadmin docs/readme refers to
https://github.com/pgadmin-org/pgadmin4/tree/master/pkg/win32

It clearly doesn't have the full automation stuff, but appears to have
the parts about building the postgres dependency.

Yeah, that's essentially the manual process, though I haven't tested it in a while. The Ansible stuff is not currently public. I suspect (or rather, hope) that we can pull in all the additional packages required using Chocolatey which shouldn't be too onerous.

Probably my main concern is that the Meson build can use the same version of the VC++ compiler that we use (v14), which is carefully matched for compatibility with all the various components, just in case anything passes CRT pointers around. Python is the one thing we don't build ourselves on Windows and the process will build modules like gssapi and psycopg (which links with libpq of course), so we're basically following what they use.

For meson you just need to to "pip install meson ninja" in your python distro and you should be good to go (they will be installed in python's Scripts directory). Don't use chocolatey to install meson/ninja - I ran into issues doing that.

AFAICT meson will use whatever version of VC you have installed, although I have only been testing with VC2019.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 4/11/23 7:54 AM, Dave Page wrote:
> 
> 
> On Tue, 11 Apr 2023 at 11:58, Andrew Dunstan <andrew@dunslane.net 
> <mailto:andrew@dunslane.net>> wrote:
> 
>     For meson you just need to to "pip install meson ninja" in your
>     python distro and you should be good to go (they will be installed
>     in python's Scripts directory). Don't use chocolatey to install
>     meson/ninja - I ran into issues doing that.
> 
>     AFAICT meson will use whatever version of VC you have installed,
>     although I have only been testing with VC2019.
> 
> OK, that sounds easy enough then (famous last words!)

[RMT hat]

Dave -- does this mean you see a way forward on moving the Windows 
builds over to use Meson instead of MSVC?

Do you think we'll have enough info by end of this week to make a 
decision on whether we can drop MSVC in v16?

Thanks,

Jonathan

Dave Page <dpage@pgadmin.org> writes:
> On Tue, 11 Apr 2023 at 13:52, Jonathan S. Katz <jkatz@postgresql.org> wrote:
>> Do you think we'll have enough info by end of this week to make a
>> decision on whether we can drop MSVC in v16?

> There's no way I can test anything this week - I'm on leave for most of it
> and AFK.
> But, my point was more that there are almost certainly more projects using
> the MSVC build system than the EDB installers; pgAdmin being just one
> example.

Yeah.  Even if EDB can manage this, we're talking about going from
"src/tools/msvc is the only option" in v15 to "meson is the only
option" in v16.  That seems pretty abrupt.  Notably, it's assuming
that there are no big problems in the meson build system that will
take awhile to fix once discovered by users.  That's a large
assumption for code that hasn't even reached beta yet.

Sadly, I think we really have to ship both build systems in v16.

            regards, tom lane

On 4/11/23 9:49 AM, Tom Lane wrote:
> Dave Page <dpage@pgadmin.org> writes:
>> On Tue, 11 Apr 2023 at 13:52, Jonathan S. Katz <jkatz@postgresql.org> wrote:
>>> Do you think we'll have enough info by end of this week to make a
>>> decision on whether we can drop MSVC in v16?
> 
>> There's no way I can test anything this week - I'm on leave for most of it
>> and AFK.
>> But, my point was more that there are almost certainly more projects using
>> the MSVC build system than the EDB installers; pgAdmin being just one
>> example.
> 
> Yeah.  Even if EDB can manage this, we're talking about going from
> "src/tools/msvc is the only option" in v15 to "meson is the only
> option" in v16.  That seems pretty abrupt.  Notably, it's assuming
> that there are no big problems in the meson build system that will
> take awhile to fix once discovered by users.  That's a large
> assumption for code that hasn't even reached beta yet.
[Personal hat]

We'll probably see some of this for non-Windows builds, too. Granted, 
autoconf still seems to work, at least based on my tests.

> Sadly, I think we really have to ship both build systems in v16.

[Personal hat]

I've come to this conclusion, too -- it does mean 5 more years of 
supporting it.

But maybe we can make it clear in the release notes + docs that this is 
slated for deprecation and will be removed from v17? That way we can say 
"we provided ample warning to move to the new build system."

Thanks,

Jonathan

"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> On 4/11/23 9:49 AM, Tom Lane wrote:
>> Sadly, I think we really have to ship both build systems in v16.

> But maybe we can make it clear in the release notes + docs that this is 
> slated for deprecation and will be removed from v17? That way we can say 
> "we provided ample warning to move to the new build system."

Yes, we absolutely should do that, as already discussed upthread.

            regards, tom lane

On 4/11/23 10:12 AM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz@postgresql.org> writes:
>> On 4/11/23 9:49 AM, Tom Lane wrote:
>>> Sadly, I think we really have to ship both build systems in v16.
> 
>> But maybe we can make it clear in the release notes + docs that this is
>> slated for deprecation and will be removed from v17? That way we can say
>> "we provided ample warning to move to the new build system."
> 
> Yes, we absolutely should do that, as already discussed upthread.

Ah yes, I saw Andres' notep[1] yesterday and had already forgotten. +1 
on that plan.

Jonathan

[1] 
https://www.postgresql.org/message-id/20230410223219.4tllxhz3hgwhh4tm%40awork3.anarazel.de

Hi,

On 2023-04-11 09:05:31 +0100, Dave Page wrote:
> Probably my main concern is that the Meson build can use the same version
> of the VC++ compiler that we use (v14), which is carefully matched for
> compatibility with all the various components, just in case anything passes
> CRT pointers around.

FWIW, Independent of meson, I don't think support for VC 2015 in postgres is
long for the world. Later versions of msvc have increased the C standard
compliance a fair bit... It's also a few years out of normal support.

I've not tested building with 2015, but I don't know of anything that should
prevent building with meson with it.  I am fairly sure that it can't build
tab-complete.c, but you're presumably not building with tab completion support
anyway?

Greetings,

Andres Freund

Hi,

On 2023-04-11 10:44:09 -0400, Jonathan S. Katz wrote:
> On 4/11/23 10:12 AM, Tom Lane wrote:
> > "Jonathan S. Katz" <jkatz@postgresql.org> writes:
> > > On 4/11/23 9:49 AM, Tom Lane wrote:
> > > > Sadly, I think we really have to ship both build systems in v16.
> > 
> > > But maybe we can make it clear in the release notes + docs that this is
> > > slated for deprecation and will be removed from v17? That way we can say
> > > "we provided ample warning to move to the new build system."
> > 
> > Yes, we absolutely should do that, as already discussed upthread.
> 
> Ah yes, I saw Andres' notep[1] yesterday and had already forgotten. +1 on
> that plan.

Here's a draft docs change.

I added the <warning/> in two places in install-windows.sgml so it's visible
on both the generated pages in the chunked output. That does mean it's visible
twice nearby in the single-page output, but I don't think that's commonly
used.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> Here's a draft docs change.

> I added the <warning/> in two places in install-windows.sgml so it's visible
> on both the generated pages in the chunked output. That does mean it's visible
> twice nearby in the single-page output, but I don't think that's commonly
> used.

I don't agree with placing that first hunk before the para that tells
people to use a binary distribution, as it's completely irrelevant
if they take that advice.  I'm not really sure we need it at all,
because quite a bit of the text right after that is not specific to using
the src/tools/msvc scripts either.  I think the <warning> under "Building
with <productname>Visual C++</productname> ..." is sufficient.

The other two changes look fine.

            regards, tom lane

On 2023-Apr-11, Michael Paquier wrote:

> Getting a CI job able to do some validation for MSVC would be indeed
> nice.  What's the plan in the buildfarm with this coverage?  Would all
> the animals switch to meson (Chocolatey + StrawberryPerl, I assume)
> for the job or will there still be some coverage with MSVC for v16
> there?

If we keep MSVC support in 16, then I agree we should have a CI task for
it -- and IMO we should make ti automatically triggers whenever any
Makefile or meson.build is modified.  Hopefully we won't touch them
much now that the branch is feature-frozen, but it could still happen.

Do we have code for it already, even if incomplete?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

Hi,

On 2023-04-11 13:30:15 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Here's a draft docs change.
> 
> > I added the <warning/> in two places in install-windows.sgml so it's visible
> > on both the generated pages in the chunked output. That does mean it's visible
> > twice nearby in the single-page output, but I don't think that's commonly
> > used.
> 
> I don't agree with placing that first hunk before the para that tells
> people to use a binary distribution, as it's completely irrelevant
> if they take that advice.

Fair point.


> I'm not really sure we need it at all, because quite a bit of the text right
> after that is not specific to using the src/tools/msvc scripts either.  I
> think the <warning> under "Building with <productname>Visual
> C++</productname> ..." is sufficient.

It seemed nicer to have it on all the "Installation from Source Code on Windows"
pages, but...

Except that we're planning to remove it anyway, the structure of the docs here
seems a bit off...

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> Except that we're planning to remove it anyway, the structure of the docs here
> seems a bit off...

Indeed.  We'll have to migrate some of that info elsewhere when the
time comes.

            regards, tom lane

Hi,

On 2023-04-11 19:44:20 +0200, Alvaro Herrera wrote:
> On 2023-Apr-11, Michael Paquier wrote:
>
> > Getting a CI job able to do some validation for MSVC would be indeed
> > nice.  What's the plan in the buildfarm with this coverage?  Would all
> > the animals switch to meson (Chocolatey + StrawberryPerl, I assume)
> > for the job or will there still be some coverage with MSVC for v16
> > there?
>
> If we keep MSVC support in 16, then I agree we should have a CI task for
> it -- and IMO we should make ti automatically triggers whenever any
> Makefile or meson.build is modified.  Hopefully we won't touch them
> much now that the branch is feature-frozen, but it could still happen.

Once 16 branched, we can just have it always run, I think. It's just the
development branch where it's worth avoiding that (for cfbot and personal
hackery).

I guess we could do something like:

  manual: "changesInclude('**.meson.build', '**Makefile*', '**.mk', 'src/tools/msvc/**')"

so the task would be manual triggered if none of those files change. If you
have write rights on the repository in question, you can trigger manual tasks
with a click.


> Do we have code for it already, even if incomplete?

My meson branch has a commit adding a bunch of additional tasks. Including
building with src/tools/msvc, building with meson + msbuild, openbsd, netbsd.

https://github.com/postgres/postgres/commit/8f7c2ffb5a5e8f0ef3722e2439484187c1356416

Currently src/tools/msvc does build successfully, although the tests haven't
finished yet:
https://cirrus-ci.com/build/6298699714789376


(the cause for the opensuse failure is known, need to find cycles to tackle
that, not related to meson)

Greetings,

Andres Freund

On 08.04.23 21:10, Andres Freund wrote:
> Do we want to drop src/tools/msvc support in 16 (i.e. now), or do it early in
> 17?

Can you build using meson from a distribution tarball on Windows?