-
Notifications
You must be signed in to change notification settings - Fork 86
Properly handle toolchains from Command Line Tools on macOS #1936
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
base: main
Are you sure you want to change the base?
Properly handle toolchains from Command Line Tools on macOS #1936
Conversation
c2116fc to
f454019
Compare
c68263d to
97cc986
Compare
award999
left a comment
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.
Some initial comments but this is fairly large so we should get some more eyes on it
| } | ||
| } | ||
|
|
||
| export function chaiPathPlugin(chai: Chai.ChaiStatic, _utils: Chai.ChaiUtils): void { |
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.
Did this serve a purpose beyond making the assertions below easier? If not then it's in place now I guess but in the future we should do these in a separate tech debt PR to cut down on the noise in this bug fix. I assume there are a lot of cases across our tests that could have made use of this that are going to be missed
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.
I can split this into a separate PR if we want. Some code paths use path.resolve() which changes the slashes to the current platform while others don't. This meant that certain tests would need to handle this while others didn't (and it isn't obvious why when looking at the test code). I wanted a consistent way to check for path equality.
|
If you set |
e9fafab to
c145074
Compare
Description
Adjust toolchain discovery to detect which tool manages the
swiftbinary that was found inPATH. This allows us to delegate to the appropriate tool when trying to find toolchain executables. Possible toolchain managers are:xcrun: Toolchain was either installed viaxcode-select --installor by downloading Xcode. Executesxcrun --find swiftto find the path to the toolchain. Only available on macOS.swiftly: Toolchain was installed via swiftly. Executesswiftly use --print-locationto find the path to the toolchain.swiftenv: Toolchain was installed via swiftenv. Executesswiftenv which swiftto find the path to the toolchain.unknown: A toolchain manager was not recognized. The path to the toolchain is assumed to be two levels above the path to theswiftbinary inPATH.Detecting toolchains that need
xcrunis done by performing anobjdumpon theswiftbinary and looking for an__xcrun_shimdata segment. If one is found then we know that we have to usexcrunwhen querying for toolchain executables even if the path to the toolchain does not live within anXcode.app. This makes the toolchain detection logic more resilient instead of using brittle heuristics that only look at the toolchain path.Issue: #1077
Tasks
[ ] Documentation has been updated