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: add a CMake based build for Windows #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Contributor

Given that swift-package-manager is not currently ready for Windows,
this addition makes it possible to build for Windows.

[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.]

@swift-server-bot
Copy link

Can one of the admins verify this patch?

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

@compnerd couldn't we generate the CMake files from SwiftPM? In SwiftNIO for example we generate the cocoapods using a script: https://github.com/apple/swift-nio/blob/master/scripts/build_podspecs.sh

If we just add the hard-coded CMake files, then we have two sources of truth. Then folks probably want to add Cocoapods files too and we'd have three. I think we should have one source of truth which is Package.swift and generate everything from there using a script. SwiftPM can output a stable .json description of the build.

@compnerd
Copy link
Contributor Author

@weissi - I don't think that package.swift has the information to really directly generate the CMake (its missing certain bits including list of sources, etc). s-p-m doesn't run on Windows really, so, this would need to be committed ahead of time.

I think it may be possible to enhance CMake to generate Package.swift ....

Either way, in the mean time, I think that this would block the use of swift-log on Windows :-(

Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

LGTM in general, but somewhat nervous to merge a changes which have no (windows) CI validation 🤔

Given that swift-package-manager is not currently ready for Windows,
this addition makes it possible to build for Windows.
@tomerd
Copy link
Member

tomerd commented May 5, 2020

thanks for this @compnerd. I really like @weissi idea of generating these from the output of swift package dump-package, if can develop a script that does that, we could then have it added to the other swift-server projects until swift-pm gets windows support. wdyt?

@compnerd
Copy link
Contributor Author

compnerd commented May 5, 2020

I don’t think that the script would be trivial to write. Since s-p-m doesn’t work on windows though, not sure how much I can help. But, if you can write such a script, sure, having the output of it checked in would be fine (remember that s-p-m doesn’t run on Windows, so you cannot run the script on Windows, so the make would still need to be committed)

@weissi
Copy link
Member

weissi commented May 5, 2020

@compnerd it just needs to work on Linux so the CI can verify that the checked in version is equal to the generated version. Happy to check it in (like the generated XCTest thingies) if we have checking that it's accurate.

@ktoso
Copy link
Member

ktoso commented Jul 6, 2020

Hey there, we're planning to tag 1.3.0 soon -- not sure about status of this PR, what should we do here?

@tomerd
Copy link
Member

tomerd commented Jul 6, 2020

I suggest we continue with release, and let this bake some more in the context of automating the generation of the CMake script or fixing SwiftPM to work on Windows

@ktoso ktoso added the stashed label Sep 3, 2020
@ktoso ktoso changed the base branch from master to main September 3, 2020 12:35
@FranzBusch FranzBusch removed the stashed label Sep 6, 2024
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.

6 participants