Обсуждение: Re: FullTransactionId changes are causing portability issues

Поиск
Список
Период
Сортировка

Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
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?

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?

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
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



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
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



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-22 15:55:50 -0400, Tom Lane wrote:
>>> 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.

TBH, I too supposed the requirement for this had gone away with the C99
move.  But according to the discussion today on -packagers, there are
still supported variants of Solaris that have compilers that speak C99
but don't have this behavior.  Per Bjorn's report:

>> The compiler used in Sun Studio 12u1, very old and and I can try to
>> upgrade and see if that helps.
> I tried Sun Studio 12u2 and then a more drastic upgrade to Developer
> Studio 12.5 but both had the same effect.

It doesn't sound like "use a newer compiler" is going to be a helpful
answer there.

(It is annoying that nobody is running such a platform in the buildfarm,
I agree.  But I don't have the resources to spin up a real-Solaris
buildfarm member.)

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-22 15:55:50 -0400, Tom Lane wrote:
>>> 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.

TBH, I too supposed the requirement for this had gone away with the C99
move.  But according to the discussion today on -packagers, there are
still supported variants of Solaris that have compilers that speak C99
but don't have this behavior.  Per Bjorn's report:

>> The compiler used in Sun Studio 12u1, very old and and I can try to
>> upgrade and see if that helps.
> I tried Sun Studio 12u2 and then a more drastic upgrade to Developer
> Studio 12.5 but both had the same effect.

It doesn't sound like "use a newer compiler" is going to be a helpful
answer there.

(It is annoying that nobody is running such a platform in the buildfarm,
I agree.  But I don't have the resources to spin up a real-Solaris
buildfarm member.)

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
Hi,

On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-22 15:55:50 -0400, Tom Lane wrote:
> Per Bjorn's report:
> >> The compiler used in Sun Studio 12u1, very old and and I can try to
> >> upgrade and see if that helps.
> > I tried Sun Studio 12u2 and then a more drastic upgrade to Developer
> > Studio 12.5 but both had the same effect.
> 
> It doesn't sound like "use a newer compiler" is going to be a helpful
> answer there.

Well, GCC is available on solaris, and IIRC not that hard to install
(isn't it just a 'pkg install gcc' or such?). Don't think we need to
invest a lot of energy fixing a compiler / OS combo that effectively
isn't developed further.

Greetings,

Andres Freund



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
Hi,

On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-22 15:55:50 -0400, Tom Lane wrote:
> Per Bjorn's report:
> >> The compiler used in Sun Studio 12u1, very old and and I can try to
> >> upgrade and see if that helps.
> > I tried Sun Studio 12u2 and then a more drastic upgrade to Developer
> > Studio 12.5 but both had the same effect.
> 
> It doesn't sound like "use a newer compiler" is going to be a helpful
> answer there.

Well, GCC is available on solaris, and IIRC not that hard to install
(isn't it just a 'pkg install gcc' or such?). Don't think we need to
invest a lot of energy fixing a compiler / OS combo that effectively
isn't developed further.

Greetings,

Andres Freund



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
>> It doesn't sound like "use a newer compiler" is going to be a helpful
>> answer there.

> Well, GCC is available on solaris, and IIRC not that hard to install
> (isn't it just a 'pkg install gcc' or such?). Don't think we need to
> invest a lot of energy fixing a compiler / OS combo that effectively
> isn't developed further.

I'm not really excited about adopting a position that PG will only
build on GCC and clones thereof.

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
>> It doesn't sound like "use a newer compiler" is going to be a helpful
>> answer there.

> Well, GCC is available on solaris, and IIRC not that hard to install
> (isn't it just a 'pkg install gcc' or such?). Don't think we need to
> invest a lot of energy fixing a compiler / OS combo that effectively
> isn't developed further.

I'm not really excited about adopting a position that PG will only
build on GCC and clones thereof.

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
Hi,

On 2019-05-23 13:46:15 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
> >> It doesn't sound like "use a newer compiler" is going to be a helpful
> >> answer there.
> 
> > Well, GCC is available on solaris, and IIRC not that hard to install
> > (isn't it just a 'pkg install gcc' or such?). Don't think we need to
> > invest a lot of energy fixing a compiler / OS combo that effectively
> > isn't developed further.
> 
> I'm not really excited about adopting a position that PG will only
> build on GCC and clones thereof.

That's not what I said though? Not supporting one compiler, on an OS
that's effectively not being developed anymore, with a pretty
indefensible behaviour, requiring not insignificant work by everyone,
isn't the same as standardizing on gcc. I mean, we obviously are going
to continue at the absolute very least gcc, llvm/clang and msvc.

Greetings,

Andres Freund



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
Hi,

On 2019-05-23 13:46:15 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
> >> It doesn't sound like "use a newer compiler" is going to be a helpful
> >> answer there.
> 
> > Well, GCC is available on solaris, and IIRC not that hard to install
> > (isn't it just a 'pkg install gcc' or such?). Don't think we need to
> > invest a lot of energy fixing a compiler / OS combo that effectively
> > isn't developed further.
> 
> I'm not really excited about adopting a position that PG will only
> build on GCC and clones thereof.

That's not what I said though? Not supporting one compiler, on an OS
that's effectively not being developed anymore, with a pretty
indefensible behaviour, requiring not insignificant work by everyone,
isn't the same as standardizing on gcc. I mean, we obviously are going
to continue at the absolute very least gcc, llvm/clang and msvc.

Greetings,

Andres Freund



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-23 13:46:15 -0400, Tom Lane wrote:
>> I'm not really excited about adopting a position that PG will only
>> build on GCC and clones thereof.

> That's not what I said though? Not supporting one compiler, on an OS
> that's effectively not being developed anymore, with a pretty
> indefensible behaviour, requiring not insignificant work by everyone,
> isn't the same as standardizing on gcc. I mean, we obviously are going
> to continue at the absolute very least gcc, llvm/clang and msvc.

I think you're vastly overstating the case for refusing support for this.
Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
--- it's certainly far less of a problem than the Microsoft-droppings
we've had to put in in so many places.  The only real issue in my mind
is the lack of buildfarm support for detecting that we need to do so.

Also relevant here is that you have no evidence for the assumption that
these old Solaris compilers are the only live platform with the problem.
Yeah, we wish our buildfarm covered everything of interest, but it does
not.  Maybe, if we get to beta2 without any additional reports of build
failures on beta1, that would be a bit of evidence that nobody else cares
--- but we have no such evidence right now.  We certainly can't assume
that any pre-v12 release provides evidence of that, because up till
I retired pademelon, it was forcing us to keep this case supported.

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-23 13:46:15 -0400, Tom Lane wrote:
>> I'm not really excited about adopting a position that PG will only
>> build on GCC and clones thereof.

> That's not what I said though? Not supporting one compiler, on an OS
> that's effectively not being developed anymore, with a pretty
> indefensible behaviour, requiring not insignificant work by everyone,
> isn't the same as standardizing on gcc. I mean, we obviously are going
> to continue at the absolute very least gcc, llvm/clang and msvc.

I think you're vastly overstating the case for refusing support for this.
Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
--- it's certainly far less of a problem than the Microsoft-droppings
we've had to put in in so many places.  The only real issue in my mind
is the lack of buildfarm support for detecting that we need to do so.

Also relevant here is that you have no evidence for the assumption that
these old Solaris compilers are the only live platform with the problem.
Yeah, we wish our buildfarm covered everything of interest, but it does
not.  Maybe, if we get to beta2 without any additional reports of build
failures on beta1, that would be a bit of evidence that nobody else cares
--- but we have no such evidence right now.  We certainly can't assume
that any pre-v12 release provides evidence of that, because up till
I retired pademelon, it was forcing us to keep this case supported.

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
Hi,

On 2019-05-23 14:05:19 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-23 13:46:15 -0400, Tom Lane wrote:
> >> I'm not really excited about adopting a position that PG will only
> >> build on GCC and clones thereof.
> 
> > That's not what I said though? Not supporting one compiler, on an OS
> > that's effectively not being developed anymore, with a pretty
> > indefensible behaviour, requiring not insignificant work by everyone,
> > isn't the same as standardizing on gcc. I mean, we obviously are going
> > to continue at the absolute very least gcc, llvm/clang and msvc.
> 
> I think you're vastly overstating the case for refusing support for this.
> Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
> --- it's certainly far less of a problem than the Microsoft-droppings
> we've had to put in in so many places.  The only real issue in my mind
> is the lack of buildfarm support for detecting that we need to do so.

Well, doing it for every single inline function is pretty annoying, just
from a bulkiness perspective. And figuring out exactly which inline
function needs this isn't easy without something that actually shows the
problem.


> Also relevant here is that you have no evidence for the assumption that
> these old Solaris compilers are the only live platform with the problem.
> Yeah, we wish our buildfarm covered everything of interest, but it does
> not.  Maybe, if we get to beta2 without any additional reports of build
> failures on beta1, that would be a bit of evidence that nobody else cares
> --- but we have no such evidence right now.  We certainly can't assume
> that any pre-v12 release provides evidence of that, because up till
> I retired pademelon, it was forcing us to keep this case supported.

I don't think I'm advocating for not fixing the issue we had for
solaris, for 12. I just don't think this a reasonable approach going
forward.

Greetings,

Andres Freund



Re: FullTransactionId changes are causing portability issues

От
Andres Freund
Дата:
Hi,

On 2019-05-23 14:05:19 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-23 13:46:15 -0400, Tom Lane wrote:
> >> I'm not really excited about adopting a position that PG will only
> >> build on GCC and clones thereof.
> 
> > That's not what I said though? Not supporting one compiler, on an OS
> > that's effectively not being developed anymore, with a pretty
> > indefensible behaviour, requiring not insignificant work by everyone,
> > isn't the same as standardizing on gcc. I mean, we obviously are going
> > to continue at the absolute very least gcc, llvm/clang and msvc.
> 
> I think you're vastly overstating the case for refusing support for this.
> Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
> --- it's certainly far less of a problem than the Microsoft-droppings
> we've had to put in in so many places.  The only real issue in my mind
> is the lack of buildfarm support for detecting that we need to do so.

Well, doing it for every single inline function is pretty annoying, just
from a bulkiness perspective. And figuring out exactly which inline
function needs this isn't easy without something that actually shows the
problem.


> Also relevant here is that you have no evidence for the assumption that
> these old Solaris compilers are the only live platform with the problem.
> Yeah, we wish our buildfarm covered everything of interest, but it does
> not.  Maybe, if we get to beta2 without any additional reports of build
> failures on beta1, that would be a bit of evidence that nobody else cares
> --- but we have no such evidence right now.  We certainly can't assume
> that any pre-v12 release provides evidence of that, because up till
> I retired pademelon, it was forcing us to keep this case supported.

I don't think I'm advocating for not fixing the issue we had for
solaris, for 12. I just don't think this a reasonable approach going
forward.

Greetings,

Andres Freund



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-23 14:05:19 -0400, Tom Lane wrote:
>> I think you're vastly overstating the case for refusing support for this.
>> Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
>> --- it's certainly far less of a problem than the Microsoft-droppings
>> we've had to put in in so many places.  The only real issue in my mind
>> is the lack of buildfarm support for detecting that we need to do so.

> Well, doing it for every single inline function is pretty annoying, just
> from a bulkiness perspective.

Oh, I certainly wasn't suggesting we do that.

> And figuring out exactly which inline
> function needs this isn't easy without something that actually shows the
> problem.

... which is why we need a buildfarm animal that shows the problem.
We had some, up until the C99 move.

If the only practical way to detect the issue were to run some ancient
platform or other, I'd tend to agree with you that it's not worth the
trouble.  But if we can spot it just by using -fkeep-inline-functions
on an animal or two, I think it's a reasonable thing to keep supporting
the case for a few years more.

            regards, tom lane



Re: FullTransactionId changes are causing portability issues

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-23 14:05:19 -0400, Tom Lane wrote:
>> I think you're vastly overstating the case for refusing support for this.
>> Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
>> --- it's certainly far less of a problem than the Microsoft-droppings
>> we've had to put in in so many places.  The only real issue in my mind
>> is the lack of buildfarm support for detecting that we need to do so.

> Well, doing it for every single inline function is pretty annoying, just
> from a bulkiness perspective.

Oh, I certainly wasn't suggesting we do that.

> And figuring out exactly which inline
> function needs this isn't easy without something that actually shows the
> problem.

... which is why we need a buildfarm animal that shows the problem.
We had some, up until the C99 move.

If the only practical way to detect the issue were to run some ancient
platform or other, I'd tend to agree with you that it's not worth the
trouble.  But if we can spot it just by using -fkeep-inline-functions
on an animal or two, I think it's a reasonable thing to keep supporting
the case for a few years more.

            regards, tom lane