-
Notifications
You must be signed in to change notification settings - Fork 55
Ship Swift Atomics 1.2 #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Gah, CMakeLists.txt references the .gyb files as sources. This doesn't seem to cause problems, but we should not list them. |
Xcode 15 seems to have trouble linking test targets building for the generic watchOS simulator, which seems to include i386. Ignoring.
|
The This package isn't designed to be built with library evolution enabled, so I'm not treating this as a blocker.
|
A wild failure appeared!
The same test is passing on every other configuration I've tried so far. The failure is specific to this one ordering. Huh? |
Oh, d'oh, the weak compare-exchange is allowed to spuriously fail, but the test expects it to unconditionally succeed. The scheduler must have decided to randomly suspend execution of the process in the middle of the LL/SC transaction. All |
The Swift 5.11 snapshot as of 2023-09-24 doesn't interoperate well with Xcode 15 GM's package integration, so watchOS builds of the xcodeproj under ./Xcode fail to link due to what looks like a missing arch issue in the toolchain's libraries. Also filing this as due to a version mismatch unrelated to this project. Otherwise 5.11 seems to work okay on the candidate commit. |
Oops, platform conditionals are broken on 5.8:
We do no enable ATOMICS_NATIVE_BUILTINS on 5.8, so evidently this is because of Swift's uniquely frustrating parsing rules. :-P |
d976dd2 fixes the parsing issue on 5.8. A quick retest confirms that this does not appear to have adverse effects on 5.9 & 5.11, so not restarting the process. |
Swift 5.8 fails the
|
Oh hm, it looks like we won't be able to properly unify the two low-level primitive implementations -- they will need to produce different operations. The fix is in commit 48fad5a. |
Swift 5.8 on Intel macOS also has trouble with library evolution enabled:
|
The .xcodeproj only supports building Swift 5.9, as it unconditionally configures the project to use native builtins. (This is by design.) |
I am using Atomics in a framework that is being build as an XCFramework which is used by others. Is there any way around this if you need |
The most satisfying solution would be to remove the Until then, we're sort of wedged -- IIRC the problem is that the C headers don't get properly embedded in the xcframework, but the swift module needs to publicly import them. Wait... in 5.9 we could actually hide the import. 🤔 There may be a solution! |
Ah, just to clarify -- @RobFrancisAu, are you currently doing this with swift-atomic 1.1 and Xcode 14.3? (I'm asking because I saw the same failures before the 1.1 release earlier this year -- so it is quite possible that the test script's failure will not interfere with your actual use case! (E.g. you're probably building atomics as a dependency, not directly as a top-level thing.)🤞) |
Using Given that the failure isn't new (and assuming that it doesn't interfere with using swift-atomics as an internal dependency of a .xcframework), I think I'll continue ignoring it for now. I expect we'll be able to eliminate the entire |
Firstly, thanks for your help @lorentey. Yes using 1.1.0 Which seems to be working fine. My project builds without an issue on my machine. The failure is occurring in the my CICD pipeline when trying to archive whilst including
|
Terrific, Swift 5.7 doesn't like seeing
|
Moving the |
Oh, so it's been broken in 1.1, too. That's unfortunate! Using I'll try to land that change anyway and partially rerun the tests to see if anything looks amiss. The risk of regressions is pretty high here, and the "full" tests aren't covering everything, but it's worth a try. |
@RobFrancisAu f26da16 has the potential fix. It will only work on Swift 5.9 or better -- unfortunately we the shims module needs to be a public import on earlier toolchains. (Sadly this may not fix all issues, but hopefully it will resolve the specific build error. Building packages with library evolution enabled can still hit other issues -- xcodebuild's package integration in particular seems to have unique issues with C modules in packages.) |
This never ends! fe0fc8b surfaced a CMake build issue on 5.9. The build config failed to enable native builtins there, so CMake is trying to use C shims even on new compilers:
|
3431b85 (attempts to) fix the CMake configuration. Previous commits to that restrict the Xcode project to only work with Xcode 15, and adjust the code signing configuration to allow running tests on devices. |
OK, it looks like 3431b85 may be a workable commit. |
Uh oh!
There was an error while loading. Please reload this page.
This release will technically not contain any API-level changes, but the fix for #88 is essentially a complete rewrite of the package, so it's risky enough to deserve a minor version bump.
I will not be going through the full test matrix for this release.
Candidate commit:
-
f10f031 (current HEAD of #102, merge pending)-
8be2ba9 (current HEAD of #103, merge pending)-
d8a5239 (main as of #102, #103 and #105)-
1abcb95 (current HEAD of #106)-
8189a83 (current HEAD of #106)-
48fad5a (current HEAD of #106)-
fe0fc8b (current HEAD of #106)-
f26da16 (current HEAD of #106)- 3431b85 (current HEAD of #106)
Diff: 1.1.0...main
Testing progress: (On Mac and Linux, this is done by running
Utilities/run-full-tests.sh
. This script builds and tests this package using various build systems and build configurations.)macOS/AS (Including simulator tests for iOS/watchOS/tvOS)
macOS/x86_64
Linux/Aarch64
Linux/x86_64
Windows 10/x86_64
Known failures:
The text was updated successfully, but these errors were encountered: