Re: FullTransactionId changes are causing portability issues
Andres Freund 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
Hi, On 2019-05-23 14:05:19 -0400, Tom Lane wrote: > Andres Freund 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 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
Hi, On 2019-05-23 13:46:15 -0400, Tom Lane wrote: > Andres Freund 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 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
Hi, On 2019-05-22 16:13:02 -0400, Tom Lane wrote: > Andres Freund 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 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
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 > >>> symbolin 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
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
I wrote: > Our Solaris packager reports that 12beta1 is failing to build for him > on some Solaris variants: >> The link failure is: >> --- >> Undefinedfirst referenced >> symbol in file >> ReadNextFullTransactionId pg_checksums.o >> ld: fatal: symbol referencing errors. No output written to pg_checksums >> --- 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? regards, tom lane
FullTransactionId changes are causing portability issues
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 > ld: fatal: symbol referencing errors. No output written to pg_checksums > --- > Now, ReadNextFullTransactionId() is implemented in > src/backend/access/transam/varsup.c but I cannot see varsop.o being > included in any of the libraries pg_checksum is linked against > (libpgcommon.a and libpgport.a). > When I check the pg_checksum.o I find that it references > ReadNextFullTransactionId on the platforms that fail but not where it > doesn't. The failed platforms are all sparc variants plus 64-bit x86 > on Solaris 11. > The compiler used in Sun Studio 12u1, very old and and I can try to > upgrade and see if that helps. > [ it didn't ] I'm a bit mystified why we did not see this problem in the buildfarm, especially since we have at least one critter (damselfly) running an OpenSolaris variant. Nonetheless, it sure looks like a "somebody was sloppy about frontend/backend separation" problem. Fix ideas anyone? I think we need to not only solve the immediate problem (which might just take an #ifndef FRONTEND somewhere) but also close the testing gap so we don't get blindsided like this again. regards, tom lane
