Thread: testclient.exe installed under MSVC
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.) If that's unwanted for some reason, could you rename testclient to something like libpq_test? Thanks, nm
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
Attachment
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. https://www.postgresql.org/message-id/74952229-b3b0-fe47-f958-4088529a3f21@dunslane.net MSVC build system installs extraexecutables https://www.postgresql.org/message-id/e4233934-98a6-6f76-46a0-992c0f4f1208@dunslane.net Re: set TESTDIR from perl ratherthan 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/
Attachment
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/