-
Notifications
You must be signed in to change notification settings - Fork 231
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
Add partial Windows support #779
Conversation
The `UMRP.deallocate` will bridge to `_aligned_free` which requires that the data be allocated with `_aligned_malloc`. This does not hold as the string is being duplicated via `strdup` and must be free'd explicitly with `free`. This happens to work on existing platforms as Darwin and Linux do not use a separate memory arena for aligned memory allocations.
Windows packages libclang however the library name is spelled differently. Adjust the library name for Windows support.
The Source Kit library on Windows is spelt `sourcekitdInProc.dll`. Add the alternate spelling to support Windows.
Add the Windows equivalent module name for the C library import that is needed to build this module.
Windows uses the environment variable `SDKROOT` to specify a default SDK to use for development. The user may override this by passing `-sdk ...` explicitly. Use the environment variable for the tests.
Ensure that we convert to the real file path on the disk when we serialise out the information. Additionally, explicitly standardize the path for Windows as it will fail to resolve drive substitutions otherwise.
The Windows platform uses msvcrt which has been replaced by the UCRT and is imported as `ucrt`. Alter the import for the C library for the `exit` function.
This was removed on 5.9 in favour of swift-syntax. Disable the test on 5.9 or newer toolchain.
SourceKit introduced the new attribute `reusingastcontext` on a cursor lookup. Add the ability to accommodate that by suffixing another version.
If there is a failure, the forced unwrapping will terminate the test suite. Prefer the `XCTUnwrap` to unwrap or fail instead.
Add a Windows specific expected results path. This allows us to exercise a greater number of tests on Windows than on Linux.
This is a terrible workaround to deal with the fact that `strings` is not really available on Windows. Without this it is not possible to pass tests.
This keeps `SourceKitTests.swift` to a more reasonable length.
Would you be open to dropping the topmost two commits from the initially committed changes? I'd like to see if we can do something about that which avoids a special case for Windows. At least a discussion around how to best handle that and if we cannot find anything, we can do this. |
Sorry which two commits? These commits are the ones I think we can easily land, I just have a few test failures to address in 3 CI jobs, 5 of them are passing now. |
Now that we are adding support for a 3rd platform, we should make the library wrapper generation more robust.
I'm ok with this for now. If you find a way to get this dynamically for Windows in the future, then we can replace this approach then, which I agree would certainly be nice. I'll merge this now that CI is passing again with the currently supported platforms. Then you can rebase #769 with a more focused set of changes. |
This cherry-picks the following commits from #769:
free
rather thanUMRP.deallocate
XCTUnwrap
over forced unwrappingThis isn't enough to get SourceKitten building and running on Windows, but rather are the building blocks that should be safe to merge as we work out the remaining issues.