Skip to content
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

build: split the SwiftDocCUtilitiesTest for Windows #608

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Jun 1, 2023

We need to replicate the target definition to exclude some files when building for Windows as they depend on NIO to create the PreviewServer. Rather than adding the files and conditionally removing the content, remove the files from being built.

@compnerd
Copy link
Member Author

compnerd commented Jun 1, 2023

@swift-ci please test

We need to replicate the target definition to exclude some files when
building for Windows as they depend on NIO to create the PreviewServer.
Rather than adding the files and conditionally removing the content,
remove thee files from being built.
@compnerd
Copy link
Member Author

compnerd commented Jun 6, 2023

@swift-ci please test

@compnerd compnerd merged commit eff7c33 into swiftlang:main Jun 6, 2023
@compnerd compnerd deleted the windows-test-port branch June 6, 2023 14:08
@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Jun 7, 2023

IMO, adding #if os check on Manifest file is not a good practice because it only indicates the host platform not the destination platform.

eg.

  1. We can't use #if os(iOS) to exclude some file because the host OS will be macOS
  2. Later when we support cross build in SwiftPM. eg. Build Linux product on Windows host, it will be a problem too.

In general, I think a conditional check for NIO's availability on the destination side is more acceptable for me.

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.

3 participants