Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

[#281] Improved interaction between Xtext builder and JDT #1156

Merged
merged 1 commit into from Aug 15, 2019
Merged

Conversation

szarnekow
Copy link
Contributor

@szarnekow szarnekow commented Aug 5, 2019

[#281] Improved interaction between Xtext builder and JDT

@szarnekow
Copy link
Contributor Author

@cdietrich What do you think about this approach?

@cdietrich cdietrich added this to the Release_2.19 milestone Aug 13, 2019
@szarnekow
Copy link
Contributor Author

Please take another look @cdietrich

@szarnekow szarnekow changed the title [#218] Improved interaction between Xtext builder and JDT [#281] Improved interaction between Xtext builder and JDT Aug 14, 2019
@szarnekow
Copy link
Contributor Author

I added a test. Please re-examine.

if (queuedBuildData.needRebuild(getProject())) {
needRebuild();
}
pollQueuedBuildData();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szarnekow this leads to a full build immediately following a project.build(clean)
is this intended?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have deltas for projects that are no related at
all.
but still the full build gets called for the project we called clean on

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problematic call is the one in clean.
unfortunately
the taskqueue ist nailed down so hard to exchange.
will try with subclassing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't follow yet.
There is a clean on some unrelated project. And then you see a full build here?

@@ -495,6 +516,8 @@ protected boolean isOpened(IResourceDelta delta) {
protected void clean(IProgressMonitor monitor) throws CoreException {
SubMonitor progress = SubMonitor.convert(monitor, 10);
try {
pollQueuedBuildData();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is the problematic call?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. there is an unrelated project with deltas (on generated code)
this call leads to a needsRebuild.
the rebuild detects that the project that was cleaned has no lastcurrentbuildstate
and thus does an immediate fullbuild on the cleaned build

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, some code / callgraph lead to unrelated projects being in the SimpleProjectDependencyGraph?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont see how simpleProjectdependencygraph is involved.
needsrebuild is globally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and org.eclipse.xtext.builder.impl.javasupport.JdtQueuedBuildData.doNeedRebuild(JavaBuilderState, Procedure1<? super UnconfirmedStructuralChangesDelta>)
does not care about any graphs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. The offending full-build trigger is not visible in this change set here but is 'legacy' from JdtQueuedBuildData.doNeedRebuild - we may need the simpleProjectdependencygraph and check, if the project without delta is even related to the "asking" project. Would that make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cant we simply skip the clean part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants