Handle progress reports better

[Arthurm1]

Currently when a compile request is sent to the build server for multiple targets, the progress reports sent back to the client are only sent for the first build target.
This looks odd to the user as they won't see what project is being compiled. It looks especially odd in projects with very many build targets.

The issue is that Gradle compile progress reports are always associated with a task but not necessarily the one the BSP server has called. E.g. to compile a target, the build server might call :project:classes but Gradle runs a number of dependent tasks and it updates any progress listener with those task names, not the name of the original task that the build server called. Most of the time spent compiling the target is actually in the :project:compileJava task.

So the SourceSetsModelBuilder now queries all the task names that are associated with the sourceset. Then the progress listeners can work out what sourceset the progressEvent#taskPath is meant for and update that build target.

@jdneo I'm not sure if you'll think this is overkill but testing this by doing a full clean + compile on a large multi target project now shows which target is compiling at what time. It also satisfies the BSP spec as the updates will be marked with the correct target and all targets will get a Finished event even on compile failure so the client doesn't think compilation is taking forever.

[jdneo]

Is it possible to establish the mapping relationship between the project path and the build id?

It looks almost impossible to fetch all the available tasks given that Gradle is so flexible. But what if we can know that a project compilation is kicked off by checking the project path of the task name?

[Arthurm1]

@jdneo I'd hold off on this PR for now. I should have changed it to draft. I have an update to this which reports back errors/warnings but the Problems API is new and keeps changing so I'm holding back on it.

I've posted various issues I need resolved: gradle/gradle#28554 gradle/gradle#28230

The issue with lining up what progress report applies to what build target is tricky.

It can't be done when calling the task because of dependent tasks.
e.g. say you want to compile project server. That would also compile model as server depends on model. But when you're calling :server:classes you have no reference to the task :model:classes which will run except the new pre-populated SourceSet#getTaskNames

Hence this PR gets the tasks for the common compile tasks and maps them to build target Ids.
I don't think rootProject is available when the status message is received so it's not possible to iterate through the projects and lookup on each of them to see if the task belongs to them. I think this might be rather slow anyway

[Arthurm1]

@jdneo I've finished this PR off and it should fix your issue with the integration tests not running on your machine.

I have another PR with the actual Java errors/warnings being reported but I'm waiting on Gradle issues to be fixed.

This gives the user a good of what target is compiling and pushes all start/finish events down into Gradle so the TaskProgressReporter isn't needed anymore.

[jdneo]

I have another PR with the actual Java errors/warnings being reported but I'm waiting on Gradle issues to be fixed.

We have monthly sync with Gradle, maybe I can mention the issue to them. gradle/gradle#28554 and gradle/gradle#28230, right?

[Arthurm1]

@jdneo And gradle/gradle#28999.

The key thing for me is that problem aggregation events are being sent after the TaskFinish event which means I can't summarise the compiler error/warning count (gradle/gradle#28230 part 2). Well I can, but it would then mean all targets' compilation progress would finish at the same time which looks bad to the user.

A minor thing for me is the TaskPathLocation is missing from problem aggregation events. (gradle/gradle#28999 part 1)

If I can turn off problem aggregation (which I suggested in gradle/gradle#28999 part 2 and is my preferred fix) then that would fix both above issues.

The other issues I've raised are minor - they lead to slightly incorrect error locations but don't prevent errors/warnings being created.

Be great if you can nudge Gradle to address these or at least find out if they will or why they won't - I haven't had any reply to my issues so don't know if they're on a todo list somewhere or they're not going to be changed.

[jdneo]

do you think if it is necessary to include the failure summary to the message of the task finish event? Which can be a convenient way to display the errors of a certain task to users, like this:

It's not a blocker, since from the client side, it's still possible to establish the relationship between the error message and the build task by the task ids.

How do you handle the build failure message in Metals?

[Arthurm1]

The TaskFinishParams will still contain StatusCode#Error but, as you say, the message doesn't help the user zoom in on the issue. The full message does get sent back as a LogMessage with LogMessageParams#getType == MessageType#ERROR but a client might ignore that. One option would be to use onBuildShowMessage but I think that would be quite intrusive to the user as they'd get it popping up every time they saved with errors.

I think (but I'm not certain) that Metals just goes off the OnBuildPublishDiagnostics which currently aren't sent.

I have a small draft PR for producing diagnostics which I'll submit after you're happy with this one. But it only works on Gradle >=8.7 after the Problems API was introduced.

I've pushed some more changes where I've added a new testProject/fail-compilation which, as named, fails compilation and I've added integrity tests for it so we can see the way errors come back through log and finish messages.

I'm not really sure how this project will be able to handle reporting errors/warnings for Gradle <8.7. Intellij seems to parse the stdout for java errors but that doesn't work for other languages e.g. Scala as the format is different. Do you want to get diagnostics working for older versions of Gradle? I'd probably just try and encourage users to upgrade. Do you know of other projects besides Metals that are looking to use this project as a BSP server?

[jdneo]

One option would be to use onBuildShowMessage but I think that would be quite intrusive to the user as they'd get it popping up every time they saved with errors.

Agree. onBuildShowMessage will be annoying.

Do you want to get diagnostics working for older versions of Gradle?

At least, adopting the Gradle new problem API is a right way to approach. As for the older project, I think there is no need to support diagnostics right now. Unless it's frequently asked by community. So I'm fine to only support diagnostics for Gradle 8.7+.

Do you know of other projects besides Metals that are looking to use this project as a BSP server?

AFAIK, right now only vscode java and Metals use this build server.

[jdneo]

LGTM.

[jdneo]

Thank you @Arthurm1!