Skip to content

Conversation

@compnerd
Copy link
Member

@compnerd compnerd commented Aug 4, 2021

The C imports are important, particularly on Windows, where HANDLE is
not available without the platform SDK (WinSDK is roughly synonymous
to Darwin).

[One line description of your change]

Motivation:

[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]

Modifications:

[Describe the modifications you've done.]

Result:

[After your change, what will change.]

The C imports are important, particularly on Windows, where `HANDLE` is
not available without the platform SDK (`WinSDK` is roughly synonymous
to `Darwin`).
@compnerd
Copy link
Member Author

compnerd commented Aug 4, 2021

@swift-ci please test

Copy link
Contributor

@drexin drexin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. FWIW, in the next week or two I also plan to clean up the package manifest output so we can get rid of some of these platform-specifics (at least the writing out of the JSON on a separate pipe) by reusing some of the code that's used for package plugins. But of course this should be put back meanwhile; I only mention it to point out that we should be able to get rid of some of this again soonish.

@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

Started the smoke test, which is what's required for main.

@compnerd compnerd merged commit 0a17db0 into swiftlang:main Aug 4, 2021
@compnerd compnerd deleted the handle branch August 4, 2021 14:39
@compnerd
Copy link
Member Author

compnerd commented Aug 4, 2021

@abertelrud cool, though, please do test on windows before merging that. We really do need to get CI here (CC: @tomerd)

@tomerd
Copy link
Contributor

tomerd commented Aug 4, 2021

@compnerd @shahmishal I would love to be able to test Windows in CI, what need to be done to get there?

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.

4 participants