Community
Participate
Working Groups
Currently the BuildManager reports problems with severity of WARNING. Warnings are no longer surfaced in the UI, so we should increase this severity to ERROR.
Created attachment 132690 [details] Fix v01
Created attachment 132691 [details] Screen shot after patch This is the dialog from an exception in a builder when running with this patch. Before this patch there was nothing shown to the user at all because the severity was WARNING.
Kevin, how does that screenshot look?
Created attachment 132692 [details] Error with autobuild off This is the dialog when autobuild is turned off, and workspace build is invoked manually (Ctrl+B). I think the top-level "Build problems" message is extra noise and should be discarded.
(In reply to comment #3) > Kevin, how does that screenshot look? Close. I'd say that the "This is an exception message" should be indented one level. Technically this means that the status which says, "Errors running builder: x on project y" should be a MultiStatus and the caught exception status should be a child of it. The reasoning being that the exception occured as part of that build. Builders A builder An exception in that builder (In reply to comment #4) > Created an attachment (id=132692) [details] > Error with autobuild off > > This is the dialog when autobuild is turned off, and workspace build is invoked > manually (Ctrl+B). I think the top-level "Build problems" message is extra > noise and should be discarded. Agree.
I checked that the current behavior is not a regression. In 3.2 and 3.4, build problems are just logged and no dialog is shown.
(In reply to comment #6) > I checked that the current behavior is not a regression. In 3.2 and 3.4, build > problems are just logged and no dialog is shown. Thanks. That matches my expectation. It strikes me as a long standing serviceability bug. I'm just trying to understand if we thought it was a _good_ idea to only log them or if this has just been an oversight.
(In reply to comment #7) > I'm just trying to understand if we thought it was a _good_ idea to only log > them or if this has just been an oversight. We could consider to show build errors as SHOW but NO_PROMPT errors. This would mean that when a build error occurs, we would get this small error icon in the bottom right corner of the workbench window. If you agree, I would raise a bug for that. John, are you going to rebuild the statuses in the fix for the bug or another bug should be raised for that?
> I'm just trying to understand if we thought it was a _good_ idea to only log > them or if this has just been an oversight. My only concern is if a user gets into a state where a builder is throwing exceptions on every build, which would result in a dialog popping up on every source change (assuming autobuild). However, the current behaviour of silently logging build errors doesn't make much sense to me. If the user didn't know to be watching the log, serious errors could be going unnoticed. Overall I think it's more important to surface these errors in some way to the end user.
Created attachment 132955 [details] Fix V02 This fix does the following: - Change build severity to ERROR when unchecked exceptions are thrown by a builder - Remove duplication of exception message by folding exception and build failure message into a single status - Remove superfluous MultiStatus for the manual "build all" action
Created attachment 132961 [details] Error Icon For Builds (In reply to comment #9) > My only concern is if a user gets into a state where a builder is throwing > exceptions on every build, which would result in a dialog popping up on every > source change (assuming autobuild)... See my comment 8. We could just show the error icon like the one on the attached picture. However this would require some changes regarding Status Handling. For instance status handling constants would have to be available for core plug-ins too.
(In reply to comment #9) > My only concern is if a user gets into a state where a builder is throwing > exceptions on every build, which would result in a dialog popping up on every > source change (assuming autobuild). Well, the build results are worth poop, so some action is required on the part of the user (if nothing else but to ignore the build output). If it were me I'd either immediately try to find the source of the error, or turn off autobuild, or close the offending project. > However, the current behaviour of silently > logging build errors doesn't make much sense to me. If the user didn't know to > be watching the log, serious errors could be going unnoticed. Overall I think > it's more important to surface these errors in some way to the end user. My feeling exactly. (In reply to comment #11) > See my comment 8. We could just show the error icon like the one on the > attached picture. However this would require some changes regarding Status > Handling. For instance status handling constants would have to be available for > core plug-ins too. The problem I have with the little icon is that it's almost like we think that build errors are just a normal part of life, "oh, by the way yoohoo, you're stuff is corrupt". What I do think we need though is some better way to turn off offending builders. But as with all problems, identification is the first step, and I believe always showing the dialog with clear identification of the builder/project is that first step. People can turn off builders in the project properties dialog but most people don't know that. A really good status manager would offer to do this for you. This would be a follow on bug.
Fixed in HEAD.
In speaking with McQ his concern is that there may be builders which have been throwing exceptions but which were not causing problems. I find it hard to believe this combination exists (a builder which did not complete but that's ok), ... nonetheless he's recommending we don't do this aspect until early 3.6.
Ok.
Created attachment 133020 [details] Patch to revert Attaching the patch to revert just the severity change here, so it will be easy to create the inverse patch later.
At the core API level, I think a severity of ERROR is the right answer for the case where a builder throws an unchecked exception. The IResourceStatus.BUILD_FAILED status code also indicates that it is an error rather than a warning. However, I did some experiments, and found exceptions thrown by builders are not surfaced in the UI as far back as the Eclipse 3.0 release, so this is very long-standing behaviour. There is some risk here if there are applications out in the field with misbehaving builders, this will suddenly be in the end user's face on every build. So, although I think this is the right fix, I'm fine with waiting until early 3.6 to release it. I'd like to release this severity change back to ERROR early in 3.6. The decision on what kinds of errors should be surfaced with a dialog in the UI is a separate question that UI's should decide separately. I believe the API status code of IResourceStatus.BUILD_FAILED that we return, would allow a UI to make a policy decision on whether such failures should be notified to end users or not.
Makes sense to me.
Like the Canadian Senate, McQ provides sober second thought :) Waiting to 3.6 is I admit the prudent choice given the unknowns.
(In reply to comment #10) > Created an attachment (id=132955) [details] > Fix V02 > > This fix does the following: > > - Change build severity to ERROR when unchecked exceptions are thrown by a > builder > - Remove duplication of exception message by folding exception and build > failure message into a single status > - Remove superfluous MultiStatus for the manual "build all" action John, could we still release parts 2 and 3 of this patch? ie. > - Remove duplication of exception message by folding exception and build > failure message into a single status > - Remove superfluous MultiStatus for the manual "build all" action These seem unrelated to whether we trigger the status dialog to show and clarify the stack in the error log (as well as status dialog).
I only reverted the severity change from ERROR back to WARNING. All other changes are still released.
(In reply to comment #21) > I only reverted the severity change from ERROR back to WARNING. All other > changes are still released. Great thanks, I guess they're not in the build I took. Maybe we should make a new bug for the severity issue and close off this one to represent the other changes released.
Btw, I've created bug #274686 about status nesting for build errors, which is a follow on for work committed in this bug.
Fix released in HEAD.
