New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix to call didFinishCommand
#6757
Conversation
@@ -81,7 +81,7 @@ extension XCBuildDelegate: XCBuildOutputParserDelegate { | |||
} | |||
case .taskComplete(let info): | |||
queue.async { | |||
self.buildSystem.delegate?.buildSystem(self.buildSystem, didStartCommand: BuildSystemCommand(name: "\(info.taskID)", description: info.result.rawValue)) | |||
self.buildSystem.delegate?.buildSystem(self.buildSystem, didFinishCommand: BuildSystemCommand(name: "\(info.taskID)", description: info.result.rawValue)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neonichu @abertelrud this seems correct on the surface, but not sure if there are some subtleties that required this less-intuitive usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this is because of how SwiftPM's logging works? Since we do log in didStartCommand
, maybe this was done to "fix" the log output when using XCBuild.
@swift-ci smoke test |
@swift-ci smoke test |
@swift-ci please test |
@swift-ci please test windows |
When
XCBuildDelegate
receives ataskComplete
message, it callsbuildSystem(:didStartCommand:)
ofBuildSystemDelegate
.However, it should probably call
buildSystem(:didFinishCommand:)
.Since there doesn't seem to be any concrete type conforming to
BuildSystemDelegate
, this change doesn't appear to have any impact on the behavior of existing code.