apple / swift-openapi-generator

Generate Swift client and server code from an OpenAPI document.
https://swiftpackageindex.com/apple/swift-openapi-generator/documentation
Apache License 2.0
1.21k stars 87 forks source link

Add macCatalyst to the PlatformChecks #524

Closed lennartkerkvliet closed 3 months ago

lennartkerkvliet commented 3 months ago

Motivation

When attempting to compile the Swift OpenAPI Generator on MacCatalyst, code that compiles on iOS will not compile as it will fail on the PlatformChecks.

Modifications

Added an extra condition in PlatformChecks to allow running on MacCatalyst.

Result

fixes #523

Test Plan

I've tried to familiarize myself with the project, but could not come up with a good way to add a test for running the tool on MacCatalyst. In my manual testing giving the instructions in the issue, it now works as expected.

simonjbeaumont commented 3 months ago

When attempting to compile the Swift OpenAPI Generator on MacCatalyst...

When compiling an iOS app, you run the generator on your macOS developer environment (usually by using the Swift package plugin), but you only link the runtime library to your app. The platform checks here are to stop you linking the generator itself in applications by mistake.

Are you trying to compile for MacCatalyst or on MacCatalyst?

lennartkerkvliet commented 3 months ago

@simonjbeaumont Good question! I wrote about this in the issue a bit more

I suspect that it has to do with the behaviour of MacCatalyst. When compiling the app normally targetEnvironment(macCatalyst) will be true while os(macOS) will be false which is already confusing by itself but with good reason. Perhaps the build tool plug-in is ran in a similar environment?

So importantly, no, I did not link to the generator. With the changes PlatformChecks, the generator will also not be embedded in the final build. I wouldn't be surprised if it's some sort of oversight in building for Catalyst. If you'd like to try it out for yourself, just take any project with the OpenAPI generator for iOS and add MacCatalyst as a supported destination. I would also be glad to provide a sample project, although it's quite straightforward to reproduce.

lennartkerkvliet commented 3 months ago

Here's an Xcode project reproducing the issue. The same configuration compiles on iOS while failing to compile on Catalyst. OpenAPI-Catalyst.zip

simonjbeaumont commented 3 months ago

Thanks for the response and for the reproducer. I can see locally with a dummy package that, when compiling the application code:

  1. When compiling for destination "My Mac", I see os(macOS) is true and targetEnvironment(macCatalyst) is false.
  2. When compiling for destination "My Mac (Mac Catalyst), I see os(iOS) is true and targetEnvironment(macCatalyst) is true.
  3. When compiling for a simulator, e.g. "iPhone 15", I see os(iOS) is true and targetEnvironment(simulator) is true.

This is all expected in the code for the application. What's unexpected is the compiler conditions that are used to build the package plugin:

I think this is a bug in the build system. For now, we can workaround it with this patch—although I've suggested an edit to document it clearer. Would you mind filing an issue using Feedback Assistant and let us know the issue number?

lennartkerkvliet commented 3 months ago

@simonjbeaumont Here's the feedback identifier: FB13589581

simonjbeaumont commented 3 months ago

@swift-server-bot test this please

simonjbeaumont commented 3 months ago

Nightly failure is a compiler crash:

error: compile command failed due to signal 6 (use -v to see invocation)
SIL verification failed: keypath value type should match value type of keypath pattern
  any CodingKey
  URICoderCodingKey
Verifying instruction:
->   %8 = keypath $KeyPath<URIValueFromNodeDecoder.CodingStackEntry, any CodingKey>, (root $URIValueFromNodeDecoder.CodingStackEntry; stored_property #URIValueFromNodeDecoder.CodingStackEntry.key : $URICoderCodingKey) // user: %9
     %9 = move_value [lexical] [var_decl] %8 : $KeyPath<URIValueFromNodeDecoder.CodingStackEntry, any CodingKey> // users: %15, %11
In function:
// URIValueFromNodeDecoder.codingPath.getter

Merging over this.

simonjbeaumont commented 3 months ago

Thanks @lennartkerkvliet!