Thread: pgsql: Reference test binary using TESTDIR in 001_libpq_pipeline.pl.
Reference test binary using TESTDIR in 001_libpq_pipeline.pl. The previous approach didn't really work on windows, due to the PATH separator being ';' not ':'. Instead of making the PATH change more complicated, reference the binary using the TESTDIR environment. Reported-By: Andres Freund <andres@anarazel.de> Suggested-By: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de Backpatch: 14-, where the test was introduced. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/795862c280c5949bafcd8c44543d887fd32b590a Modified Files -------------- src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
On 10/1/21 6:34 PM, Andres Freund wrote: > Reference test binary using TESTDIR in 001_libpq_pipeline.pl. > > The previous approach didn't really work on windows, due to the PATH separator > being ';' not ':'. Instead of making the PATH change more complicated, > reference the binary using the TESTDIR environment. > > Reported-By: Andres Freund <andres@anarazel.de> > Suggested-By: Andrew Dunstan <andrew@dunslane.net> > Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de > Backpatch: 14-, where the test was introduced. > I don't think any of us were thinking very clearly about this. Of course, MSVC doesn't build executables in $TESTDIR. If we want to pick up the executable from where it's built we'll need a little help from vcregress.pl. I haven't tested it but What I have in mind is something like the attached. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Attachment
On 10/8/21 11:41 AM, Andrew Dunstan wrote: > On 10/1/21 6:34 PM, Andres Freund wrote: >> Reference test binary using TESTDIR in 001_libpq_pipeline.pl. >> >> The previous approach didn't really work on windows, due to the PATH separator >> being ';' not ':'. Instead of making the PATH change more complicated, >> reference the binary using the TESTDIR environment. >> >> Reported-By: Andres Freund <andres@anarazel.de> >> Suggested-By: Andrew Dunstan <andrew@dunslane.net> >> Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de >> Backpatch: 14-, where the test was introduced. >> > I don't think any of us were thinking very clearly about this. Of > course, MSVC doesn't build executables in $TESTDIR. If we want to pick > up the executable from where it's built we'll need a little help from > vcregress.pl. I haven't tested it but What I have in mind is something > like the attached. > > Confirmed that this makes the test pass. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Hi, On 2021-10-08 11:41:50 -0400, Andrew Dunstan wrote: > On 10/1/21 6:34 PM, Andres Freund wrote: > > Reference test binary using TESTDIR in 001_libpq_pipeline.pl. > > > > The previous approach didn't really work on windows, due to the PATH separator > > being ';' not ':'. Instead of making the PATH change more complicated, > > reference the binary using the TESTDIR environment. > > > > Reported-By: Andres Freund <andres@anarazel.de> > > Suggested-By: Andrew Dunstan <andrew@dunslane.net> > > Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de > > Backpatch: 14-, where the test was introduced. > > > > I don't think any of us were thinking very clearly about this. Of > course, MSVC doesn't build executables in $TESTDIR. If we want to pick > up the executable from where it's built we'll need a little help from > vcregress.pl. I haven't tested it but What I have in mind is something > like the attached. Hm. Clearly this needs more work. But I don't really like having checks for MSBUILDDIR in individual tests - we should do this somewhere more central. Afaictl the windows build just installs libpq_pipeline.exe. If we make sure that on !msvc builds CURDIR is on PATH and on msvc temp_install/bin/ is on PATH (which it should already be), then we should be good? I guess the reason this didn't work for me before was that I invoked install.pl from the top directory, which then didn't install libpq_pipeline.exe, because of the config.pl issue I mentioned somewhere? Greetings, Andres Freund
On 10/8/21 5:14 PM, Andres Freund wrote: > Hi, > > On 2021-10-08 11:41:50 -0400, Andrew Dunstan wrote: >> On 10/1/21 6:34 PM, Andres Freund wrote: >>> Reference test binary using TESTDIR in 001_libpq_pipeline.pl. >>> >>> The previous approach didn't really work on windows, due to the PATH separator >>> being ';' not ':'. Instead of making the PATH change more complicated, >>> reference the binary using the TESTDIR environment. >>> >>> Reported-By: Andres Freund <andres@anarazel.de> >>> Suggested-By: Andrew Dunstan <andrew@dunslane.net> >>> Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de >>> Backpatch: 14-, where the test was introduced. >>> >> I don't think any of us were thinking very clearly about this. Of >> course, MSVC doesn't build executables in $TESTDIR. If we want to pick >> up the executable from where it's built we'll need a little help from >> vcregress.pl. I haven't tested it but What I have in mind is something >> like the attached. > Hm. Clearly this needs more work. But I don't really like having checks for > MSBUILDDIR in individual tests - we should do this somewhere more central. > Afaictl the windows build just installs libpq_pipeline.exe. If we make sure > that on !msvc builds CURDIR is on PATH and on msvc temp_install/bin/ is on > PATH (which it should already be), then we should be good? > > I guess the reason this didn't work for me before was that I invoked > install.pl from the top directory, which then didn't install > libpq_pipeline.exe, because of the config.pl issue I mentioned somewhere? The whole point of 6abc8c2596 AIUI was to have this program not installed (that's why for the test we need to get it from $TESTDIR where it's built except when we're using MSVC). That commit should probably have made sure the MSVC install process likewise didn't install it. So ISTM that relying on it being installed is going in the wrong direction. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 10/8/21 8:33 PM, Andrew Dunstan wrote: > On 10/8/21 5:14 PM, Andres Freund wrote: >> Hi, >> >> On 2021-10-08 11:41:50 -0400, Andrew Dunstan wrote: >>> On 10/1/21 6:34 PM, Andres Freund wrote: >>>> Reference test binary using TESTDIR in 001_libpq_pipeline.pl. >>>> >>>> The previous approach didn't really work on windows, due to the PATH separator >>>> being ';' not ':'. Instead of making the PATH change more complicated, >>>> reference the binary using the TESTDIR environment. >>>> >>>> Reported-By: Andres Freund <andres@anarazel.de> >>>> Suggested-By: Andrew Dunstan <andrew@dunslane.net> >>>> Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de >>>> Backpatch: 14-, where the test was introduced. >>>> >>> I don't think any of us were thinking very clearly about this. Of >>> course, MSVC doesn't build executables in $TESTDIR. If we want to pick >>> up the executable from where it's built we'll need a little help from >>> vcregress.pl. I haven't tested it but What I have in mind is something >>> like the attached. >> Hm. Clearly this needs more work. But I don't really like having checks for >> MSBUILDDIR in individual tests - we should do this somewhere more central. >> Afaictl the windows build just installs libpq_pipeline.exe. If we make sure >> that on !msvc builds CURDIR is on PATH and on msvc temp_install/bin/ is on >> PATH (which it should already be), then we should be good? >> >> I guess the reason this didn't work for me before was that I invoked >> install.pl from the top directory, which then didn't install >> libpq_pipeline.exe, because of the config.pl issue I mentioned somewhere? > > > The whole point of 6abc8c2596 AIUI was to have this program not > installed (that's why for the test we need to get it from $TESTDIR where > it's built except when we're using MSVC). That commit should probably > have made sure the MSVC install process likewise didn't install it. So > ISTM that relying on it being installed is going in the wrong direction. > > It's been 10 days or so, so we really need to get this fixed. Is the attached more to your taste? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com