testclient.exe installed under MSVC

On Sun, May 01, 2022 at 01:07:06AM -0700, Noah Misch wrote:

My annual audit for executables missing Windows icons turned up these:

pginstall/bin/testclient.exe
pginstall/bin/uri-regress.exe

I was going to add the icons, but I felt the testclient.exe name is too
generic-sounding to be installed. testclient originated in commit ebc8b7d. I
recommend ceasing to install both programs under MSVC. (The GNU make build
system does not install them.)

But MSVC works differently. vcregress.pl does a TempInstall(), which
is a simple Install(), so isn't it going to be an issue for the tests
if these two tools are not installed anymore?

If that's unwanted for some reason, could you
rename testclient to something like libpq_test?

Yes, the renaming makes sense. I'd say to do more, and also rename
uri-regress, removing the hyphen from the binary name and prefix both
binaries with a "pg_".
--
Michael

On Sun, May 01, 2022 at 10:23:18PM +0900, Michael Paquier wrote:

On Sun, May 01, 2022 at 01:07:06AM -0700, Noah Misch wrote:

My annual audit for executables missing Windows icons turned up these:

pginstall/bin/testclient.exe
pginstall/bin/uri-regress.exe

I was going to add the icons, but I felt the testclient.exe name is too
generic-sounding to be installed. testclient originated in commit ebc8b7d. I
recommend ceasing to install both programs under MSVC. (The GNU make build
system does not install them.)

See also:
a17fd67d2f2861ae0ce00d1aeefdf2facc47cd5e Build libpq test programs under MSVC.
/messages/by-id/74952229-b3b0-fe47-f958-4088529a3f21@dunslane.net MSVC build system installs extra executables
/messages/by-id/e4233934-98a6-6f76-46a0-992c0f4f1208@dunslane.net Re: set TESTDIR from perl rather than Makefile

I'm not really sure what the plan is for the TESTDIR patches. Is "vcregress
alltaptests" still an interesting patch to pursue, or is that going to be
obsoleted by meson build ?

But MSVC works differently. vcregress.pl does a TempInstall(), which
is a simple Install(), so isn't it going to be an issue for the tests
if these two tools are not installed anymore?

Andrew didn't propose any mechanism for avoiding installation of the
executables, so it would break the tests. However, at least cfbot currently
doesn't run them anyway.

One idea is if "vcregress install" accepted an option like
"vcregress install check", which would mean "install extra binaries needed for
running tests". Something maybe not much more elegant than this.

next
if ($insttype eq "client" && !grep { $_ eq $pf }
@client_program_files);

+ next if ($pf =~ /testclient|uri-regress/);

On 1 May 2022, at 15:23, Michael Paquier <michael@paquier.xyz> wrote:

On Sun, May 01, 2022 at 01:07:06AM -0700, Noah Misch wrote:

My annual audit for executables missing Windows icons turned up these:

pginstall/bin/testclient.exe
pginstall/bin/uri-regress.exe

I was going to add the icons, but I felt the testclient.exe name is too
generic-sounding to be installed. testclient originated in commit ebc8b7d. I
recommend ceasing to install both programs under MSVC. (The GNU make build
system does not install them.)

But MSVC works differently. vcregress.pl does a TempInstall(), which
is a simple Install(), so isn't it going to be an issue for the tests
if these two tools are not installed anymore?

If that's unwanted for some reason, could you
rename testclient to something like libpq_test?

Yes, the renaming makes sense. I'd say to do more, and also rename
uri-regress, removing the hyphen from the binary name and prefix both
binaries with a "pg_".

Renaming is probably the best option given how MSVC works. Using a pg_ prefix
makes them sound like actual useful tools though with (albeit small) risk for
confusion? Noah's suggestion of libpq_ is perhaps better: libpq_testclient.

--
Daniel Gustafsson https://vmware.com/

On Mon, 2022-05-02 at 15:14 +0200, Daniel Gustafsson wrote:

Using a pg_ prefix
makes them sound like actual useful tools though with (albeit small) risk for
confusion? Noah's suggestion of libpq_ is perhaps better: libpq_testclient.

+1

I also like Justin's idea of only installing the test executables when
asked to explicitly, but I don't know enough about our existing MSVC
conventions to have a strong opinion there.

Thanks,
--Jacob

On Mon, May 02, 2022 at 03:14:50PM +0200, Daniel Gustafsson wrote:

On 1 May 2022, at 15:23, Michael Paquier <michael@paquier.xyz> wrote:
On Sun, May 01, 2022 at 01:07:06AM -0700, Noah Misch wrote:

My annual audit for executables missing Windows icons turned up these:

pginstall/bin/testclient.exe
pginstall/bin/uri-regress.exe

I was going to add the icons, but I felt the testclient.exe name is too
generic-sounding to be installed. testclient originated in commit ebc8b7d. I
recommend ceasing to install both programs under MSVC. (The GNU make build
system does not install them.)

But MSVC works differently. vcregress.pl does a TempInstall(), which
is a simple Install(), so isn't it going to be an issue for the tests
if these two tools are not installed anymore?

Resolving that would be part of any project to stop installing them.

If that's unwanted for some reason, could you
rename testclient to something like libpq_test?

Yes, the renaming makes sense. I'd say to do more, and also rename
uri-regress, removing the hyphen from the binary name and prefix both
binaries with a "pg_".

Renaming is probably the best option given how MSVC works. Using a pg_ prefix
makes them sound like actual useful tools though with (albeit small) risk for
confusion? Noah's suggestion of libpq_ is perhaps better: libpq_testclient.

Agreed. libpq_testclient and libpq_uri_regress sound fine.

On 3 May 2022, at 04:02, Noah Misch <noah@leadboat.com> wrote:
On Mon, May 02, 2022 at 03:14:50PM +0200, Daniel Gustafsson wrote:

Renaming is probably the best option given how MSVC works. Using a pg_ prefix
makes them sound like actual useful tools though with (albeit small) risk for
confusion? Noah's suggestion of libpq_ is perhaps better: libpq_testclient.

Agreed. libpq_testclient and libpq_uri_regress sound fine.

The attached works in both Linux and (Cirrus CI) MSVC for me.

--
Daniel Gustafsson https://vmware.com/

On Tue, May 03, 2022 at 03:04:26PM +0200, Daniel Gustafsson wrote:

On 3 May 2022, at 04:02, Noah Misch <noah@leadboat.com> wrote:
On Mon, May 02, 2022 at 03:14:50PM +0200, Daniel Gustafsson wrote:

Renaming is probably the best option given how MSVC works. Using a pg_ prefix
makes them sound like actual useful tools though with (albeit small) risk for
confusion? Noah's suggestion of libpq_ is perhaps better: libpq_testclient.

Agreed. libpq_testclient and libpq_uri_regress sound fine.

The attached works in both Linux and (Cirrus CI) MSVC for me.

Michael Paquier recommended s/-/_/ for uri-regress, and I agree with that.
What do you think?

On 2022-May-03, Noah Misch wrote:

Michael Paquier recommended s/-/_/ for uri-regress, and I agree with that.
What do you think?

libpq_uri-regress is horrible, so +1 for that. I would personally
rename more thoroughly (say pq_uri_test), but I doubt it's worth the
bikeshedding effort.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"At least to kernel hackers, who really are human, despite occasional
rumors to the contrary" (LWN.net)

On 3 May 2022, at 15:58, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

On 2022-May-03, Noah Misch wrote:

Michael Paquier recommended s/-/_/ for uri-regress, and I agree with that.
What do you think?

libpq_uri-regress is horrible, so +1 for that.

Agreed, I'll do that before pushing.

--
Daniel Gustafsson https://vmware.com/

On 2022-05-01 Su 09:23, Michael Paquier wrote:

On Sun, May 01, 2022 at 01:07:06AM -0700, Noah Misch wrote:

My annual audit for executables missing Windows icons turned up these:

pginstall/bin/testclient.exe
pginstall/bin/uri-regress.exe

I was going to add the icons, but I felt the testclient.exe name is too
generic-sounding to be installed. testclient originated in commit ebc8b7d. I
recommend ceasing to install both programs under MSVC. (The GNU make build
system does not install them.)

But MSVC works differently. vcregress.pl does a TempInstall(), which
is a simple Install(), so isn't it going to be an issue for the tests
if these two tools are not installed anymore?

If that's unwanted for some reason, could you
rename testclient to something like libpq_test?

Yes, the renaming makes sense. I'd say to do more, and also rename
uri-regress, removing the hyphen from the binary name and prefix both
binaries with a "pg_".

I've complained before about binaries that are installed under MSVC
where the equivalent are not installed under Unix or msys{2}.

I think we should make the standard MSVC install look as much like the
standard Unix/msys install as possible. If we need a test mode that
installs a few extra things then that can be managed fairly simply I
think. I'm prepared to help out with that.

cheers

andrew

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

On 3 May 2022, at 16:50, Daniel Gustafsson <daniel@yesql.se> wrote:

On 3 May 2022, at 15:58, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

On 2022-May-03, Noah Misch wrote:

Michael Paquier recommended s/-/_/ for uri-regress, and I agree with that.
What do you think?

libpq_uri-regress is horrible, so +1 for that.

Agreed, I'll do that before pushing.

Done that way.

--
Daniel Gustafsson https://vmware.com/

On 4 May 2022, at 02:34, Andrew Dunstan <andrew@dunslane.net> wrote:

I think we should make the standard MSVC install look as much like the
standard Unix/msys install as possible.

+1

--
Daniel Gustafsson https://vmware.com/