Thread: [COMMITTERS] pgsql: Record full paths of programs sought by "configure".
Record full paths of programs sought by "configure". Previously we had a mix of uses of AC_CHECK_PROG[S] and AC_PATH_PROG[S]. The only difference between those macros is that the latter emits the full path to the program it finds, eg "/usr/bin/prove", whereas the former emits just "prove". Let's standardize on always emitting the full path; this is better for documentation of the build, and it might prevent some types of failures if later build steps are done with a different PATH setting. I did not touch the AC_CHECK_PROG[S] calls in ax_pthread.m4 and ax_prog_perl_modules.m4. There seems no need to make those diverge from upstream, since we do not record the programs sought by the former, while the latter's call to AC_CHECK_PROG(PERL,...) will never be reached. Discussion: https://postgr.es/m/25937.1501433410@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/40b9f192170a300cd9456eb71ba7c792ba9533e1 Modified Files -------------- config/docbook.m4 | 2 +- config/programs.m4 | 6 +- configure | 285 +++++++++++++++++++++++++++++++---------------------- configure.in | 22 ++--- 4 files changed, 180 insertions(+), 135 deletions(-)
Re: [COMMITTERS] pgsql: Record full paths of programs sought by"configure".
From
Andrew Dunstan
Date:
On 07/31/2017 01:02 PM, Tom Lane wrote: > Record full paths of programs sought by "configure". > > Previously we had a mix of uses of AC_CHECK_PROG[S] and AC_PATH_PROG[S]. > The only difference between those macros is that the latter emits the > full path to the program it finds, eg "/usr/bin/prove", whereas the > former emits just "prove". Let's standardize on always emitting the > full path; this is better for documentation of the build, and it might > prevent some types of failures if later build steps are done with > a different PATH setting. > > I did not touch the AC_CHECK_PROG[S] calls in ax_pthread.m4 and > ax_prog_perl_modules.m4. There seems no need to make those diverge from > upstream, since we do not record the programs sought by the former, while > the latter's call to AC_CHECK_PROG(PERL,...) will never be reached. > > Discussion: https://postgr.es/m/25937.1501433410@sss.pgh.pa.us The problem with this commit, as jacana is demonstrating, is that on Msys it finds the wrong prove. configure needs to runagainst the perl we build against, i.e. a native Windows perl, but prove needs to run with the perl from the MSys DTKthat understands MSys virtualized paths. I have a hack that will allow the buildfarm to overcome the difficulty, (essentiallyit passes 'PROVE=prove' to make) but that's fairly ugly and certainly non-intuitive for someone running an MSysbuild and TAP tests without the buildfarm client. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 07/31/2017 01:02 PM, Tom Lane wrote:
>> Record full paths of programs sought by "configure".
> The problem with this commit, as jacana is demonstrating, is that on
> Msys it finds the wrong prove. configure needs to run against the perl
> we build against, i.e. a native Windows perl, but prove needs to run
> with the perl from the MSys DTK that understands MSys virtualized
> paths. I have a hack that will allow the buildfarm to overcome the
> difficulty, (essentially it passes 'PROVE=prove' to make) but that's
> fairly ugly and certainly non-intuitive for someone running an MSys
> build and TAP tests without the buildfarm client.
I'm confused. AFAIK, that commit did not change which "prove" would
be used --- at least not unless you change PATH between configure and
make. It only changed how specifically that program would be named in
Makefile.global. Please clarify how that broke anything.
regards, tom lane
Re: [COMMITTERS] pgsql: Record full paths of programs sought by"configure".
From
Andrew Dunstan
Date:
On 08/07/2017 03:21 PM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 07/31/2017 01:02 PM, Tom Lane wrote: >>> Record full paths of programs sought by "configure". >> The problem with this commit, as jacana is demonstrating, is that on >> Msys it finds the wrong prove. configure needs to run against the perl >> we build against, i.e. a native Windows perl, but prove needs to run >> with the perl from the MSys DTK that understands MSys virtualized >> paths. I have a hack that will allow the buildfarm to overcome the >> difficulty, (essentially it passes 'PROVE=prove' to make) but that's >> fairly ugly and certainly non-intuitive for someone running an MSys >> build and TAP tests without the buildfarm client. > I'm confused. AFAIK, that commit did not change which "prove" would > be used --- at least not unless you change PATH between configure and > make. It only changed how specifically that program would be named in > Makefile.global. Please clarify how that broke anything. > > That's exactly what we do. See <https://github.com/PGBuildFarm/client-code/blob/master/run_build.pl> at line 1649. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: > On 08/07/2017 03:21 PM, Tom Lane wrote: >> I'm confused. AFAIK, that commit did not change which "prove" would >> be used --- at least not unless you change PATH between configure and >> make. It only changed how specifically that program would be named in >> Makefile.global. Please clarify how that broke anything. > That's exactly what we do. See > <https://github.com/PGBuildFarm/client-code/blob/master/run_build.pl> at > line 1649. My goodness, that's ugly. Is it really better than injecting "PROVE=prove"? (I'd suggest saying that to configure, not make, so that the configure log bears some resemblance to what you want done.) regards, tom lane
Re: [COMMITTERS] pgsql: Record full paths of programs sought by"configure".
From
Andrew Dunstan
Date:
On 08/07/2017 03:36 PM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 08/07/2017 03:21 PM, Tom Lane wrote: >>> I'm confused. AFAIK, that commit did not change which "prove" would >>> be used --- at least not unless you change PATH between configure and >>> make. It only changed how specifically that program would be named in >>> Makefile.global. Please clarify how that broke anything. >> That's exactly what we do. See >> <https://github.com/PGBuildFarm/client-code/blob/master/run_build.pl> at >> line 1649. > My goodness, that's ugly. Is it really better than injecting > "PROVE=prove"? (I'd suggest saying that to configure, not make, > so that the configure log bears some resemblance to what you > want done.) > > This is what we had to do BEFORE the change in this commit. Now it's no longer sufficient. It would certainly be better if we could tell configure a path to prove and a path to the perl we need to test IPC::Run against. e.g. PROVE=/usr/bin/prove PROVE_PERL=/usr/bin/perl configure ... The problem in all this is that we're assuming incorrectly that the perl we use to build against is the same as the perl we need to run the build with. On Msys that's emphatically not true. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 08/07/2017 03:36 PM, Tom Lane wrote:
>> My goodness, that's ugly. Is it really better than injecting
>> "PROVE=prove"? (I'd suggest saying that to configure, not make,
>> so that the configure log bears some resemblance to what you
>> want done.)
> This is what we had to do BEFORE the change in this commit. Now it's no
> longer sufficient.
Sorry, I was imprecise. What I'm suggesting is that you drop the
runtime PATH-foolery and instead put this in configure's environment:
PROVE=$perlpathdir/prove
Otherwise you're basically lying to configure about what you're going
to use, and that's always going to break eventually.
> It would certainly be better if we could tell configure a path to prove
> and a path to the perl we need to test IPC::Run against.
Hm, yeah, the IPC::Run test would need to deal with this as well.
A PROVE_PERL environment variable is one way. Or maybe simpler,
just skip the probe for IPC::Run if PROVE has been specified
externally; assume the user knows what he's doing in that case.
Are there any other gotchas in the build sequence?
> The problem in all this is that we're assuming incorrectly that the perl
> we use to build against is the same as the perl we need to run the build
... I think you meant "TAP tests" here ? --------------------------- ^^^^^
> with. On Msys that's emphatically not true.
Do we have/need any explicit references to the test version of "perl",
or is "prove" a sufficient API?
regards, tom lane
Re: [COMMITTERS] pgsql: Record full paths of programs sought by"configure".
From
Andrew Dunstan
Date:
On 08/07/2017 04:07 PM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 08/07/2017 03:36 PM, Tom Lane wrote: >>> My goodness, that's ugly. Is it really better than injecting >>> "PROVE=prove"? (I'd suggest saying that to configure, not make, >>> so that the configure log bears some resemblance to what you >>> want done.) >> This is what we had to do BEFORE the change in this commit. Now it's no >> longer sufficient. > Sorry, I was imprecise. What I'm suggesting is that you drop the > runtime PATH-foolery and instead put this in configure's environment: > > PROVE=$perlpathdir/prove > > Otherwise you're basically lying to configure about what you're going > to use, and that's always going to break eventually. Hmm, you're saying this should work now? OK, I'll try it when I get a minute to spare. > >> It would certainly be better if we could tell configure a path to prove >> and a path to the perl we need to test IPC::Run against. > Hm, yeah, the IPC::Run test would need to deal with this as well. > A PROVE_PERL environment variable is one way. Or maybe simpler, > just skip the probe for IPC::Run if PROVE has been specified > externally; assume the user knows what he's doing in that case. WFM > Are there any other gotchas in the build sequence? > Not that I can think of. > Do we have/need any explicit references to the test version of "perl", > or is "prove" a sufficient API? > > Probably sufficient. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 08/07/2017 04:07 PM, Tom Lane wrote:
>> Sorry, I was imprecise. What I'm suggesting is that you drop the
>> runtime PATH-foolery and instead put this in configure's environment:
>>
>> PROVE=$perlpathdir/prove
>>
>> Otherwise you're basically lying to configure about what you're going
>> to use, and that's always going to break eventually.
> Hmm, you're saying this should work now? OK, I'll try it when I get a
> minute to spare.
I'm pretty sure it's always worked, at least in the sense that you could
override what configure would put into Makefile.global that way. I'm not
quite clear on whether providing an exact path to "prove" there is enough
to fix your problem. If we have any places where we need to invoke the
corresponding version of "perl", then we have more things to fix.
>> Hm, yeah, the IPC::Run test would need to deal with this as well.
>> A PROVE_PERL environment variable is one way. Or maybe simpler,
>> just skip the probe for IPC::Run if PROVE has been specified
>> externally; assume the user knows what he's doing in that case.
> WFM
OK, I'll go make that happen.
regards, tom lane
Re: [COMMITTERS] pgsql: Record full paths of programs sought by"configure".
From
Andrew Dunstan
Date:
On 08/07/2017 04:20 PM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 08/07/2017 04:07 PM, Tom Lane wrote: >>> Sorry, I was imprecise. What I'm suggesting is that you drop the >>> runtime PATH-foolery and instead put this in configure's environment: >>> >>> PROVE=$perlpathdir/prove >>> >>> Otherwise you're basically lying to configure about what you're going >>> to use, and that's always going to break eventually. >> Hmm, you're saying this should work now? OK, I'll try it when I get a >> minute to spare. > I'm pretty sure it's always worked, at least in the sense that you could > override what configure would put into Makefile.global that way. I'm not > quite clear on whether providing an exact path to "prove" there is enough > to fix your problem. If we have any places where we need to invoke the > corresponding version of "perl", then we have more things to fix. I'm pretty sure that's all we need. But we can find out ;-) > >>> Hm, yeah, the IPC::Run test would need to deal with this as well. >>> A PROVE_PERL environment variable is one way. Or maybe simpler, >>> just skip the probe for IPC::Run if PROVE has been specified >>> externally; assume the user knows what he's doing in that case. >> WFM > OK, I'll go make that happen. > > OK, thanks. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services