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

Make sure the auto build is triggered after a force kill #1671

Merged
merged 3 commits into from
May 25, 2021

Conversation

tivervac
Copy link
Contributor

@tivervac tivervac commented May 3, 2021

When an Xtext application is force killed, the Xtext will not have been
written (as it's written on shutdown). The applications thus forgets
the last built state during the next startup. The idea is that
the auto-builder will then pick up that the projects need to be rebuild.

However, for the latter we need to first touch the relevant projects.

When an Xtext application is force killed, the Xtext will not have been
written (as it's written on shutdown). The applications thus forgets
the last built state during the next startup. The idea is that
the auto-builder will then pick up that the projects need to be rebuild.

However, for the latter we need to first touch the relevant projects.

Signed-off-by: Titouan Vervack <titouan.vervack@sigasi.com>
@tivervac tivervac added the bug label May 3, 2021
@tivervac tivervac requested a review from szarnekow May 3, 2021 16:25
Signed-off-by: Titouan Vervack <titouan.vervack@sigasi.com>
@szarnekow
Copy link
Contributor

@cdietrich Do you recall any issues with using isTreeLocked for such a call? I know there is a small window of opportunity here that the tree will be locked immediately after the guard. Do you expect this to be a problem?

@cdietrich
Copy link
Member

cdietrich commented May 7, 2021

@szarnekow am not sure if this still can leads to bad situations like we have with the workspace (non)locks on startup with builder state loading and Storage2UriMapperJavaImpl and build

e.g. eclipse/xtext#2431 and eclipse/xtext#2408

Signed-off-by: Titouan Vervack <titouan.vervack@sigasi.com>
@tivervac tivervac merged commit 90596b4 into master May 25, 2021
@tivervac tivervac deleted the tv_build_after_kill branch May 25, 2021 12:36
* @since 2.26
*/
protected void touchProject(IProject project) {
if (!project.getWorkspace().isTreeLocked()) {
Copy link

@rytina rytina May 27, 2021

Choose a reason for hiding this comment

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

Thank you for this PR, it also fixed my issue #1676, but I recognized in the builds some exceptions due to a missing check in case for closed projects. When I add project.isAccessible() to that condition and before line 71 in the touch from the job, then our builds are green

Copy link
Member

Choose a reason for hiding this comment

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

@rytina could you provide a pr

Copy link
Member

Choose a reason for hiding this comment

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

or create an addtional issue? i also wonder how you can bypass the
BuildManagerAccess.findBuilder calling isAccessible in line 62

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

will try to run the stuff locally over the weekend.
then i will hopefully be able to provide a complete jstack

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

problem is reproducing locally.
undoing the change makes test green again.
it seems to "always" run in a job at least also helps.
will try to verify with jenkins/gh actions

Copy link
Member

Choose a reason for hiding this comment

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

cdietrich added a commit that referenced this pull request May 28, 2021
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
cdietrich added a commit that referenced this pull request May 28, 2021
…n job

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
cdietrich added a commit that referenced this pull request Jun 1, 2021
[#1671]always run in job to avoid deadlock and check accessibility in job
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

4 participants