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.23k stars 89 forks source link

How to avoid SwiftSyntax annoyance? #329

Closed groue closed 7 months ago

groue commented 7 months ago

Hello,

I'm a happy user of Swift OpenAPI generator, and this afternoon I met my first SPM problem due to a conflicting dependency on http://github.com/apple/swift-syntax from another package.

SPM error >Failed to resolve dependencies Dependencies could not be resolved because 'XXX' depends on 'swift-openapi-generator' 0.3.0..<0.4.0 and 'XXX' depends on 'swift-snapshot-testing' 1.14.1..<2.0.0. > 'swift-snapshot-testing' is incompatible with 'swift-openapi-generator' because 'swift-openapi-generator' 0.3.0 depends on 'swift-syntax' 508.0.1..<509.0.0 and no versions of 'swift-openapi-generator' match the requirement 0.3.1..<0.4.0. > 'swift-snapshot-testing' >= 1.14.1 practically depends on 'swift-syntax' 509.0.0..<510.0.0 because 'swift-snapshot-testing' 1.14.1 depends on 'swift-syntax' 509.0.0..<510.0.0 and no versions of 'swift-snapshot-testing' match the requirement 1.14.2..<2.0.0.

SwiftSyntax is well-known for not following semver, and just create churn in the package ecosystem. See the forum thread Macro Adoption Concerns around SwiftSyntax for an interesting exploration of the problem.

Yes, the landscape may improve over time. For example, maybe SPM will learn to better deal with packages that only depend on swift-syntax for their own tests.

Meanwhile, the dependency of swift-openapi-generator on swift-syntax is just a ticking bomb waiting to annoy your users when they least expect it.

I'm opening a symmetric issue in the other repository: https://github.com/pointfreeco/swift-snapshot-testing/discussions/794.

czechboy0 commented 7 months ago

Hi @groue, what do you suggest here?

groue commented 7 months ago

The (SwiftSyntax, SPM) pair is breaking the ecosystem, so this is a pretty big problem to solve.

Yet, each individual package that depends on SwiftSyntax is also a part of the problem.

Why not drop the dependency on SwiftSyntax? If Swift OpenAPI generator uses it to generate Swift code, aren't there any alternative? Like, say, print?

Also: why not contact other Apple folks and remind them that this issue should be solved at a level or another, because it makes your repo hard to use? (I won't tell you the ugly SPM workarounds I have to do right now because of this problem).

czechboy0 commented 7 months ago

While I'd also like to see improvements in this area and I appreciate the issues it's causing, how can we help in the short term? For example, we are planning moving to 5.9 as the minimum Swift version, which would include bumping the minimum swift-syntax and swift-format version to 5.9: #75 - would you be interested in contributing that change?

groue commented 7 months ago

I won't tell you the ugly SPM workarounds I have to do right now because of this problem

I was wrong: there is no workaround 🤦

we are planning moving to 5.9 as the minimum Swift version [...] would you be interested in contributing that change

Why not, but honestly this won't be because of this issue.

My only choice here is to drop swift-openapi-generator, or drop swift-snapshot-testing. Relying on both libraries to synchronize their dependency on SwiftSyntax on the long run is not a reasonable bet. Add a third library in the equation, and I'm basically stuck, unable to upgrade anything until all three libs agree.

The problem is the immaturity of the (SwiftSyntax, SPM) pair. I guess it will be fixed eventually. But SwiftSyntax moves fast and break things, while SPM development is slow and conservative. I'm not holding my breath. And I won't shoot in my foot.

czechboy0 commented 7 months ago

Understood. I appreciate you also pushing in the thread on this topic.

stephencelis commented 7 months ago

I noted in the thread that this package currently conflicts with any package that includes a macro with the same dependencies generated by the Xcode template for macro packages.

As @mbrandonw notes (in the issue @groue opened in our repo), we can update swift-snapshot-testing to more widely target swift-syntax starting from 508.0 with a liberal dash of #if, that would technically prevent consumers of both packages from actually using any of the macro features of our library, since swift-openapi-generator would still force the earlier version.

I think @ahoppen may be able to suggest a solution here.

groue commented 7 months ago

And @czechboy0, don't hesitate hinting me at how I can help for #75 :)

czechboy0 commented 7 months ago

@groue Ha, well I'd say let's bump everything to 5.9 (since we're still pre-1.0), so essentially grep -r '5\.8' and grep -r '508' in this, runtime, URLSession transport, and AHC transport repos, and move it all up to 5.9. Then you'll spot if anything needs to be updated, but it's possible this might be it.

mbrandonw commented 7 months ago

@czechboy0 I may be missing something, but it doesn't seem like SwiftSyntax is actually used anywhere in the library? I seem to be able to build everything even after I remove the dependency. So, maybe it's just not needed?

simonjbeaumont commented 7 months ago

@czechboy0 I may be missing something, but it doesn't seem like SwiftSyntax is actually used anywhere in the library? I seem to be able to build everything even after I remove the dependency. So, maybe it's just not needed?

It's will still be coming in as a dependency of swift-format, which presumably you did not remove from the package dependencies in your experiment?

mbrandonw commented 7 months ago

Hi @simonjbeaumont, that is true, making the situation all the more complex.

It really does seem that Apple's own open source projects should be leading the way in terms of supporting older versions of SwiftSyntax and being a good citizen in the ecosystem. I think OpenAPIGenerator should relax their dependency on SwiftFormat to be "508.0.1"..<"510.0.0". That would allow people to start using macros while still depending on OpenAPIGenerator, and it doesn't take much to accomplish.

By having such huge, linchpin projects in the ecosystem commit to just a single version of SwiftSyntax we are slowly creeping into really messy territory. And I think it's really going to hit the fan once Swift 5.10 comes out.

simonjbeaumont commented 7 months ago

@mbrandonw I agree we need to have a solution here and thanks for taking the time to look at what it would take for this project.

It’s always been on our radar to have a good story for this before 1.0. And now is as good a time as any.

FWIW, I’m not against that proposed patch. We’ll get back to this early next week.

czechboy0 commented 7 months ago

Same, the patch looks good, just should probably also relax the swift-syntax dependency in addition to the swift-format one. @mbrandonw, if you turn that into a PR that'd be a much welcome contribution.

groue commented 7 months ago

Thank you all for such a good description of the topic, sharing so much experience, and soothing my concerns. Whenever I have to open a similar issue in another repository, I'll write a much better original post.

mbrandonw commented 7 months ago

Hi @simonjbeaumont & @czechboy0, sure thing. Done here https://github.com/apple/swift-openapi-generator/pull/331 🙂

czechboy0 commented 7 months ago

With #331, we should have this issue addressed for the 5.9 release shortly, and I'd encourage anyone who has ideas to chime in on the Forums post to help the community improve the story around using swift-syntax. Thanks @groue for driving this!