eclipse / xtext-eclipse

xtext-eclipse
Eclipse Public License 2.0
49 stars 73 forks source link

Building twice because of new "touch on recovery" #1710

Closed Bananeweizen closed 3 years ago

Bananeweizen commented 3 years ago

1671 has led to the situation that projects are built twice in certain situations:

The technical background is as follows:

I believe the exceptions and deadlocks reported in the early versions of #1671 (when the touch was executed immediately instead of via job) were also caused by the touch happening when the auto build already built the same project.

The most simple fix would be to check again inside the touch job run() method whether touching is actually still needed. However, I did not find any good method to check whether a project has been built (via Xtext BuildManagerAccess). Any advice how that could be checked, or do you imagine another fix eventually?

cdietrich commented 3 years ago

cc @szarnekow @tivervac

cdietrich commented 3 years ago

maybe also andreys changes mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=549704 are related

cdietrich commented 3 years ago

with Xtext 2.26 nightly and eclipse 2021-09 nightly i see

Import

[Worker-6: Building] Building demo
[Worker-6: Building] Built demo in 2 ms
[Worker-2: Building] Building demo
[Worker-2: Building] indexing platform:/resource/demo/src/mydsl/MrsGrantsSecretCompartments.statemachine
[Worker-2: Building] Built demo in 152 ms
[Worker-2: Building] Building demo
[Worker-2: Building] Built demo in 0 ms

open

[Worker-10: Building] Building demo
[Worker-10: Building] indexing platform:/resource/demo/src/mydsl/MrsGrantsSecretCompartments.statemachine
[Worker-10: Building] Built demo in 100 ms
[Worker-10: Building] Building demo
[Worker-10: Building] Built demo in 0 ms

so there is one real full build only @Bananeweizen @rytina i am not sure if the reproduction steps are sufficient or if andreys fix actually fixed the problem

rytina commented 3 years ago

i am not sure if the reproduction steps are sufficient or if andreys fix actually fixed the problem

@cdietrich Please make sure to import the Xtext project from an archive file into a clean workspace how it is demonstrated with https://drive.google.com/file/d/1N6ax6sTQ7EuvSA5rL537lPddasYXIgUf/view?usp=drivesdk I can reproduce this bug with standard Eclipse 2021-06 where I installed org.eclipse.xtext.sdk.p2-repository-2.26.0-SNAPSHOT.zip using the default domain model example.

cdietrich commented 3 years ago

interestingly i can see it with the domain model example but i could not see it with a statemachine example

[Worker-1: Building] 6
[Worker-1: Building] Building dully
[Worker-1: Building] indexing platform:/resource/dully/src/dully.dmodel
[Worker-1: Building] Built dully in 3828 ms
[Worker-7: Building] 6
[Worker-7: Building] Building dully
[Worker-7: Building] indexing platform:/resource/dully/src/dully.dmodel
[Worker-7: Building] Built dully in 570 ms
[Worker-3: Building] 9
[Worker-3: Building] Building dully
[Worker-3: Building] Built dully in 3 ms

will check next if the problem is zip import

cdietrich commented 3 years ago

zip or not does not play a role dm builds twice, statemachine not.

cdietrich commented 3 years ago

with yet another mydsl i get

[Worker-1: Building] 6
[Worker-1: Building] Building myp
[Worker-1: Building] Built myp in 13 ms
[Worker-3: Building] 6
[Worker-3: Building] Building myp
[Worker-3: Building] indexing platform:/resource/myp/demo.mydsl1
[Worker-3: Building] Built myp in 835 ms

=> is build cancelled?!?

Bananeweizen commented 3 years ago

@cdietrich I'm not sure but I suspect that very small (and fast built) models might not trigger the bug simply because it requires some "timely overlap" from the auto build job and the xtext refresh job. As far as I remember, the auto build job itself uses a randomized schedule delay of around 1 second, therefore you probably need models of at least 1 or 2 seconds build time to trigger it. I haven't investigated when the "needsBuild" flag of the auto build is cleared, but maybe in your case the build from the refresh trigger cleared that flag before the auto build job was even scheduled.

szarnekow commented 3 years ago

@cdietrich Can you by chance take another look?