-
Notifications
You must be signed in to change notification settings - Fork 65
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
Make rust-binary-calls-swift-package
example support Linux
#297
Make rust-binary-calls-swift-package
example support Linux
#297
Conversation
} | ||
|
||
fn compile_swift() { | ||
let swift_package_dir = manifest_dir().join("swift-library"); | ||
|
||
let mut cmd = Command::new("swift"); | ||
|
||
cmd.current_dir(swift_package_dir) | ||
.arg("build") | ||
.args(&["-Xswiftc", "-static"]) |
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 removed this because the Package.swift already defines this as a static library.
// This fix is for macOS only | ||
#[cfg(target_os = "macos")] |
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 am not sure if this is also required for other Apple platforms?
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.
Not sure, but the line "cargo:rustc-link-search={}/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx/
suggests that it's macos
only.
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.
Hm, maybe it is problematic in cross-compilation scenarios?
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.
Looks good. Left some minor feedback.
There have been multiple issues/PRs lately about Linux support.
There's at least some level of demand for swift-bridge
to support Swift+Linux.
This PR would be helpful.
Hopefully you're still able to complete it after such a long delay. I can review your fixes quickly.
Thanks for working on this.
// This fix is for macOS only | ||
#[cfg(target_os = "macos")] |
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.
Not sure, but the line "cargo:rustc-link-search={}/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx/
suggests that it's macos
only.
} | ||
} | ||
|
||
fn get_swift_lib_path() -> Result<String, String> { |
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.
Is there a default path for the Swift library?
If so, instead of spawning a which
command can we just use the default path, and if it doesn't exist we can panic and tell the user to set a SWIFT_LIBRARY_PATH=...
environment variable that points to "explanation of what to point to here".
This way:
- we avoid spawning an extra command
- the example more clearly explains its dependencies
Right now this is a bit magical, whereas an example should be more explicit and help the user understand exactly what's happening.
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 adapted the code to use /usr/lib/swift/linux
as a default path, if not otherwise configured by a user via the SWIFT_LIBRARY_PATH
environment variable.
// This fix is for Linux only | ||
#[cfg(target_os = "linux")] | ||
{ | ||
// Without this there will be a lot of missing symbols when linking to the Swift library. |
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.
Can you give an example of one or a few of the errors?
This helps someone who has run into that error find this example when they search online.
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 added an example.
rust-binary-calls-swift-package
example run on Linux
rust-binary-calls-swift-package
example run on Linuxrust-binary-calls-swift-package
example support Linux
I had another look at this PR and got it working on a newer Swift version (most likely I still used 5.9 or 5.10 when submitting this PR). I was also quickly trying to get this built correctly using the static-linux-sdk, but I had no luck with that. I guess the real issue is that Swift Package Manager does not correctly support static linking for libraries, but only for executables, as this recent issue suggests: swiftlang/swift-package-manager#8198 |
Got it. Well, before this PR a user couldn't run on Linux at all, so this is an improvement. If people have problems with these instructions they can open issues and we can try to continue to make the example to work on more Swift versions. Thanks for landing your first contribution. People will appreciate this example. |
Closes #107.