Community
Participate
Working Groups
As documented in the IncrementalProjectBuilder#build() method and in http://http://www.eclipse.org/articles/Article-Builders/builders.html (see section "Handling cancelation") a builder is expected to call forgetLastBuiltState() and throw an OperationCanceledException if the progress monitor requests the operation to be canceled. Currently the XtextBuilder will simply return when the user attempts to cancel the build. I think the ClusteringBuilderState should also be allowed to throw an OperationCanceledException exception which the Xtext Builder would catch and re-throw: try { ... } catch (OperationCanceledException e) { forgetLastBuiltState(); throw e; }
Created attachment 179301 [details] proposed patch Attached a proposed patch. Please review.
Knut, thanks for the patch. May builder participants throw an OperationCanceledException as well or should the calling component deal with this state? Could you please provide a testcase to illustrate that the next triggered build is a full build? Marked as Indigo+
Sebastian, I will document that the builder participants are also expected to throw OperationCanceledExceptions and update RegistryBuilderParticipant accordingly. Isn't the test case you propose more of a test for the Eclipse build manager instead of for the XtextBuilder? Also I don't quite see how you would go about implementing that test. Would the test override the XtextBuilder binding?
Knut, I'ld register a builder participant in the test that throws an operation canceld on the first run and logs the build kind on the second invocation. Creating and modifying a resource with the existing builder test infrastructure should be straight forward, shouldn't it?
OK, thanks. I'm sure you're right :-) I'll take a look at BuilderParticipantTest.
Created attachment 179355 [details] patch updated with test I've attached an updated patch containing a test case as requested.
Created attachment 179356 [details] updated patch now including test case Not quite up to speed with git diff etc... This patch now includes the test case!
Thanks for the updated patch. Besides this assertion: assertTrue(0 < participant.getInvocationCount()); the test looks good to me. You could get rid of it, if you add an waitForAutobuild in line 55 and reset the builder participant afterwards. Furtermore there seem to be some minor inconsistencies with the formatting (at least the diff in the web interface suggests this - see AbstractBuilderState#145/146). A reformat should help. Please feel free to apply the fix.
Thanks for the review. Do we still have an Eclipse formatter config somewhere?
the formatting settings are applied (and checked-in) on the project level.
I explicitly didn't reformat the entire sources before because it changes things around quite heavily. Do you typically run the formatter on selected regions only?
I usually try to keep the existing formatting as well. However in this case it seems to be inconsistent and that's worth to be fixed. I'd simply reformat the file and double check the result.
Fix pushed to master. Sorry about all the whitespace diffs...
The BuildScheduler calls the incremental project builder and has thus to cope with any OperationCanceledExceptions by returning Status.CANCEL_STATUS. Currently it logs an error and returns an ERROR status.
fix pushed to master
(In reply to comment #15) > fix pushed to master Knut, would you please be so kind and backport this one to the Helios_Maintenance branch for the SR2?
(In reply to comment #16) > Knut, would you please be so kind and backport this one to the > Helios_Maintenance branch for the SR2? Certainly. I will do that this week.
Fix backported to Helios_Maintenance.
*** Bug 343771 has been marked as a duplicate of this bug. ***
Closing all bugs that were set to RESOLVED before Neon.0
