Conversation
|
If we wait until 6.3 for this we can avoid the split manifest right @czechboy0? If we really want to do it before I'd prefer we use compiler conditionals inside the package manifest. |
|
@simonjbeaumont Yeah, traits were introduced in 6.1 It would be great if we could get this in before... we cannot disable the "FullFoundation" trait in packages where we depend on the generator and the runtime atm. |
|
I'm sympathetic to the motivation to get this over the line. Let's see how we can get it done with minimal fuss. I triggered the CI. Looks like there's some issues. Might be worth experimenting locally with I think sticking to the single package manifest might make this simpler to play around with a get it working locally. |
|
@simonjbeaumont Thanks, will take a look at it locally. Hm, I am not sure if because that will still fail when parsing the package description: |
|
Hm. We've used compiler conditions in manifests before (https://github.com/swift-otel/swift-otel/blob/main/Package.swift) but I guess that was for the same compiler major version. Have you tried with |
|
@simonjbeaumont yeah, |
|
@madsodgaard Could you look at this a bit more. I just managed to get this to compile. I added this extension near the end of the Then using this in the package dependencies: With this patch, I have no problems compiling the package with |
|
@simonjbeaumont Hm, I cannot get that to work when compiling on MacOS with just I am guessing you are also keeping // swift-tools-version:6.0
// ...
.package(url: "https://github.com/apple/swift-openapi-runtime", from: "1.11.0", traitsIfAvailable: []),
// ...
extension Package.Dependency {
static func package(url: String, from version: Version, traitsIfAvailable traits: [String]) -> Package.Dependency {
#if swift(>=6.1)
.package(url: url, from: version, traits: Set(traits.map(Trait.init(stringLiteral:))))
#else
.package(url: url, from: version)
#endif
}
}results in: with |
|
I left it as it was ( Seems that bumping the minimum tools version to 6.0 is causing problems. Why don't we split that out so that you can land this patch? |
|
@simonjbeaumont Just to make sure I understood. You were suggesting to keep |
|
Yes. If you're in a rush to land this PR before 6.3 is released, we can do it that way. Once 6.3 is released this workaround is moot but I'd really rather avoid multiple manifests. |
|
@simonjbeaumont Unfortunately it does not seem that trick actually works when you build. shows that This does not happen when we use multiple manifests versions, so I guess we are forced to do that. |
| @@ -0,0 +1,167 @@ | |||
| // swift-tools-version:6.0 | |||
There was a problem hiding this comment.
main currently supports 5.10+, but I guess this PR drops support for 5.10 and moves to 6.0. #867 does this as well.
Do you want me to revert this to 5.10?
What version did you build this on? root@946be1992f3c:/pwd# swift --version
Swift version 6.2.4 (swift-6.2.4-RELEASE)
Target: aarch64-unknown-linux-gnu
root@946be1992f3c:/pwd# rm -rf .build && swift build -v --target PetstoreConsumerTestCore 2>&1 | grep "OpenAPIRuntime.build" | grep -o "\-DFullFoundation" | head -3
root@946be1992f3c:/pwd# echo $?
0root@223d66ebbf84:/pwd# swift --version
Swift version 6.0.3 (swift-6.0.3-RELEASE)
Target: aarch64-unknown-linux-gnu
root@223d66ebbf84:/pwd# rm -rf .build && swift build -v --target PetstoreConsumerTestCore 2>&1 | grep "OpenAPIRuntime.build" | grep -o "\-DFullFoundation" | head -3
root@223d66ebbf84:/pwd# echo $?
0% swift --version
swift-driver version: 1.127.15 Apple Swift version 6.2.4 (swiftlang-6.2.4.1.4 clang-1700.6.4.2)
Target: arm64-apple-macosx26.0
% rm -rf build && swift build -v --target PetstoreConsumerTestCore 2>&1 | grep "OpenAPIRuntime.build" | grep -o "\-DFullFoundation" | head -3
% echo $?
0 |
|
@simonjbeaumont Not sure if it was a typo, but in your Mac terminal you have I am seeing the issue on 6.2.4 as well. |
|
Heh, I had removed the build directory, but... what I hadn't done (facepalm) is updated the swift-openapi-runtime dependency. I'm very surprised this cannot be done in the package manifest since I have already linked a concrete example where we do use compiler conditionals to change behaviour. I played around some more and agree this isn't feasible. Even with some runtime checks, it seems that Well, thanks for at least entertaining the idea. Let me kick the CI on the current state of the PR and we can go from there. I guess this additional manifest will have a short shelf-life anyway. |
2 participants
Disables the default traits for the
openapi-runtime, to make sure that other packages depending on the generator can actually disable the traits if they want to.