-
Notifications
You must be signed in to change notification settings - Fork 216
Fix build on Windows #200
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
Fix build on Windows #200
Conversation
|
Updated this PR, now it depends on the changes in swiftlang/swift-tools-support-core#106. |
artemcm
left a comment
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.
This all looks good.
| let encoder = JSONEncoder() | ||
|
|
||
| #if os(Linux) || os(Android) | ||
| #if !canImport(ObjectiveC) |
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 does withoutEscapingSlashes require ObjectiveC?
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.
When this was written, withoutEscapingSlashes wasn't implemented in corelibs-foundation yet. It looks like it's since been added in swiftlang/swift-corelibs-foundation@c3fd6ec, so I think this conditional block can be removed entirely.
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 @owenv, turns out I was building with an outdated Foundation.
I'll drop that commit to keep the PR focused on Windows-related things.
|
@swift-ci please test |
|
@egorzhdan would you mind removing the SQLite3 dependency? That's not a true dependency for this project AFAICT, and its better to fix that in t-s-c rather than add that dependency here. (Or did I miss the import of SQLite in this project?) |
SwiftPM depends on SwiftDriver and SwiftOptions DLLs, so they should be installed into the toolchain bin directory
|
@compnerd ok, removed it. |
|
CC: @DougGregor |
|
lgtm, @DougGregor to approve |
|
LGTM, thank you! |
SwiftDrivertarget importsTSCUtility(which has a dependency onTSCBasic) but only linked againstTSCBasicSwiftDriver.dll&SwiftOptions.dll, so they should be installed into the toolchain bin directorystrsignalis not available