Skip to content

Conversation

@slashmo
Copy link
Collaborator

@slashmo slashmo commented Oct 1, 2022

Since we plan on introducing Baggage as a dependency for Logging (apple/swift-log#235) we want to lower our minimum supported Swift version to 5.0 to match Logging.

@slashmo slashmo requested a review from ktoso October 1, 2022 08:32
@slashmo slashmo force-pushed the support/swift-5.0 branch from 201444d to af4cca5 Compare October 1, 2022 08:33
@ktoso
Copy link
Member

ktoso commented Oct 1, 2022

@yim-lee could we ask you to help us out and add 5.0 and 5.1 CI to this project? We want to match what swift-log supports, even it supporting 5.0 is a bit ridiculous at this point 😆

@yim-lee
Copy link
Member

yim-lee commented Oct 1, 2022

Added CI for 5.0 and 5.1. @swift-server-bot test this please

@slashmo slashmo force-pushed the support/swift-5.0 branch 2 times, most recently from 7822c98 to 8da2911 Compare October 1, 2022 18:25
@slashmo slashmo force-pushed the support/swift-5.0 branch from 8da2911 to f06643b Compare October 1, 2022 18:29
@slashmo
Copy link
Collaborator Author

slashmo commented Oct 1, 2022

Added CI for 5.0 and 5.1.

Thank you 🙏

@slashmo
Copy link
Collaborator Author

slashmo commented Oct 1, 2022

5.0 is failing because of what I think is a compiler bug. We have a test which contains conditionally compiled code (#if swift(>=5.5) && canImport(_Concurrency)), but it still fails to compile on 5.0:

/code/Tests/InstrumentationBaggageTests/BaggageTests.swift:81:17: error: expected numeric value following '$'
        Baggage.$current.withValue(baggage, operation: exampleFunction)
                        ^

CI Run: https://ci.swiftserver.group/job/swift-distributed-tracing-baggage-swift50-prb/4/console
Unit Test in question: https://github.com/apple/swift-distributed-tracing-baggage/blob/8ea12077aafca36aa28500adead394b7789b8f5c/Tests/InstrumentationBaggageTests/BaggageTests.swift#L66

@ktoso ktoso force-pushed the support/swift-5.0 branch 2 times, most recently from 3b10fe2 to 08902d5 Compare October 3, 2022 02:38
@ktoso ktoso force-pushed the support/swift-5.0 branch from 08902d5 to 39be441 Compare October 3, 2022 03:47
@ktoso
Copy link
Member

ktoso commented Oct 3, 2022

Can't seem to be able to workaround the 5.0 issue, even compiler(>=5.5) does not help. We may need to drop 5.0 support tbh.

@ktoso ktoso force-pushed the support/swift-5.0 branch 3 times, most recently from 6b3ff36 to 39be441 Compare October 4, 2022 06:55
@ktoso
Copy link
Member

ktoso commented Oct 5, 2022

Talked a lot with the swift team about the bug here -- there does not seem to be a workaround, we can't support 5.0 basically -- even with the compiler guards it still will attempt to lex the task local but will fail doing so.

@ktoso ktoso changed the title Add support for Swift 5.0 & 5.1 Add support for Swift 5.1 (5.0 impossible due to compiler bug) Nov 6, 2022
@ktoso ktoso merged commit 6c545ed into apple:main Nov 6, 2022
@ktoso
Copy link
Member

ktoso commented Nov 6, 2022

Discussed this in depth with the Swift team, it is impossible for a library to support any task-local usage and "#if-out" its usage without breaking Swift 5.0, since it will still attempt and fail to lex this piece of "iffed out" code. As such, we cannot support 5.0, nor can any library which optionally (!!!) supports task-locals, because of this compiler bug in 5.0.

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.

3 participants