Date: Wed, 22 May 2019 13:03:12 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: buildfarm-members@lists.postgresql.org,
	pgsql-hackers@lists.postgresql.org
Subject: Re: FullTransactionId changes are causing portability issues
Message-ID: <20190522200312.kmh6qv3jbms7dtpx@alap3.anarazel.de>
References: <27054.1558533367@sss.pgh.pa.us>
 <6039.1558536488@sss.pgh.pa.us>
 <11261.1558554950@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <11261.1558554950@sss.pgh.pa.us>
Precedence: bulk

Hi,

On 2019-05-22 15:55:50 -0400, Tom Lane wrote:
> I wrote:
> >> Our Solaris packager reports that 12beta1 is failing to build for him
> >> on some Solaris variants:
> >>> The link failure is:
> >>> Undefined			first referenced
> >>> symbol  			    in file
> >>> ReadNextFullTransactionId           pg_checksums.o
> 
> > On looking closer, the fix is simple and matches what we've done
> > elsewhere: transam.h needs to have "#ifndef FRONTEND" to protect
> > its static inline function from being compiled into frontend code.
> 
> > So the disturbing thing here is that we no longer have any active
> > buildfarm members that can build HEAD but have the won't-elide-
> > unused-static-functions problem.  Clearly we'd better close that
> > gap somehow ... anyone have an idea about how to test it better?

I'm somewhat inclined to just declare that people using such old
compilers ought to just use something newer. Having to work around
broken compilers that are so old that we don't even have a buildfarm
animal actually exposing that behaviour, seems like wasted effort. IMO
it'd make sense to just treat this as part of the requirements for a C99
compiler.


> Ah-hah --- some study of the gcc manual finds that modern versions
> of gcc have
> 
> `-fkeep-inline-functions'
>      In C, emit `static' functions that are declared `inline' into the
>      object file, even if the function has been inlined into all of its
>      callers.  This switch does not affect functions using the `extern
>      inline' extension in GNU C89.  In C++, emit any and all inline
>      functions into the object file.
> 
> This seems to do exactly what we need to test for this problem.
> I've confirmed that with it turned on, a modern platform finds
> the ReadNextFullTransactionId problem with yesterday's sources,
> and that everything seems green as of HEAD.
> 
> So, we'd obviously not want to turn this on for normal builds,
> but could we get a buildfarm animal or two to use this switch?

I could easily add that to one of mine, if we decide to go for that.

Greetings,

Andres Freund