Dear coreboot folks,
seeing the following patch
commit 527ceccbadc501bcdeb26b2b62e93f05dc8fcedb Author: Martin Roth martin.roth@se-eng.com Date: Sat Feb 23 15:32:11 2013 -0700
Persimmon: Fix warning, enable warnings as errors
http://review.coreboot.org/2494
it would be awesome if Gerrit could check Jenkins build log or Jenkins could notify Gerrit about newly introduced warnings by comparing the build log of each board. Reporting fixed warnings would also be nice.
Currently this is only possible if a board is warning free and -Werror is passed to the compiler by setting the Kconfig option WARNINGS_ARE_ERRORS to true so the the build aborts on new warnings.
Does somebody have experience with this? It would certainly be nice and should be doable automatically.
Thanks,
Paul
Am 2013-02-24 13:28, schrieb Paul Menzel:
Currently this is only possible if a board is warning free and -Werror is passed to the compiler by setting the Kconfig option WARNINGS_ARE_ERRORS to true so the the build aborts on new warnings.
And that's what we should aim for. The introduction of WARNINGS_ARE_ERRORS was for AGESA boards. Since it seems that we can touch AGESA now, there's no need to keep this special provision around for much longer.
One other AGESA provision we could kick out is the CAR globals exception: For all boards except AGESA, using global variables in romstage breaks the build unless they marked as CAR globals (in which case they're moved into CAR space). AGESA has its own mechanism for doing so (which is much more crude, you simply reserve some CAR space and hope it's enough. coreboot's implementation has byte-precision) that we could replace.
Does somebody have experience with this? It would certainly be nice and should be doable automatically.
The main problem is that jenkins' coreboot-gerrit task is non-linear: Introduce tons of warnings with one proposed change in gerrit, and the next change build-tested by jenkins has no problems to pass...
Patrick
Am Sonntag, den 24.02.2013, 13:34 +0100 schrieb Patrick Georgi:
Am 2013-02-24 13:28, schrieb Paul Menzel:
Currently this is only possible if a board is warning free and -Werror is passed to the compiler by setting the Kconfig option WARNINGS_ARE_ERRORS to true so the the build aborts on new warnings.
And that's what we should aim for. The introduction of WARNINGS_ARE_ERRORS was for AGESA boards. Since it seems that we can touch AGESA now, there's no need to keep this special provision around for much longer.
True. But as we are not there yet and I just found out that Jenkins already checks for fixed and introduced warnings [1], maybe this could be added nevertheless to the Jenkins report Gerrit displays.
One other AGESA provision we could kick out is the CAR globals exception: For all boards except AGESA, using global variables in romstage breaks the build unless they marked as CAR globals (in which case they're moved into CAR space). AGESA has its own mechanism for doing so (which is much more crude, you simply reserve some CAR space and hope it's enough. coreboot's implementation has byte-precision) that we could replace.
Does somebody have experience with this? It would certainly be nice and should be doable automatically.
The main problem is that jenkins' coreboot-gerrit task is non-linear: Introduce tons of warnings with one proposed change in gerrit, and the next change build-tested by jenkins has no problems to pass...
Sorry, I did not understand. Could you give an example? If every build is done from master and the patches with dependencies then this should not be a problem, should not it?
Thanks,
Paul
Am 24.02.2013 23:35, schrieb Paul Menzel:
True. But as we are not there yet and I just found out that Jenkins already checks for fixed and introduced warnings [1], maybe this could be added nevertheless to the Jenkins report Gerrit displays.
That data is mostly useful for the non-Gerrit builders, since history is linearized there.
The main problem is that jenkins' coreboot-gerrit task is non-linear: Introduce tons of warnings with one proposed change in gerrit, and the next change build-tested by jenkins has no problems to pass...
Sorry, I did not understand. Could you give an example? If every build is done from master and the patches with dependencies then this should not be a problem, should not it?
It is a problem for a couple of reasons:
1. Jenkins has no notion of "master + patches", but of commit ids 2. Jenkins' warnings analysis works relative to the prior build.
The first issue can be worked around: Just walk back from the commit id until you end up with a commit that is equal to, or a parent of master. However you still have to hope that any analysis data is still around - it's legal to push commits to gerrit that branched off against 2 year old upstream.
The second issue goes deeper:
Consider the recent build history of the coreboot-gerrit builder:
#4857 built 2502,2 (meaning: change 2502, patchset 2) #4856 built 2501,2 #4855 built 2501,1 #4854 built 2500,1 #4853 built 2497,2
This means that the statistics tell you how the warnings count changed from 2497,2 to 2500,1 to 2501,1 to 2501,2 to 2502,2. That's not very useful.
The only warning analysis that's somewhat useful is for #4856, since it can tell you if there are any changes to the warnings count compared to the earlier patchset of the same change.
Of course, a change like you propose would be useful, but I doubt it's possible within the current architecture of Jenkins. The plugin integrating Gerrit into Jenkins also requires changes to be able to push custom data into the build reports, and it might be necessary to expose such data within the Jenkins framework first.
I guess they'd accept patches, but I won't be the one writing them.
Patrick
-
Patrick Georgi -
Paul Menzel