Thread: testclient.exe installed under MSVC

testclient.exe installed under MSVC

From
Noah Misch
Date:
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



Re: testclient.exe installed under MSVC

From
Michael Paquier
Date:
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

Re: testclient.exe installed under MSVC

From
Justin Pryzby
Date:
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/);




Re: testclient.exe installed under MSVC

From
Daniel Gustafsson
Date:
> 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/




Re: testclient.exe installed under MSVC

From
Jacob Champion
Date:
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

Re: testclient.exe installed under MSVC

From
Noah Misch
Date:
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.



Re: testclient.exe installed under MSVC

From
Daniel Gustafsson
Date:
> 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

Re: testclient.exe installed under MSVC

From
Noah Misch
Date:
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?



Re: testclient.exe installed under MSVC

From
Alvaro Herrera
Date:
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)



Re: testclient.exe installed under MSVC

From
Daniel Gustafsson
Date:
> 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/




Re: testclient.exe installed under MSVC

From
Andrew Dunstan
Date:
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




Re: testclient.exe installed under MSVC

From
Daniel Gustafsson
Date:
> 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/




Re: testclient.exe installed under MSVC

From
Daniel Gustafsson
Date:
> 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/