Skip to content
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

Write output-file-map.json atomically #7406

Merged

Conversation

ahoppen
Copy link
Contributor

@ahoppen ahoppen commented Mar 19, 2024

There are two subtle changes in behavior here:

  • This call will fail if the file system doesn’t support atomic operations. I’m not sure if we need to support file systems that don’t allow atomic operations here.

rdar://124727242

@ahoppen
Copy link
Contributor Author

ahoppen commented Mar 19, 2024

@swift-ci Please test

@MaxDesiatov
Copy link
Member

  • This call will fail if the file system doesn’t support atomic operations. I’m not sure if we need to support file systems that don’t allow atomic operations here.

Does this apply to Windows? That needs to be verified before we can merge this as we shouldn't regress it.

@ahoppen ahoppen force-pushed the ahoppen/write-output-file-map-atomically branch from 7c927fa to 64610bc Compare March 19, 2024 12:44
@ahoppen
Copy link
Contributor Author

ahoppen commented Mar 19, 2024

@swift-ci Please test

There are two subtle changes in behavior here:
- This call will fail if the file system doesn’t support atomic operations. I’m not sure if we need to support file systems that don’t allow atomic operations here.

rdar://124727242
@ahoppen ahoppen force-pushed the ahoppen/write-output-file-map-atomically branch from 64610bc to f9d3e99 Compare March 19, 2024 13:22
@ahoppen
Copy link
Contributor Author

ahoppen commented Mar 19, 2024

@swift-ci Please test

@bnbarham
Copy link
Contributor

@swift-ci please test Windows platform

@ahoppen ahoppen merged commit adb8ea0 into apple:main Mar 20, 2024
5 checks passed
@ahoppen ahoppen deleted the ahoppen/write-output-file-map-atomically branch March 20, 2024 13:39
ahoppen added a commit that referenced this pull request Mar 25, 2024
* **Explanation**: When using sourcekit-lsp with VS Code it often
happens that sourcekitd is trying to read the `output-file-map.json`
while SwiftPM is writing it non-atomically. This results in a buffer in
sourcekitd that is not null-terminated. To fix this issue, generate the
output file map atomically.
* **Scope**: Generation of `output-file-map.json`
* **Risk**: This could fail if the `FileSystem` subclass in SwiftPM does
not support atomic write operations. But I think we only really care
about the local file system here, which does support atomic operations
* **Testing**: Tests continue passing. I will verify that this fixes the
sourcekitd crashes when a new toolchain is released with the fix
* **Issue**:  rdar://124727242
* **Reviewer**: @MaxDesiatov on
#7406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants