-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[swiftSyntax] Fix issue where swiftc was not found when swiftSyntax is build via Xcode #18673
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
Conversation
@swift-ci Please smoke test |
.deletingLastPathComponent() | ||
.appendingPathComponent("bin") | ||
.appendingPathComponent("swiftc") | ||
// Traverse the path up until we find the |
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.
Nitpick: unfinished comment
This is definitely an improvement, thanks! |
0112573
to
a46d08e
Compare
@swift-ci Please smoke test and merge |
Hmm, is there a reason to not try to make the layout uniform across the two builds? It seems that it only complicates things to have the different layouts. Making the code more robust anyways is probably a good idea. |
@compnerd I think this is the correct answer -- this library is going to be somewhere under I agree that the layouts should match across platforms, but that's a more invasive change. |
@swift-ci Please smoke test and merge |
@harlanhaskins - also would be interesting to consider windows :-) |
Sorry @ahoppen about causing the conflicts. Should be relatively easy to get these changes moved on top of the changes that I had. |
@compnerd Ah, yes! Apologies -- I am admittedly entirely uninformed of Windows library structure. What does the layout currently look like on Windows? |
a46d08e
to
a4c879c
Compare
@swift-ci Please smoke test |
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.
Yeah, this is incorrect, sorry. For Apple platforms, both Ninja and Xcode build into the architecture-specific directories first, but then lipo together all the architectures into the platform directory. If Xcode's not doing that for you we either have a CMake bug or you're building the wrong target.
a4c879c
to
e6228c0
Compare
…s build via Xcode
e6228c0
to
3534733
Compare
@swift-ci Please smoke test |
@compnerd This change still covers your use case, right? |
[swiftSyntax] Fix issue where swiftc was not found when swiftSyntax is build via Xcode
swiftSyntax
findsswiftc
based on the directory layout of the build folder. However, that layout is different ifswiftSyntax
is built via Xcode than if it's built with Ninja.For Ninja:
For Xcode:
Allow
swiftSyntax
to findswiftc
by traversing up to thelib
folder and then moving tobin/
instead of hardcoding the nesting level oflibswiftSwiftSyntax.dylib
insidelib
.