-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[build-script] Cross-compile sourcekit-lsp for arm64 #59046
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
Conversation
7fdbbf4
to
e88f0fb
Compare
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.
Makes sense, but some changes needed.
@@ -74,6 +74,10 @@ def get_dependencies(cls): | |||
swiftpm.SwiftPM, | |||
swiftsyntax.SwiftSyntax] | |||
|
|||
@classmethod | |||
def has_cross_compile_hosts(self, args): | |||
return args.cross_compile_hosts |
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.
Maybe pull this into the base class so all products can use it?
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.
Good idea 👍
helper_cmd += [cross_compile_host] | ||
elif product.is_cross_compile_target(host_target): | ||
helper_cmd += ['--cross-compile-hosts', host_target, | ||
'--skip-cmake-bootstrap'] |
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.
Please just copy the above lines here instead, as this is a SPM-specific flag.
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.
Thanks. I missed this.
2a323e6
to
7c5be0f
Compare
37dc138
to
498d4f7
Compare
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.
Made a minor suggestion, but LGTM.
def supports_cross_compilation(self): | ||
""" | ||
Whether the product can be built for a different architecture than the | ||
host by passing --cross-compile-host to it. |
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.
Why not just override has_cross_compile_hosts on IndexStoreDB?
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.
Because I didn’t think of that. Good idea 👍
Currently, when building an open source toolchain, SourceKit-LSP is only built for x86_64. Copy the cross-compilation config from swiftpm.py to also produce a fat sourcekit-lsp executable for both x86_64 and arm64. rdar://78039145
498d4f7
to
c1994b4
Compare
@swift-ci Please smoke test |
@swift-ci Please build toolchain |
Currently, when building an open source toolchain, SourceKit-LSP is only built for x86_64. Copy the cross-compilation config from swiftpm.py to also produce a fat sourcekit-lsp executable for both x86_64 and arm64.
rdar://78039145