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

Linux and Windows Support #3446

Merged
merged 37 commits into from
Jun 7, 2021
Merged

Linux and Windows Support #3446

merged 37 commits into from
Jun 7, 2021

Conversation

jshier
Copy link
Contributor

@jshier jshier commented Apr 28, 2021

Issue Link 🔗

#1935

Goals ⚽

This PR builds on #3098 and #3462 to add preliminary support for Alamofire on Linux and Windows now that Swift 5.4 greatly decreases the changes needed between the platforms.

Implementation Details 🚧

Functionally, this PR doesn't change implementation except where types are unavailable on non-Apple. This includes:

  • AFError support for ServerTrustFailureReason has been checked.
  • All Security framework type usage has been checked.
  • Various UT types from CoreServices were checked.
  • Access to various Info.plist values has been moved from CoreFoundation constants to raw Strings.
  • Missing APIs were checked.

TODO ✅

  • Enable URLAuthenticationChallenge for everything that's not ServerTrust.
    • Enabled, but doesn't work on Linux / Windows.
  • Replace getBoundStreams usage.
    • Needs solution.
  • Replace MIME type detection for MultipartFormData.
    • Needs solution.

Testing Details 🔍

Testing needs to be updated to run against Firewalk.

@jshier jshier mentioned this pull request Apr 28, 2021
9 tasks
@alex-taffe alex-taffe mentioned this pull request May 28, 2021
4 tasks
@alex-taffe
Copy link
Contributor

I’d like to get Windows support in here. Thoughts on renaming this “Linux/Windows support” and combining the efforts from #3462?

@jshier
Copy link
Contributor Author

jshier commented May 28, 2021

@alex-taffe Sure. Once you're done with the PR we can merge it in here and work towards full testing support.

@alex-taffe
Copy link
Contributor

Awesome, will let you know when it’s done, should probably just mark as a draft

@alex-taffe
Copy link
Contributor

Actually @jshier, the local build just completed on my Windows box and I think we’re good to go.

* Correct example (#3453)

* Exclude NetworkReachabilityManager from Windows

Fixes #3459. Windows does not have SystemConfiguration, so we have to exclude it

* Initial windows support

* Fix lock issues

* Add Windows tests

* Fix Windows mutex error

* Remove enable test discovery

* Combine Windows and Linux mutex lock with one common NSLock based class

* Remove locked variable check

* Simplify locking extension
cd ${{ github.workspace}}
set SDKROOT=%SystemDrive%\Library\Developer\Platforms\Windows.platform\Developer\SDKs\Windows.sdk
%SystemDrive%\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr\bin\swift-build.exe -c debug -Xlinker /INCREMENTAL:NO -v
if not exist .build\x86_64-unknown-windows-msvc\debug\alamofire.exe exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

@jshier I don’t have write access so I can’t actually change this. TLDR is that for some reason if Swift fails to compile, it doesn’t kill the action. I got the path wrong. Need to change:
if not exist .build\x86_64-unknown-windows-msvc\debug\alamofire.exe exit 1
to:
if not exist .build\x86_64-unknown-windows-msvc\debug\Alamofire.swiftmodule exit 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like that fixed it, nice!

@jshier jshier changed the title Linux Support Linux and Windows Support May 28, 2021
@jshier
Copy link
Contributor Author

jshier commented May 28, 2021

@alex-taffe If you're interested, our next step would be to get our Firewalk test server building and running on Linux and Windows so we can start executing tests. This includes some sort of deployment mechanism for both platforms, whether as a raw binary or through a package manager (Homebrew on Linux?). It's a Vapor server so it should already be cross platform, so it's mainly a tooling issue. Would you be interested in helping with that?

@jshier
Copy link
Contributor Author

jshier commented May 28, 2021

@alex-taffe Since SPM is supposed to work on Windows now, would it be possible to update the Windows build to use that instead of a direct executable?

@alex-taffe
Copy link
Contributor

@alex-taffe If you're interested, our next step would be to get our Firewalk test server building and running on Linux and Windows so we can start executing tests. This includes some sort of deployment mechanism for both platforms, whether as a raw binary or through a package manager (Homebrew on Linux?). It's a Vapor server so it should already be cross platform, so it's mainly a tooling issue. Would you be interested in helping with that?

I would love to, but unfortunately Vapor doesn’t work on Windows yet. I had tried using it a few months ago for a project, but Vapor depends on SwiftNIO (see https://github.com/vapor/vapor/blob/9d154df6214a1d6027f04a495e8981bac49224da/Package.swift#L19). compnerd (not tagging just to avoid junking up his inbox) is working on SwiftNIO support on Windows, but it’s not done yet (apple/swift-nio#1673)

@alex-taffe
Copy link
Contributor

@alex-taffe Since SPM is supposed to work on Windows now, would it be possible to update the Windows build to use that instead of a direct executable?

I’m not entirely sure what you mean. Do you mean replacing %SystemDrive%\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr\bin\swift-build.exe with something more simplistic? SPM is indeed supported (I’m actually using Alamofire as an SPM dependency in an additional project on Windows). We just need that massive command on GitHub Actions because certain environment variables aren’t set properly by the installer on the runner. On my local Windows box, you can just run swift build.

@jshier
Copy link
Contributor Author

jshier commented May 28, 2021

I’m not entirely sure what you mean. Do you mean replacing %SystemDrive%\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr\bin\swift-build.exe with something more simplistic? SPM is indeed supported (I’m actually using Alamofire as an SPM dependency in an additional project on Windows). We just need that massive command on GitHub Actions because certain environment variables aren’t set properly by the installer on the runner. On my local Windows box, you can just run swift build.

I just meant that on Linux we can build (and eventually test) the framework with just swift build. If we need the more complex command that's fine.

I would love to, but unfortunately Vapor doesn’t work on Windows yet. I had tried using it a few months ago for a project, but Vapor depends on SwiftNIO (see https://github.com/vapor/vapor/blob/9d154df6214a1d6027f04a495e8981bac49224da/Package.swift#L19). compnerd (not tagging just to avoid junking up his inbox) is working on SwiftNIO support on Windows, but it’s not done yet (apple/swift-nio#1673)

That's too bad, I was hoping to bring everything into parity. I suppose we need a way to say that Windows just isn't fully supported, which would also apply to Linux until I can get Firewalk ported. You wouldn't have any interest in that, would you?

@alex-taffe
Copy link
Contributor

I might be able to take a look this weekend, otherwise it'll probably be in a few weeks. The Windows support was mainly for a project at work that needs to get done soon. Won't fully commit to it, but I'll try to get to it at some point

@jshier jshier marked this pull request as ready for review May 29, 2021 03:02
@jshier
Copy link
Contributor Author

jshier commented May 31, 2021

@alex-taffe I've updated the PR so that the test suite builds on Linux and Windows. However, there are rather extensive issues with getting it to pass (authentication crashes, bizarre test failures) even once I got Firewalk running on Linux. So while I think we're okay shipping the changes required to build on the other platforms, Alamofire will be completely unsupported on Linux and Windows.

@alex-taffe
Copy link
Contributor

I think I'm good with that

@jshier jshier merged commit a41c07a into master Jun 7, 2021
@jshier jshier deleted the feature/linux-support branch June 7, 2021 22:47
@jshier jshier added this to the 5.4.4 milestone Sep 20, 2021
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.

None yet

2 participants