Add a new CommandProgress API. #7350
Conversation
In addition to ExecuteProgress, this new interface allows builds and plugins to receive events when commands start and finish, including the State before and after each command. It also makes cancellation visible to clients by making the Cancelled type top-level.
|
Please take another look |
|
Do you have a demo build that you use locally use to demonstrate this feature, if so could we add a scripted test for this? |
18d12c2 to
8c96002
Compare
|
Scripted test added, PTAL |
|
Is there anything else I can do to move this forward? |
I think the next step is to create a 1.10.x branch and to redirect and merge this PR to it. I am a bit busy currently with the preparation of Scala Days but I will do it when coming back. |
There was a problem hiding this comment.
Since the new API exposes commands and tasks, should the old methods (eg. beforeWork, afterCompleted etc.) be deprecated and renamed to mention that they relate to tasks only (ie. beforeWork -> beforeTaskWork, afterAllCompleted -> afterAllTasksCompleted, etc.)?
I think it would be good to document roughly the order in which the methods will be called. For instance, it's not entirely clear to me what will be called first between beforeCommand and initial or between afterAllCompleted, stop and afterCommand (I guess afterCommand is last, but it would be good to have that fully specified?)
| * @param cmd The command string. | ||
| * @param result Left in case of an error. If the command cannot be parsed, it will be | ||
| * signalled as a ParseException with a detailed message. If the command | ||
| * was cancelled by the user, as sbt.Cancelled. |
There was a problem hiding this comment.
The comment doesn't describe the result in case of a successful command execution. Looking at the implementation, my understanding is that this is the state after the command has been executed. Maybe it's worth mentioning for the sake of clarity?
|
After discussion with @eed3si9n and given what happened to sbt 1.9.5, we will avoid patching 1.9.x with any new feature. So I moved the target branch from 1.9.x to 1.10.x. We plan to start the RC cycle of sbt 1.10.0 in about a month from now. Should we merge this PR now or wait for more documentation, as suggested by @Duhemm? |
|
@dragos asked me to take over this PR. I've added the small documentation fixes that I wanted. I believe this should be ready to merge. |
4da1feb to
77e22e9
Compare
Link to discussion thread: #7351
Motivation
Sbt defines the ExecuteProgress interface that offers fine-grained observability into task execution. However, users interact with sbt via commands, which in the most common case trigger one or more root tasks to execute. As an implementer of ExecuteProgress, it is currently impossible to infer what command is executing or if the current command was canceled by the user, or even if a command failed to parse. Moreover, a single command may trigger several “lifecycles” of execute progress (delimited by initial() and stop() calls), which makes it even harder to delimit logical workloads and correctly attribute task execution to user-facing actions. Examples are cross-compilation and sequential composition via semicolons (i.e. compile;test;publish).
Imagine a developer productivity team that would like to collect data about successful and failed builds across an organization. This data can then be used to diagnose errors (“20% of builds on the CI failed with the same error”, etc). Correctly determining the start, end, and failure status of each command is fundamental for such a service.
This proposal stems from our experience implementing Build Scans® for sbt, released as part of Gradle Enterprise (available for free at https://scans.gradle.com).
Goals
Non-goals
The proposal shouldn’t change how tasks and commands are executed nor the order and content of existing events.