pgsql: Use better comment marker in Autoconf input

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

pgsql: Use better comment marker in Autoconf input

Peter Eisentraut-3
Use better comment marker in Autoconf input

The comment marker "#" is copied to the output, so it's only
appropriate for comments that make sense in the shell output.  For
comments about the Autoconf language, "dnl" should be used.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/4446565d36ac4482282146a9ab35068f18dff4fd

Modified Files
--------------
config/llvm.m4     |  2 +-
config/programs.m4 |  2 +-
configure          |  9 +--------
configure.in       | 12 ++++++------
4 files changed, 9 insertions(+), 16 deletions(-)

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Andres Freund
Hi,

On 2019-02-09 14:57:27 +0000, Peter Eisentraut wrote:
> Use better comment marker in Autoconf input
>
> The comment marker "#" is copied to the output, so it's only
> appropriate for comments that make sense in the shell output.  For
> comments about the Autoconf language, "dnl" should be used.

Weirdly enough this might have had some sideeffects. According to
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=elver&dt=2019-02-09%2014%3A58%3A09
this is where elver started to fail. I'd for a minute just put this down
to an independent upgrade on the animal, but the other branches still
build fine.   I'm somewhat confused.

Greetings,

Andres Freund

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Tom Lane-2
Andres Freund <[hidden email]> writes:
> On 2019-02-09 14:57:27 +0000, Peter Eisentraut wrote:
>> Use better comment marker in Autoconf input

> Weirdly enough this might have had some sideeffects. According to
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=elver&dt=2019-02-09%2014%3A58%3A09
> this is where elver started to fail.

Yeah, I noticed that coincidence too.

> I'd for a minute just put this down
> to an independent upgrade on the animal, but the other branches still
> build fine.

The error seems to be a library version mismatch, which sure looks
like a busted software upgrade.  But you have a darn good point:
if that's the problem, why didn't v11 break too?

> I'm somewhat confused.

Me too, now.  I stared closely at the configure shell script
changes, and they certainly seem to be innocuous.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Andres Freund
Hi,

On 2019-02-10 01:45:58 -0500, Tom Lane wrote:
> Andres Freund <[hidden email]> writes:
> > On 2019-02-09 14:57:27 +0000, Peter Eisentraut wrote:
> > I'd for a minute just put this down
> > to an independent upgrade on the animal, but the other branches still
> > build fine.
>
> The error seems to be a library version mismatch, which sure looks
> like a busted software upgrade.  But you have a darn good point:
> if that's the problem, why didn't v11 break too?

After a few coffees it's clear why: In v11, we have jit=0. As LLVM is
loaded on-demand, we'll not see errors in v11.

Greetings,

Andres Freund

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Thomas Munro
On Sun, 10 Feb 2019 at 19:22, Andres Freund <[hidden email]> wrote:

> On 2019-02-10 01:45:58 -0500, Tom Lane wrote:
> > Andres Freund <[hidden email]> writes:
> > > On 2019-02-09 14:57:27 +0000, Peter Eisentraut wrote:
> > > I'd for a minute just put this down
> > > to an independent upgrade on the animal, but the other branches still
> > > build fine.
> >
> > The error seems to be a library version mismatch, which sure looks
> > like a busted software upgrade.  But you have a darn good point:
> > if that's the problem, why didn't v11 break too?
>
> After a few coffees it's clear why: In v11, we have jit=0. As LLVM is
> loaded on-demand, we'll not see errors in v11.

It was using CC="ccache cc" for C, but no explicit CXX.  Somehow our
configure script flipped from finding c++ (the system C++ compiler,
which is Clang) to finding g++ (an old GCC port I had installed) after
that commit, and the resulting .so didn't work too well.  I did not
upgrade or change anything at that time.  I must admit that commit of
Peter's looks fairly innocent though.  Is it possible that an earlier
change caused it, but this change caused the "accache" to be cleared?
That accache seems a bit flimsy, I've had to rm it before.  Anyway,
now that I've explicitly set CXX="ccache c++" and nuked the accache,
elver is green again.

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Tom Lane-2
Thomas Munro <[hidden email]> writes:
> It was using CC="ccache cc" for C, but no explicit CXX.  Somehow our
> configure script flipped from finding c++ (the system C++ compiler,
> which is Clang) to finding g++ (an old GCC port I had installed) after
> that commit, and the resulting .so didn't work too well.  I did not
> upgrade or change anything at that time.  I must admit that commit of
> Peter's looks fairly innocent though.  Is it possible that an earlier
> change caused it, but this change caused the "accache" to be cleared?

Yeah, I think you're on the right track.  Had you made any changes in
that animal's configuration file between 21 Jan (the previous touch
of "configure" was in ee27584c4) and 9 Feb?  Or updated the platform's
compilers?

> That accache seems a bit flimsy, I've had to rm it before.

Yup, the buildfarm script only knows to nuke it if "configure" changes.
If you change the critter's buildfarm parameters, or the build
environment, that might require revisiting configure tests, but it won't
happen automatically.  I can't think of any really easy way to improve
that.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Thomas Munro-3
On Tue, Feb 12, 2019 at 3:46 AM Tom Lane <[hidden email]> wrote:

> Thomas Munro <[hidden email]> writes:
> > It was using CC="ccache cc" for C, but no explicit CXX.  Somehow our
> > configure script flipped from finding c++ (the system C++ compiler,
> > which is Clang) to finding g++ (an old GCC port I had installed) after
> > that commit, and the resulting .so didn't work too well.  I did not
> > upgrade or change anything at that time.  I must admit that commit of
> > Peter's looks fairly innocent though.  Is it possible that an earlier
> > change caused it, but this change caused the "accache" to be cleared?
>
> Yeah, I think you're on the right track.  Had you made any changes in
> that animal's configuration file between 21 Jan (the previous touch
> of "configure" was in ee27584c4) and 9 Feb?  Or updated the platform's
> compilers?

Oh, yeah, I remember now that I installed an old GCC a few weeks back
when testing some theory or other.  I am slightly surprised that we
prefer g++ to c++, so that merely installing it has this effect.
Anyway, I've now set the compiler explicitly.  Sorry for the noise.

--
Thomas Munro
http://www.enterprisedb.com

Reply | Threaded
Open this post in threaded view
|

Re: pgsql: Use better comment marker in Autoconf input

Andrew Dunstan-8
In reply to this post by Tom Lane-2
On Mon, Feb 11, 2019 at 11:46 AM Tom Lane <[hidden email]> wrote:
>
> Yup, the buildfarm script only knows to nuke it if "configure" changes.
> If you change the critter's buildfarm parameters, or the build
> environment, that might require revisiting configure tests, but it won't
> happen automatically.  I can't think of any really easy way to improve
> that.
>


Could stash a checksum of the config settings and obsolete the cache
if there was a mismatch. The danger is that we could get some false
positives, but the only downside to that is a one time extra amount of
time spent in configure.

cheers

andrew


--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services