Thread: Bison 3.0 updates
Buildfarm member anchovy has been failing for the last couple of days,
evidently because its owner just couldn't wait to adopt bison 3.0,
which is all of 3 days old. The failures look like
cubeparse.y:42.1-13: warning: deprecated directive, use '%name-prefix' [-Wdeprecated]
%name-prefix="cube_yy"
^^^^^^^^^^^^^
(which looks like 3.0 isn't actually ready for prime time, but at least
it's only a warning)
cubeparse.c:163:5: error: conflicting types for 'cube_yyparse'
int cube_yyparse (void);
^
cubeparse.y:32:5: note: previous declaration of 'cube_yyparse' was here
int cube_yyparse(void *result);
^
A look in the Bison release notes explains this one: they stopped
supporting YYPARSE_PARAM, which contrib/cube and contrib/seg both use.
The recommended replacement is %parse-param, which is certainly a whole
lot cleaner: it lets you specify the datatype of the extra parser
parameter, instead of having it default to "void *". This option also
changes the signature of yyerror(), but that's not a problem.
At first I thought this was going to make us go through a tool upgrade
exercise, because I couldn't find %parse-param in the documentation for
bison 1.875, which is our oldest supported version. But further
research shows that %parse-param actually was introduced in 1.875,
they just forgot to document it :-(.
So I propose the attached patch, which I've verified still works with
1.875. I don't plan to install 3.0 just to test this, but I assume it's
OK there.
I'm thinking we should apply this to all supported branches, in case
somebody gets the idea to build an older branch with bleeding-edge
tools. Any objections?
regards, tom lane
Attachment
Tom,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Buildfarm member anchovy has been failing for the last couple of days,
[...]
> I'm thinking we should apply this to all supported branches, in case
> somebody gets the idea to build an older branch with bleeding-edge
> tools. Any objections?
Certainly looks cleaner to me, and if it works for older bison then it
seems reasonable to back-patch it.
However, I comment on this mainly because anchovy has had issues with
9.1 and older for some time, which looks like an issue with GCC 4.8.0.
Did you happen to resolve or identify what is happening there..?
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes: > However, I comment on this mainly because anchovy has had issues with > 9.1 and older for some time, which looks like an issue with GCC 4.8.0. > Did you happen to resolve or identify what is happening there..? Yeah, we know about that: http://www.postgresql.org/message-id/14242.1365200084@sss.pgh.pa.us The bottom line was: >> It looks like our choices are (1) teach configure to enable >> -fno-aggressive-loop-optimizations if the compiler recognizes it, >> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9. I am in favor of fixing the back branches via (1), because it's less work and much less likely to break third-party extensions. Some other people argued for (2), but I've not seen any patch emerge from them, and you can bet I'm not going to do it. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > However, I comment on this mainly because anchovy has had issues with > > 9.1 and older for some time, which looks like an issue with GCC 4.8.0. > > Did you happen to resolve or identify what is happening there..? > > Yeah, we know about that: > http://www.postgresql.org/message-id/14242.1365200084@sss.pgh.pa.us Ah, right, read the thread but didn't attach it to anchovy. > The bottom line was: > >> It looks like our choices are (1) teach configure to enable > >> -fno-aggressive-loop-optimizations if the compiler recognizes it, > >> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9. > > I am in favor of fixing the back branches via (1), because it's less > work and much less likely to break third-party extensions. Some other > people argued for (2), but I've not seen any patch emerge from them, > and you can bet I'm not going to do it. Yea, just passing -fno-aggressive-loop-optimizations seems like the safest and best option to me also.. Thanks, Stephen
On 2013-07-29 07:11:13 -0400, Stephen Frost wrote: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > > Stephen Frost <sfrost@snowman.net> writes: > > > However, I comment on this mainly because anchovy has had issues with > > > 9.1 and older for some time, which looks like an issue with GCC 4.8.0. > > > Did you happen to resolve or identify what is happening there..? > > > > Yeah, we know about that: > > http://www.postgresql.org/message-id/14242.1365200084@sss.pgh.pa.us > > Ah, right, read the thread but didn't attach it to anchovy. > > > The bottom line was: > > >> It looks like our choices are (1) teach configure to enable > > >> -fno-aggressive-loop-optimizations if the compiler recognizes it, > > >> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9. > > > > I am in favor of fixing the back branches via (1), because it's less > > work and much less likely to break third-party extensions. Some other > > people argued for (2), but I've not seen any patch emerge from them, > > and you can bet I'm not going to do it. > > Yea, just passing -fno-aggressive-loop-optimizations seems like the > safest and best option to me also.. I think we need to do both. There very well might be other optimizations made based on the unreachability information. Like concluding some conditional block isn't reachable. (1) would be back branches only, right? If others aggree I'll happily (ok, that's a blatant lie), provide patches if that actually helps $committer. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-07-29 07:11:13 -0400, Stephen Frost wrote:
>> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>>> The bottom line was:
>>> It looks like our choices are (1) teach configure to enable
>>> -fno-aggressive-loop-optimizations if the compiler recognizes it,
>>> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9.
>>>
>>> I am in favor of fixing the back branches via (1), because it's less
>>> work and much less likely to break third-party extensions. Some other
>>> people argued for (2), but I've not seen any patch emerge from them,
>>> and you can bet I'm not going to do it.
>> Yea, just passing -fno-aggressive-loop-optimizations seems like the
>> safest and best option to me also..
> I think we need to do both. There very well might be other optimizations
> made based on the unreachability information.
If we turn off the optimization, that will fix any other cases as well,
no? So why would we risk breaking third-party code by back-porting the
struct declaration changes?
regards, tom lane
On 2013-07-29 08:02:49 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-07-29 07:11:13 -0400, Stephen Frost wrote: > >> * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >>> The bottom line was: > >>> It looks like our choices are (1) teach configure to enable > >>> -fno-aggressive-loop-optimizations if the compiler recognizes it, > >>> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9. > >>> > >>> I am in favor of fixing the back branches via (1), because it's less > >>> work and much less likely to break third-party extensions. Some other > >>> people argued for (2), but I've not seen any patch emerge from them, > >>> and you can bet I'm not going to do it. > > >> Yea, just passing -fno-aggressive-loop-optimizations seems like the > >> safest and best option to me also.. > > > I think we need to do both. There very well might be other optimizations > > made based on the unreachability information. > > If we turn off the optimization, that will fix any other cases as well, > no? So why would we risk breaking third-party code by back-porting the > struct declaration changes? The -fno-agressive-loop thingie afaics only controls the optimization with regard to loopey constructs, not in general. I *think* there are independent hazards with general unreachability detection. Not sure whether they trigger at -O2 or only at -O3 though. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-07-29 08:02:49 -0400, Tom Lane wrote:
>> If we turn off the optimization, that will fix any other cases as well,
>> no? So why would we risk breaking third-party code by back-porting the
>> struct declaration changes?
> The -fno-agressive-loop thingie afaics only controls the optimization
> with regard to loopey constructs, not in general. I *think* there are
> independent hazards with general unreachability detection. Not sure
> whether they trigger at -O2 or only at -O3 though.
I'm not excited about breaking code in order to fix optimization bugs
that are purely hypothetical (and for which there's no particular reason
to believe that the proposed change would fix them anyway). If we were
seeing such things in the field it would be a different story.
regards, tom lane
On 2013-07-29 08:17:46 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-07-29 08:02:49 -0400, Tom Lane wrote: > >> If we turn off the optimization, that will fix any other cases as well, > >> no? So why would we risk breaking third-party code by back-porting the > >> struct declaration changes? > > > The -fno-agressive-loop thingie afaics only controls the optimization > > with regard to loopey constructs, not in general. I *think* there are > > independent hazards with general unreachability detection. Not sure > > whether they trigger at -O2 or only at -O3 though. > > I'm not excited about breaking code in order to fix optimization bugs > that are purely hypothetical (and for which there's no particular reason > to believe that the proposed change would fix them anyway). If we were > seeing such things in the field it would be a different story. Well, given the problems we're discussing here, it's not all that hypothetical. Obviously the compiler *does* have information it believes to proof some code to be unreachable. And we know that information comes from the way the catalog structs are declared. I don't really fancy finding more and more optimizations we didn't think about and disabling them one by one after annoying debugging sessions. Also, just about all code that would get broken by that change would be code that's already pretty much broken. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-07-29 08:17:46 -0400, Tom Lane wrote:
>> I'm not excited about breaking code in order to fix optimization bugs
>> that are purely hypothetical (and for which there's no particular reason
>> to believe that the proposed change would fix them anyway). If we were
>> seeing such things in the field it would be a different story.
> Well, given the problems we're discussing here, it's not all that
> hypothetical. Obviously the compiler *does* have information it believes
> to proof some code to be unreachable.
The known case here has nothing to do with unreachability IMO; it has
to do with concluding that a loop can be unrolled into exactly one
execution. But in any case, there is no point in arguing about what
hypothetical versions of gcc might do in hypothetical cases. We have
experimental evidence that -fno-aggressive-loop-optimizations fixes the
observed problem with gcc 4.8.0. So we can apply a one-line patch that
fixes the observed problem and seems 100% safe, or we can apply a very
large patch that might possibly fix problems that we don't know exist,
and might also break third-party code. Given the available evidence
the choice seems pretty clear to me. If you want to provide concrete
proof that there are additional optimization hazards then that would
change the tradeoff.
regards, tom lane
On 07/29/2013 01:05 AM, Tom Lane wrote: > Buildfarm member anchovy has been failing for the last couple of days, > evidently because its owner just couldn't wait to adopt bison 3.0, > which is all of 3 days old. The failures look like > Reminder to buildfarm animal owners: if you upgrade software please make sure your buildfarm members are still working. And if the upgrade involves the OS or the compiler, please use the udate_personality.pl script to update the server. cheers andrew
On 2013-07-29 08:44:56 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-07-29 08:17:46 -0400, Tom Lane wrote: > >> I'm not excited about breaking code in order to fix optimization bugs > >> that are purely hypothetical (and for which there's no particular reason > >> to believe that the proposed change would fix them anyway). If we were > >> seeing such things in the field it would be a different story. > > > Well, given the problems we're discussing here, it's not all that > > hypothetical. Obviously the compiler *does* have information it believes > > to proof some code to be unreachable. > > The known case here has nothing to do with unreachability IMO; it has > to do with concluding that a loop can be unrolled into exactly one > execution. If I understand the optimization correctly what it does is building a flow control model of a loop and concluding which its iterations can be reached assuming standard conforming code. In this case, since it knows (from looking at the underlying data structures) that the upper bound of the iteration has to be 1 it decides to stop there. If oidvector had values[2] it would make two iterations. > But in any case, there is no point in arguing about what > hypothetical versions of gcc might do in hypothetical cases. We have > experimental evidence that -fno-aggressive-loop-optimizations fixes the > observed problem with gcc 4.8.0. Well, it seems to me like we don't have particularly good experience with fixing compiler optimization issues by disabling some specific optimization. They tend to crop up again under the umbrella of another feature a compiler version or two later. FWIW cherry-picking bf66bfb4cd726b6ddab3fe2718648f65a7106149 and e58f3181fdaacc91d4cc1bd98a4a8ad7d286544c fixes the issue for me (After fixing trivial conflicts in the latter). 32 files changed, 98 insertions(+), 50 deletions(-) > So we can apply a one-line patch that > fixes the observed problem and seems 100% safe, or we can apply a very > large patch that might possibly fix problems that we don't know exist, > and might also break third-party code. Given the available evidence > the choice seems pretty clear to me. If you want to provide concrete > proof that there are additional optimization hazards then that would > change the tradeoff. No, don't really want to spend more time on this. We'll see whether it crops up again or not. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Hi, > On 07/29/2013 01:05 AM, Tom Lane wrote: >> Buildfarm member anchovy has been failing for the last couple of days, >> evidently because its owner just couldn't wait to adopt bison 3.0, >> which is all of 3 days old. Hmm? Anchovy is upgrading automatically to newest Arch Linux packages daily. I assumed that's a good thing -- the purpose of build farm is to test PostgreSQL in various different real-life environments? Arch Linux is one such environment that adopts new packages very quickly. If Arch users are unable to build PostgreSQL then surely it's good to be notified by the build farm before real users start reporting problems? I don't mean to sound reluctant, I'm open to suggestions, but please help me understand why this is bad. On Mon, Jul 29, 2013 at 4:59 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > Reminder to buildfarm animal owners: if you upgrade software please make > sure your buildfarm members are still working. If I had checked the machine's status manually after upgrading, the best course of action would be to report the incompatibility to PostgreSQL mailing lists. The end result is the same, in that sense, it was "working". Manual upgrades would only delay that reporting, not prevent it? I realize there have been problems with anchovy that are my own fault and I'm sorry about that. I check the buildfarm status every week or two. > And if the upgrade involves > the OS or the compiler, please use the udate_personality.pl script to update > the server. Is it OK to run update_personality.pl automatically every day from crontab? Regards, Marti
On 07/29/2013 10:28 AM, Marti Raudsepp wrote: > Hi, > >> On 07/29/2013 01:05 AM, Tom Lane wrote: >>> Buildfarm member anchovy has been failing for the last couple of days, >>> evidently because its owner just couldn't wait to adopt bison 3.0, >>> which is all of 3 days old. > Hmm? Anchovy is upgrading automatically to newest Arch Linux packages daily. > > I assumed that's a good thing -- the purpose of build farm is to test > PostgreSQL in various different real-life environments? Arch Linux is > one such environment that adopts new packages very quickly. If Arch > users are unable to build PostgreSQL then surely it's good to be > notified by the build farm before real users start reporting problems? > > I don't mean to sound reluctant, I'm open to suggestions, but please > help me understand why this is bad. The buildfarm is principally designed to detect when some change in the Postgres code breaks something. As such, the expectation is that the animals will provide a fairly stable platform. A totally moving target will present us with false positives, since the failure could be due to no action of ours at all. A machine that can auto-upgrade anything at any time is really not very fit for the purpose. What that would be testing is not if a change in Postgres code has broken something, but if the host packaging system has broken something. >> And if the upgrade involves >> the OS or the compiler, please use the udate_personality.pl script to update >> the server. > Is it OK to run update_personality.pl automatically every day from crontab? No. It is designed to support a fairly small number of changes. cheers andrew
Andres Freund <andres@2ndquadrant.com> writes:
> FWIW cherry-picking bf66bfb4cd726b6ddab3fe2718648f65a7106149 and
> e58f3181fdaacc91d4cc1bd98a4a8ad7d286544c fixes the issue for me
> (After fixing trivial conflicts in the latter).
I've already spent more time on this than I wanted to, but just for
the archives' sake: neither of those commits exist in the master
repo.
regards, tom lane
On 2013-07-29 10:46:41 -0400, Andrew Dunstan wrote: > > On 07/29/2013 10:28 AM, Marti Raudsepp wrote: > >Hi, > > > >>On 07/29/2013 01:05 AM, Tom Lane wrote: > >>>Buildfarm member anchovy has been failing for the last couple of days, > >>>evidently because its owner just couldn't wait to adopt bison 3.0, > >>>which is all of 3 days old. > >Hmm? Anchovy is upgrading automatically to newest Arch Linux packages daily. > > > >I assumed that's a good thing -- the purpose of build farm is to test > >PostgreSQL in various different real-life environments? Arch Linux is > >one such environment that adopts new packages very quickly. If Arch > >users are unable to build PostgreSQL then surely it's good to be > >notified by the build farm before real users start reporting problems? > > > >I don't mean to sound reluctant, I'm open to suggestions, but please > >help me understand why this is bad. > > > The buildfarm is principally designed to detect when some change in the > Postgres code breaks something. As such, the expectation is that the animals > will provide a fairly stable platform. A totally moving target will present > us with false positives, since the failure could be due to no action of ours > at all. > > A machine that can auto-upgrade anything at any time is really not very fit > for the purpose. What that would be testing is not if a change in Postgres > code has broken something, but if the host packaging system has broken > something. FWIW given the evidence here anchovy seems to do a good job. Both the gcc 4.8 and the bison 3.0 problems are things the project needs to know about and that need to get fixed. And none of the other animals seems to run anything really up2date. Also, afaik Arch has a rolling release model, so there isn't any other specific release that should be tested instead. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2013-07-29 10:52:10 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > FWIW cherry-picking bf66bfb4cd726b6ddab3fe2718648f65a7106149 and > > e58f3181fdaacc91d4cc1bd98a4a8ad7d286544c fixes the issue for me > > (After fixing trivial conflicts in the latter). > > I've already spent more time on this than I wanted to, but just for > the archives' sake: neither of those commits exist in the master > repo. Hrmpf, sorry for that. Posting the hashes *after* cherry-picking them ontop REL9_1_STABLE obviously isn't very helpful. The correct ones are: 8a3f745f160d8334ad978676828d3926ac949f43 8137f2c32322c624e0431fac1621e8e9315202f9 Those definitely are reachable via gitweb. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andrew Dunstan <andrew@dunslane.net> writes:
> On 07/29/2013 10:28 AM, Marti Raudsepp wrote:
>> Hmm? Anchovy is upgrading automatically to newest Arch Linux packages daily.
>> I assumed that's a good thing -- the purpose of build farm is to test
>> PostgreSQL in various different real-life environments? Arch Linux is
>> one such environment that adopts new packages very quickly. If Arch
>> users are unable to build PostgreSQL then surely it's good to be
>> notified by the build farm before real users start reporting problems?
> The buildfarm is principally designed to detect when some change in the
> Postgres code breaks something. As such, the expectation is that the
> animals will provide a fairly stable platform. A totally moving target
> will present us with false positives, since the failure could be due to
> no action of ours at all.
I can see both sides of this. It's definitely nice to get early warning
when toolchain changes create new problems for Postgres, but it's not
clear that the buildfarm is the best way to get such notifications.
The big problem here is that it might take a long time before we have
a suitable fix, and in the meantime that buildfarm member is basically
useless: it can't tell us anything new, and even if it tried, probably
nobody would notice because they'd not realize the failure cause changed.
We've had cases before where a buildfarm member that was failing for
some known reason was unable to detect a later-introduced bug, so this
isn't hypothetical. (And I'm not too thrilled that we've let the
back-branch failures on anchovy persist for months, though I suppose
that's not as bad as a breakage in HEAD.)
Ideally we'd not rotate toolchain updates into the buildfarm until
they'd been checked out in some other way. On the other hand, this
doesn't seem like a big enough problem to justify building some
different infrastructure for it, so I'm not seeing a practical way
to do better.
regards, tom lane
On Mon, Jul 29, 2013 at 5:53 PM, Andres Freund <andres@2ndquadrant.com> wrote: > Both the > gcc 4.8 and the bison 3.0 problems are things the project needs to know > about Perl 5.18 too: http://www.postgresql.org/message-id/2825.1370226552@sss.pgh.pa.us Marti
On 07/29/2013 11:05 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 07/29/2013 10:28 AM, Marti Raudsepp wrote: >>> Hmm? Anchovy is upgrading automatically to newest Arch Linux packages daily. >>> I assumed that's a good thing -- the purpose of build farm is to test >>> PostgreSQL in various different real-life environments? Arch Linux is >>> one such environment that adopts new packages very quickly. If Arch >>> users are unable to build PostgreSQL then surely it's good to be >>> notified by the build farm before real users start reporting problems? >> The buildfarm is principally designed to detect when some change in the >> Postgres code breaks something. As such, the expectation is that the >> animals will provide a fairly stable platform. A totally moving target >> will present us with false positives, since the failure could be due to >> no action of ours at all. > I can see both sides of this. It's definitely nice to get early warning > when toolchain changes create new problems for Postgres, but it's not > clear that the buildfarm is the best way to get such notifications. > > The big problem here is that it might take a long time before we have > a suitable fix, and in the meantime that buildfarm member is basically > useless: it can't tell us anything new, and even if it tried, probably > nobody would notice because they'd not realize the failure cause changed. > We've had cases before where a buildfarm member that was failing for > some known reason was unable to detect a later-introduced bug, so this > isn't hypothetical. (And I'm not too thrilled that we've let the > back-branch failures on anchovy persist for months, though I suppose > that's not as bad as a breakage in HEAD.) > > Ideally we'd not rotate toolchain updates into the buildfarm until > they'd been checked out in some other way. On the other hand, this > doesn't seem like a big enough problem to justify building some > different infrastructure for it, so I'm not seeing a practical way > to do better. > > There has to be something between "set in stone and never changes" and "auto-updates everything every 24 hours" that would suit us. I'm toying with the idea of a check_upgrade mode for the buildfarm client where it wouldn't do a git pull, but would report changes if the build result was different from the previous result. You'd run this immediately after pulling new changes into your OS. Other, brighter ideas welcome. cheers andrew
On Mon, Jul 29, 2013 at 9:15 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > There has to be something between "set in stone and never changes" and > "auto-updates everything every 24 hours" that would suit us. Well sure I could change the update frequency. But again, it seems like delaying the inevitable. > I'm toying with the idea of a check_upgrade mode for the buildfarm client > where it wouldn't do a git pull, but would report changes if the build > result was different from the previous result. You'd run this immediately > after pulling new changes into your OS. Other, brighter ideas welcome. What would be the right course of action if check_upgrade fails? Is it reasonable to expect buildfarm volunteers to downgrade the system and postpone until the problem is resolved? Or do you think the member should be removed from the farm until the build succeeds again? Sounds like worst of both worlds. Regards, Marti
On 07/29/2013 02:26 PM, Marti Raudsepp wrote: >> I'm toying with the idea of a check_upgrade mode for the buildfarm client >> where it wouldn't do a git pull, but would report changes if the build >> result was different from the previous result. You'd run this immediately >> after pulling new changes into your OS. Other, brighter ideas welcome. > What would be the right course of action if check_upgrade fails? Is it > reasonable to expect buildfarm volunteers to downgrade the system and > postpone until the problem is resolved? > > Or do you think the member should be removed from the farm until the > build succeeds again? Sounds like worst of both worlds. > Neither, I don't think you're understanding me at all. The idea is to have some way of saying "well, the code is the same, but the tools have changed." i.e. we want to be able to hold one of these variables constant. The buildfarm server knows how the run was invoked, and could distinguish runs done in this mode from other runs. cheers andrew
On Mon, Jul 29, 2013 at 4:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I can see both sides of this. It's definitely nice to get early warning > when toolchain changes create new problems for Postgres, but it's not > clear that the buildfarm is the best way to get such notifications. Perhaps something as simple as choosing a sufficiently oddball or frightening animal name? predator or alien? killerbee? bedbug? -- greg
On Mon, Jul 29, 2013 at 11:45 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > > On 07/29/2013 02:26 PM, Marti Raudsepp wrote: >>> >>> I'm toying with the idea of a check_upgrade mode for the buildfarm client >>> where it wouldn't do a git pull, but would report changes if the build >>> result was different from the previous result. You'd run this immediately >>> after pulling new changes into your OS. Other, brighter ideas welcome. >> >> What would be the right course of action if check_upgrade fails? Is it >> reasonable to expect buildfarm volunteers to downgrade the system and >> postpone until the problem is resolved? >> >> Or do you think the member should be removed from the farm until the >> build succeeds again? Sounds like worst of both worlds. >> > > > Neither, I don't think you're understanding me at all. The idea is to have > some way of saying "well, the code is the same, but the tools have changed." > i.e. we want to be able to hold one of these variables constant. Could it remove the need for manual intervention, by detecting if the tools have changed first, and then suppressing the git pull and adding the appropriate reporting flag, if they have? Cheers, Jeff
* Andrew Dunstan wrote: > I'm toying with the idea of a check_upgrade mode for the buildfarm > client where it wouldn't do a git pull, but would report changes if the > build result was different from the previous result. You'd run this > immediately after pulling new changes into your OS. Other, brighter > ideas welcome. Might it be possible for the buildfarm client to query the various tools (compiler, autotools, flex, bison, perl, shell, etc.) for their version numbers and report those too? If the config page said "since the last successful build, these commits have been added, and the compiler is now tomorrow's snapshot of gcc 4.8.4711", diagnosing a failed build could be easier. This could be done either through the usual -V, --version flags or by asking the host's packaging system, although there are probably too many of the latter to support. I would not want to have to implement this, but perhaps the client could also automatically drop back to the last known good commit if it gets a failed build immediately after a tool version change. -- Christian
On Mon, Jul 29, 2013 at 11:05:52AM -0400, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > On 07/29/2013 10:28 AM, Marti Raudsepp wrote: > >> Hmm? Anchovy is upgrading automatically to newest Arch Linux packages daily. > >> I assumed that's a good thing -- the purpose of build farm is to test > >> PostgreSQL in various different real-life environments? Arch Linux is > >> one such environment that adopts new packages very quickly. If Arch > >> users are unable to build PostgreSQL then surely it's good to be > >> notified by the build farm before real users start reporting problems? > > > The buildfarm is principally designed to detect when some change in the > > Postgres code breaks something. As such, the expectation is that the > > animals will provide a fairly stable platform. A totally moving target > > will present us with false positives, since the failure could be due to > > no action of ours at all. > > I can see both sides of this. It's definitely nice to get early warning > when toolchain changes create new problems for Postgres, but it's not > clear that the buildfarm is the best way to get such notifications. Early notification when a dependency's compatibility break affects PostgreSQL is a Very Good Thing. > The big problem here is that it might take a long time before we have > a suitable fix, and in the meantime that buildfarm member is basically > useless: it can't tell us anything new, and even if it tried, probably > nobody would notice because they'd not realize the failure cause changed. > We've had cases before where a buildfarm member that was failing for > some known reason was unable to detect a later-introduced bug, so this > isn't hypothetical. (And I'm not too thrilled that we've let the > back-branch failures on anchovy persist for months, though I suppose > that's not as bad as a breakage in HEAD.) True, but the solution, if any, is more buildfarm members. anchovy plays the important role of a sentinel for trouble with bleeding-edge dependency packages. Doing that with excellence inevitably makes it less useful for detecting other kinds of problems. > On the other hand, this > doesn't seem like a big enough problem to justify building some > different infrastructure for it, so I'm not seeing a practical way > to do better. Agreed. Let's stick an "Updates OS packages daily, regularly acquiring bleeding-edge upstream releases" note on the buildfarm status page, like we have for the CLOBBER_CACHE_ALWAYS members. That should be enough for folks to realize that a failure in this animal alone is more likely the fault of a new package version than of the latest commit. -- Noah Misch EnterpriseDB http://www.enterprisedb.com
On Tue, Jul 30, 2013 at 3:56 AM, Noah Misch <noah@leadboat.com> wrote: > Agreed. Let's stick an "Updates OS packages daily, regularly acquiring > bleeding-edge upstream releases" note on the buildfarm status page FWIW, I added "[updated daily]" to the OS version field. I haven't changed other configuration yet, will wait until there's a consensus. Regards, Marti
On 2013-07-29 08:02:49 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-07-29 07:11:13 -0400, Stephen Frost wrote: > >> * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >>> The bottom line was: > >>> It looks like our choices are (1) teach configure to enable > >>> -fno-aggressive-loop-optimizations if the compiler recognizes it, > >>> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9. > >>> > >>> I am in favor of fixing the back branches via (1), because it's less > >>> work and much less likely to break third-party extensions. Some other > >>> people argued for (2), but I've not seen any patch emerge from them, > >>> and you can bet I'm not going to do it. > > >> Yea, just passing -fno-aggressive-loop-optimizations seems like the > >> safest and best option to me also.. > > > I think we need to do both. There very well might be other optimizations > > made based on the unreachability information. > > If we turn off the optimization, that will fix any other cases as well, > no? So why would we risk breaking third-party code by back-porting the > struct declaration changes? This seems to be the agreed upon course of action, so I've prepared a patch including a preliminary commit message. I confirmed that it fixes the issue with gcc 4.8 and 9.1 for me. The patch needs to be applied to 9.1, 9.0 and 8.4. Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Attachment
Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-07-29 08:02:49 -0400, Tom Lane wrote:
>>> It looks like our choices are (1) teach configure to enable
>>> -fno-aggressive-loop-optimizations if the compiler recognizes it,
>>> or (2) back-port commit 8137f2c32322c624e0431fac1621e8e9315202f9.
>>>
>>> I am in favor of fixing the back branches via (1), because it's less
>>> work and much less likely to break third-party extensions.
> This seems to be the agreed upon course of action, so I've prepared a
> patch including a preliminary commit message. I confirmed that it fixes
> the issue with gcc 4.8 and 9.1 for me.
Committed --- thanks for doing the legwork to check it fixes the problem.
regards, tom lane
<div class="moz-cite-prefix">I'm getting some more of these, including some I thought you had fixed.<br /><br /> Bison
3.0.2on current head.<br /><br /><br /> <><br /> Writing postgres.bki<br /> Writing schemapg.h<br /> Writing
postgres.description<br/> Writing postgres.shdescription<br /> gram.y:172.1-13: warning: deprecated directive, use
‘%name-prefix’[-Wdeprecated]<br /> %name-prefix="base_yy"<br /> ^^^^^^^^^^^^^<br /> Writing fmgroids.h<br /> Writing
fmgrtab.c<br/> bootparse.y:96.1-13: warning: deprecated directive, use ‘%name-prefix’ [-Wdeprecated]<br />
%name-prefix="boot_yy"<br/> ^^^^^^^^^^^^^<br /> In file included from gram.y:14004:0:<br /> scan.c: In function
‘yy_try_NUL_trans’:<br/> scan.c:10188:23: warning: unused variable ‘yyg’ [-Wunused-variable]<br /> struct yyguts_t
*yyg = (struct yyguts_t*)yyscanner; /* This var may be unused depending upon options. */<br />
^<br/> repl_gram.y:43.1-13: warning: deprecated directive, use ‘%name-prefix’ [-Wdeprecated]<br />
%name-prefix="replication_yy"<br/> ^^^^^^^^^^^^^<br /> preproc.y:576.1-13: warning: deprecated directive, use
‘%name-prefix’[-Wdeprecated]<br /> %name-prefix="base_yy"<br /> ^^^^^^^^^^^^^<br /> pl_gram.y:113.1-13: warning:
deprecateddirective, use ‘%name-prefix’ [-Wdeprecated]<br /> %name-prefix="plpgsql_yy"<br /> ^^^^^^^^^^^^^<br />
cubeparse.y:42.1-13:warning: deprecated directive, use ‘%name-prefix’ [-Wdeprecated]<br /> %name-prefix="cube_yy"<br
/> ^^^^^^^^^^^^^<br /> segparse.y:45.1-13: warning: deprecated directive, use ‘%name-prefix’ [-Wdeprecated]<br />
%name-prefix="seg_yy"<br/> ^^^^^^^^^^^^^<br /> PostgreSQL, contrib, and documentation installation complete.<br />
</><br/><br /><br /><br /> On 07/29/2013 01:05 AM, Tom Lane wrote:<br /></div><blockquote
cite="mid:7367.1375074342@sss.pgh.pa.us"type="cite"><pre wrap="">Buildfarm member anchovy has been failing for the last
coupleof days,
evidently because its owner just couldn't wait to adopt bison 3.0,
which is all of 3 days old. The failures look like
cubeparse.y:42.1-13: warning: deprecated directive, use '%name-prefix'
[-Wdeprecated]%name-prefix="cube_yy"^^^^^^^^^^^^^
(which looks like 3.0 isn't actually ready for prime time, but at least
it's only a warning)
cubeparse.c:163:5: error: conflicting types for 'cube_yyparse'int cube_yyparse (void); ^
cubeparse.y:32:5: note: previous declaration of 'cube_yyparse' was hereint cube_yyparse(void *result); ^
A look in the Bison release notes explains this one: they stopped
supporting YYPARSE_PARAM, which contrib/cube and contrib/seg both use.
The recommended replacement is %parse-param, which is certainly a whole
lot cleaner: it lets you specify the datatype of the extra parser
parameter, instead of having it default to "void *". This option also
changes the signature of yyerror(), but that's not a problem.
At first I thought this was going to make us go through a tool upgrade
exercise, because I couldn't find %parse-param in the documentation for
bison 1.875, which is our oldest supported version. But further
research shows that %parse-param actually was introduced in 1.875,
they just forgot to document it :-(.
So I propose the attached patch, which I've verified still works with
1.875. I don't plan to install 3.0 just to test this, but I assume it's
OK there.
I'm thinking we should apply this to all supported branches, in case
somebody gets the idea to build an older branch with bleeding-edge
tools. Any objections?
regards, tom lane
</pre><br /><fieldset class="mimeAttachmentHeader"></fieldset><br /><pre wrap="">
</pre></blockquote><br /><br /><pre class="moz-signature" cols="72">--
Vik</pre>
Vik Fearing <vik.fearing@dalibo.com> writes:
> I'm getting some more of these, including some I thought you had fixed.
> Bison 3.0.2 on current head.
I didn't do anything to suppress those warnings:
> gram.y:172.1-13: warning: deprecated directive, use ‘%name-prefix’
> [-Wdeprecated]
> %name-prefix="base_yy"
> ^^^^^^^^^^^^^
because it's hard to see how that's anything but a bison bug.
regards, tom lane
On 05/21/2014 12:29 PM, Tom Lane wrote: > Vik Fearing <vik.fearing@dalibo.com> writes: >> I'm getting some more of these, including some I thought you had fixed. >> Bison 3.0.2 on current head. > I didn't do anything to suppress those warnings: > >> gram.y:172.1-13: warning: deprecated directive, use ‘%name-prefix’ >> [-Wdeprecated] >> %name-prefix="base_yy" >> ^^^^^^^^^^^^^ > because it's hard to see how that's anything but a bison bug. The wording is a little odd as it seems we're supposed to replace %name-prefix with... %name-prefix, but the docs say we're supposed to use api.prefix now instead. I'll just take your word for it as I've never done anything with bison before. -- Vik
Vik Fearing <vik.fearing@dalibo.com> writes:
> On 05/21/2014 12:29 PM, Tom Lane wrote:
>> I didn't do anything to suppress those warnings:
>>> gram.y:172.1-13: warning: deprecated directive, use ‘%name-prefix’
>>> [-Wdeprecated]
>>> %name-prefix="base_yy"
>>> ^^^^^^^^^^^^^
>> because it's hard to see how that's anything but a bison bug.
> The wording is a little odd as it seems we're supposed to replace
> %name-prefix with... %name-prefix, but the docs say we're supposed to
> use api.prefix now instead.
Oh, just a bad error message eh?
[ Reads docs ... ] AFAICT, they've deprecated this in favor of some
other syntax that they introduced in 2.6, which was released in July 2012.
That makes it a nonstarter for us. We're not going to break PG altogether
for most people in order to hide a warning message on the bleeding edge
version.
(For comparison, I've got bison 2.3 on my Mac and 2.4.1 on my RHEL6
machine, both of which are pretty up-to-date platforms as such things
go. Some of the buildfarm machines are still running 1.875.)
regards, tom lane
On 5/21/14, 12:29 PM, Tom Lane wrote: > Vik Fearing <vik.fearing@dalibo.com> writes: >> I'm getting some more of these, including some I thought you had fixed. >> Bison 3.0.2 on current head. > > I didn't do anything to suppress those warnings: > >> gram.y:172.1-13: warning: deprecated directive, use ‘%name-prefix’ >> [-Wdeprecated] >> %name-prefix="base_yy" >> ^^^^^^^^^^^^^ > > because it's hard to see how that's anything but a bison bug. What they want is that you use %name-prefix "base_yy" instead of %name-prefix="base_yy" That makes the warning go away. The %something=something syntax is not documented anywhere, so it looks like it worked more or less by accident. We don't use it anywhere else either (e.g. %expect 0, not %expect=0).
Peter Eisentraut <peter_e@gmx.net> writes:
> What they want is that you use
> %name-prefix "base_yy"
> instead of
> %name-prefix="base_yy"
> That makes the warning go away.
Oh really!?
> The %something=something syntax is not documented anywhere, so it looks
> like it worked more or less by accident. We don't use it anywhere else
> either (e.g. %expect 0, not %expect=0).
Hah. That's probably my fault, somewhere way back when. I'll fix it,
unless you're already on it.
regards, tom lane
On 5/28/14, 2:43 PM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> What they want is that you use >> %name-prefix "base_yy" >> instead of >> %name-prefix="base_yy" >> That makes the warning go away. > > Oh really!? > >> The %something=something syntax is not documented anywhere, so it looks >> like it worked more or less by accident. We don't use it anywhere else >> either (e.g. %expect 0, not %expect=0). > > Hah. That's probably my fault, somewhere way back when. I'll fix it, > unless you're already on it. Go ahead.