Minimize dirtying of swift build relative to sourcekit-lsp invocations.
#6648
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ons. Replaces a couple of writeFileContents calls with writeIfChanged to avoid updating modification times, triggering unnecessary work in future builds. Also capitalize drive letters on Windows when writing resource_bundle_accessor.swift files since VSCode (and therefore vscode-swift) send lowercase drive letters to sourcekit-lsp, again dirtying the output tree unnecessarily. (vscode seems unlikely to change to use capital letters given e.g. microsoft/vscode#42159 (comment)) Also make unrelated changes to appease snapshot swiftc where AbsolutePath?.self is no longer valid Swift.
fischman-bcny
requested review from
abertelrud,
neonichu,
tomerd and
MaxDesiatov
as code owners
tomerd
reviewed
Jun 13, 2023
tomerd
reviewed
Jun 13, 2023
tomerd
reviewed
Jun 13, 2023
|
thanks @fischman-bcny, some follow up ideas and questions |
fischman-bcny
commented
Jun 13, 2023
There was a problem hiding this comment.
Thanks @tomerd for the (very!) quick turnaround; please see responses inline.
compnerd
reviewed
Jun 14, 2023
|
I think that this code is now split across: Going to close this off for now. Thanks @fischman-bcny for the investigation and the fix! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
On Windows, building on the command line using
swift buildand also using VSCode causessourcekit-lspto modify the.buildtree, making subsequentswift builds do nontrivial work instead of being null builds.With this PR in SwiftPM that the sourcekit-lsp is built against, this extra work is no longer done 🎉
Modifications:
writeFileContentscalls withwriteIfChangedto avoid updating modification times, triggering unnecessary work in future builds.resource_bundle_accessor.swiftfiles since VSCode (and therefore vscode-swift) send lowercase drive letters tosourcekit-lsp, again dirtying the output tree unnecessarily. (vscode seems unlikely to change to use capital letters given e.g. Windows drive letter case different between ${workspaceFolder} and integratedTerminal microsoft/vscode#42159 (comment))Result:
The latter build in the command-line below is near instant and does no work, as expected, instead of rebuilding a whole lot of stuff:
swift build && code . && sleep 30 && swift build