refactor how plugins call the build system when using the CLI #4185
Conversation
motivation: consolidate use of build operation construction which is important for cancellation support changes: * use the general purpose function to create a build operation * small refactoring of how build plan is extracted after the build is complete
|
@abertelrud this is a breakout PR for a cleanup required by #4173 |
| observabilityScope: self.swiftTool.observabilityScope | ||
| customBuildParameters: buildParameters, | ||
| customOutputStream: outputStream, | ||
| customLogLevel: logLevel |
There was a problem hiding this comment.
this is the key change - basically use the generic swiftTool.createBuildOperation which will record things with the cancellation utility which is less error prone than doing it manually
|
|
||
| // Get the build plan used | ||
| guard let buildPlan = buildOperation.buildPlan else { | ||
| throw InternalError("invalid state, buildPlan is undefined") |
There was a problem hiding this comment.
the above is just a small cleanup
There was a problem hiding this comment.
This is better than the !. Thanks for cleaning it up! In practice, should never happen but still.
| return false | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
we can also use observabilityScope.trap instead of this, but this preserved the previous behavior exactly
There was a problem hiding this comment.
Looks like there was only one call site for this. Do you think there will be more, or would it be better to keep this do/catch at the one callsite?
There was a problem hiding this comment.
normally you want to do something with the error, but in this case the code only seems to care about the end result.
tomerd
added
the
ready
|
@swift-ci please smoke test |
motivation: consolidate use of build operation construction which is important for cancellation support
changes: