Skip to content

Disable default OpenAPIRuntime traits#87

Merged
simonjbeaumont merged 6 commits intoapple:mainfrom
madsodgaard:main
Apr 8, 2026
Merged

Disable default OpenAPIRuntime traits#87
simonjbeaumont merged 6 commits intoapple:mainfrom
madsodgaard:main

Conversation

@madsodgaard
Copy link
Copy Markdown
Contributor

Disables the default traits for OpenAPIRuntime, so that anyone that depends on urlsession transport, can choose disable the default. runtime traits

Related:

@simonjbeaumont
Copy link
Copy Markdown
Collaborator

We're not anticipating that many folks are using this transport on non-Darwin platforms. Are we expecting that dropping the foundation dependency is bringing any value here?

@madsodgaard
Copy link
Copy Markdown
Contributor Author

The issue is that if you have a cross-platform lib, where you use URLSession transport on Darwin and AsyncHTTPClient on non-Darwin, like:

.package(url: "https://github.com/apple/swift-openapi-urlsession", from: "1.2.0"),
.package(url: "https://github.com/swift-server/swift-openapi-async-http-client", from: "1.4.0"),
.package(url: "https://github.com/apple/swift-openapi-runtime", from: "1.11.0", traits: []),

The traits: [] won't work, as long as swift-openapi-urlsession also depends on the runtime lib, but with the default traits being enabled. So, we'd have to conditionally include the urlsession transport using something like canImport(Darwin) in the manifest file, which seems suboptimal.

This is even if you conditionally include the target:

.product(
  name: "OpenAPIURLSession",
  package: "swift-openapi-urlsession",
  condition: .when(platforms: [.macOS, .iOS])
),

@simonjbeaumont
Copy link
Copy Markdown
Collaborator

This is even if you conditionally include the target:

This seems like a serious limitation of the trait-based resolution in SwiftPM. Have we filed an issue for that?

@madsodgaard
Copy link
Copy Markdown
Contributor Author

@simonjbeaumont Hm, I am not sure that is a "bug". swift-openapi-urlsession depends on openapi-runtime like any other package would. Let's say that swift-openapi-urlsession in fact used some of the features that was enabled by certain traits from the runtime. Then the "outer" package that depended on both, cannot just override that, otherwise urlsession would not compile. Maybe @FranzBusch can chime in on this as well.

@madsodgaard
Copy link
Copy Markdown
Contributor Author

@simonjbeaumont Would you be against getting this merged?

@madsodgaard
Copy link
Copy Markdown
Contributor Author

@simonjbeaumont Hey, just wanted to check-in on this PR.

We are currently forced to do an ugly #canImport(Darwin) to conditionally include URLSession transport, to prevent linking Foundation on our cross-platform lib.

We've done the same empty traits change for other repos, anything stopping us from doing the same here, and merging this in?

@simonjbeaumont
Copy link
Copy Markdown
Collaborator

Sorry. Still on vacation. I don't mind getting this merged. I kicked the CI. Maybe @guoye-zhang can shepherd it through while I'm out.

@simonjbeaumont
Copy link
Copy Markdown
Collaborator

Looks like something is wrong with the CI related to the import statements:


  |  `- error: consecutive statements on a line must be separated by ';'

@guoye-zhang
Copy link
Copy Markdown
Contributor

We should probably get #89 merged first, but I don’t have the admin rights to do so

@simonjbeaumont
Copy link
Copy Markdown
Collaborator

OK, I merged #89 and updated this PR branch to unblock progress on this PR.

@simonjbeaumont simonjbeaumont added the 🆕 semver/minor Adds new public API. label Apr 7, 2026
@simonjbeaumont
Copy link
Copy Markdown
Collaborator

Looks like the C++ interop is unrelated. This PR is good to review now. I'll take a look.

Copy link
Copy Markdown
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

@madsodgaard thanks for your patience here while I was OOO. We've now dropped 6.0, so can we remove the additional manifest? Otherwise this LGTM.

@madsodgaard
Copy link
Copy Markdown
Contributor Author

@simonjbeaumont no problem! I removed the 6.0 manifest

Copy link
Copy Markdown
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

Thanks @madsodgaard!

@simonjbeaumont simonjbeaumont merged commit 576a65b into apple:main Apr 8, 2026
88 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants