Skip to content

Conversation

JaroslavTulach
Copy link

Continuation of #2464 - once a build on unloadable Gradle project successfully finishes, then let's schedule a reload to bring the module to loadable state:

Ideally the fix disappears and one can invoke "Start Debugging".

@JaroslavTulach JaroslavTulach added this to the 12.2 milestone Oct 19, 2020
@JaroslavTulach JaroslavTulach self-assigned this Oct 19, 2020
Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Well, let's talk about this one.
The functionality you are looking for is there. Through it is provided by using actions, more properly ActionMapping-s. It is not the duty of the executor to re-evaluate the internal GradleProject structure, that is decided at around:

Also if you're using the built-in actions, make sure that the project is opened, that's the only way to set the NbGradleProjectImpl's AimedQuality property to FULL, which means it can turn to Gradle to fetch the information. (though if this patch is working, that means the project aims are higher than FALLBACK)

@JaroslavTulach
Copy link
Author

OK, I'll investigate the getReloadRule() and why it is not in action in my case.

@JaroslavTulach
Copy link
Author

The problem with the current approach is that the

     task.addTaskListener((Task t) -> {
                try {
                    OutputWriter out1 = task.getInputOutput().getOut();
                    boolean canReload = project.getLookup().lookup(BeforeReloadActionHook.class).beforeReload(action, outerCtx, task.result(), out1);
                    if (needReload && canReload) {
                        String[] reloadArgs = RunUtils.evaluateActionArgs(project, mapping.getName(), mapping.getReloadArgs(), outerCtx);
                        prj.reloadProject(true, maxQualily, reloadArgs);
                    }
                    project.getLookup().lookup(AfterBuildActionHook.class).afterAction(action, outerCtx, task.result(), out1);
                    for (AfterBuildActionHook l : context.lookupAll(AfterBuildActionHook.class)) {
                        l.afterAction(action, outerCtx, task.result(), out1);
                    }
                } finally {
                    task.getInputOutput().getOut().close();
                    task.getInputOutput().getErr().close();
                }
            });

code is invoke too late!

I'd like the reload of the project to happen immediately after the build is over. However the task listener is notified tens of seconds later.

@JaroslavTulach JaroslavTulach force-pushed the jtulach/RefreshGradleAfterSuccessfulBuild branch from 4a0080b to d7d4faf Compare October 20, 2020 03:19
@JaroslavTulach
Copy link
Author

d7d4faf comes with a solution that (tries to) honor the existing getReloadRule(). As soon as the build succeeds, the ExecutorTask wrapper listeners are notified and may immediately refresh the build state.

Btw. the refresh is also made synchronous and only then the build is claimed to be over.

Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Let's give it a go! Thank you!

@JaroslavTulach
Copy link
Author

Thanks Laszlo, however I have added few more changes meanwhile, so let's review again:

  • support for ActionProgress added to Gradle project, that's for @lkishalmi to review
  • java.lsp.server modified to invoke BUILD when the project seems misconfigured - a bit of CompletableFuture for @sdedic and @entlicher
  • Maven project changes to refresh its state before build is over to be consistent with Gradle.

With these changes running VSCode "Start Debugging" on broken Gradle (just create new project) and broken Maven (remove some .m2/repository entries) seems to work.

@JaroslavTulach JaroslavTulach changed the title Refresh unloadable Gradle project after successful build Refresh unloadable Gradle/Maven project after successful build Oct 20, 2020
Copy link
Contributor

@entlicher entlicher left a comment

Choose a reason for hiding this comment

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

The changes look fine to me. Thanks.

@JaroslavTulach
Copy link
Author

OK, let's try. Everything is green (except on java.hints batch), so I assume the change passes existing tests. Let's integrate.

@JaroslavTulach JaroslavTulach merged commit 5c8fd84 into apache:delivery Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants