-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Concurrency] Disable applying the -fswift-async-fp flag for Xcode builds until someone figures out the root cause #39561
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @buttaface
Does this works locally for you? I thought about testing that but because of this comment #39368 (comment), trying to compile the stdlib without this flag doesn't work so I'm not sure this will solve the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned a couple times on that earlier pull, I don't use macOS, so this pull has not been tested whatsoever.
My understanding of the problem from what you and others initially described is that the host clang at
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/
is being wrongly used by the Xcode generator to build the Swift stdlib, despite the Swift CMake config specifying that it use the just-built Swift-forked clang instead. This pull works around that by not passing the flag when the Xcode generator is used, because the host clang doesn't support this flag.In your linked comment, you then claim that the Xcode generator was using the Swift-forked, freshly-built clang to build this Concurrency library, in which case this pull won't work, as you then have much deeper problems if the Xcode generator is randomly choosing which clang compiler to use.
I suggest you do a normal build without this pull, and paste the full error you get here. Then, apply this pull and rebuild without cleaning and see what difference it makes. If both runs show different compilers being used, I don't know what to tell you, there's something deeply wrong with your Xcode setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the suggestion, I'll take a deeper look at it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any luck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @buttaface,
No, I tried this change and keep hitting that linker issue ... so I tried Doug's change locally from #39573 and it seems to be enough to unblock Xcode and build everything =]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, can you provide the error messages I asked for before, ie the full error on a stock build and after applying this patch? You don't need to do a full rebuild because these patches only affect the stdlib, ie this should be pretty easy to do with an existing build just by changing the patch applied to this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, I completely missed this sorry, but it is the same linker error as mentioned on the original PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Xcode not show the failing command? I don't care about the error as much as what particular command caused it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It fails while linking
libswift_Concurrency.dylib
As for the build-script command I'm using
SKIP_XCODE_VERSION_CHECK=1 ./swift/utils/build-script --xcode --release-debuginfo --debug-swift --skip-build-benchmarks --swift-darwin-supported-archs "$(uname -m)" --sccache --skip-ios --skip-tvos --skip-watchos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, okay, I didn't know the macOS linker also requires that flag to be applied, I thought this was purely a compilation issue. This will require looking into the Xcode generator for CMake to see why it is building the stdlib with the host clang and not with Swift's fork of clang, as I mentioned before. Since I can't do that, I'll close this second attempt to work around that CMake bug.