tooling: report gradle wrapper download progress

[vladsoroka]

Any of the checked boxes below indicate that I took action:

• [+] Reviewed the Contribution Guidelines.
• [+] Signed the Gradle CLA.
• [+] Ensured that basic checks pass: ./gradlew quickCheck

Tooling api should provide progress information during the gradle distribution download

[adammurdoch]

Thanks for the pull request.

Download of the distribution happens in the tooling API client, so we don't need to, and should not, change any of the provider-side events or the protocol types.

Instead, the Download implementation that is passed to the wrapper should forward progress events directly to the internal wrapper over the ProgressLogger provided by the application.

This wrapper should not need to do any special processing to understand distribution download events.

[vladsoroka]

@adammurdoch Thanks for your comments!
The download implementation uses ProgressLogger via org.gradle.wrapper.DownloadProgressLogger#notify.
After such notification we get the progress event from the previous step in org.gradle.tooling.internal.consumer.connection.ProgressLoggingConsumerActionExecutor.ProgressListenerAdapter#progress method.
Previously the method had empty implementation, I can guess, the reason was that org.gradle.tooling.internal.protocol.ProgressListenerVersion1 doesn't have appropriate method to pass the event to the tooling client.
There is only onStart and onEnd event types supported (there is no method for interim progress event)
More over it uses (already) legacy tapi event listener (according to javadoc at LongRunningOperation#addProgressListener(org.gradle.tooling.ProgressListener)) via org.gradle.tooling.internal.consumer.parameters.ConsumerOperationParameters#legacyProgressListeners.

That's why I decided to use new tooling client event listener which is extensible and will not force to parse every progress event description string (see org.gradle.tooling.ProgressEvent#getDescription).
To find out which is the start of the event, end or interim results.

I have feeling that downloading of the distribution can be considered as a part of the build.
So I don't see why it can not be reported using tooling event listener?

If I miss something and we should use legacy tooling logging listener, could you advise how can I pass the status event from org.gradle.tooling.internal.consumer.connection.ProgressLoggingConsumerActionExecutor.ProgressListenerAdapter#progress method to org.gradle.tooling.ProgressListener#statusChanged?

[adammurdoch]

We're generating a new kind of event, so I think we should pass in a new kind of listener to Distribution.getToolingImplementationClasspath(), rather than try to jam this into the ProgressLoggerFactory that is passed in, and then parsing the progress messages to extract the data. This listener can simply be notified directly with the data as the download progresses.

This listener would do 2 things:

1. it would use the ProgressLoggerFactory to generate progress log messages, instead of DistributionFactory.ProgressReportingDownload doing this, which in turn will be forwarded to legacy ProgressListeners that the client registers.
2. it would generate StatusEvent instances that are forwarded to ConsumerOperationParameters.getBuildProgressListener(), which then forwards it to the ProgressListeners that the client registers.

[vladsoroka]

@adammurdoch yep, sounds reasonable, thank you! I'll work on it at the beginning of the next week.

[vladsoroka]

@adammurdoch I've accidentally pushed merge commit and other commits from master branch have appeared here. So, I had to reset my branch and force-pushed my changes using single commit, sorry for inconvenience.
Btw, for the purpose of this PR, InternalOperationProgress doesn't needed. Should I remove newly introduced classes in org.gradle.tooling.internal.protocol.events or it can be considered for use by other build events on provider side?

[adammurdoch]

Thanks, perhaps remove any new types that are not yet needed.

[adammurdoch]

We should remove the unused new type.

Also, it would be good to add an integration test for this. We could add some tests in ToolingApiRemoteIntegrationTest (or a new test class) to make sure the events are received by the application.

[adammurdoch]

Do we need this yet? Certainly at some point (hopefully soon) we want to send status events from the daemon, but for this PR the only status events are coming from the client side.

[vladsoroka]

I've added the test in ToolingApiRemoteIntegrationTest. Also I've removed internal implementation of the status events on daemon side.

Just to clarify, in order to avoid creation of a class duplicate for DefaultOperationDescriptor I had to move classes from subprojects/launcher/src/main/java/org/gradle/tooling/internal/provider/events to subprojects/tooling-api/src/main/java/org/gradle/tooling/internal/provider/events
Is it ok or you'd prefer to have those classes in launcher project?

[adammurdoch]

This looks good, I'll merge it soon. Thanks for making these changes. This is a long missing feature that will make the experience from the IDE much nicer for people.

[eriwen]

@adammurdoch I must be crazy, because I thought I saw this merged into release for 3.5. Is that correct?

[adammurdoch]

It is correct.