shims/super/cc: remove isysroot space to fix cpp#6749
Merged
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom Nov 20, 2019
Merged
shims/super/cc: remove isysroot space to fix cpp#6749MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
Conversation
30 tasks
Contributor
|
👍
args << "-isysroot #{sysroot}" |
Member
Author
|
Indeed that should also work. Ruby will handle the rest. |
5 tasks
MikeMcQuaid
approved these changes
Nov 20, 2019
Member
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Works for me, thanks @Bo98 for another great PR!
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
brew stylewith your changes locally?brew testswith your changes locally?This is a fix to #5153, where the
cppshim was broken in Xcode 10 and later.It turns out that Xcode's
cppis just a dumb script. Here's a snippet from it:It splits arguments by spaces, and completely breaks down when we pass
-isysroot sdk. The entire flow revolves around$#,$1andshiftwhich work by splitting by spaces so sees-isysrootandsdkas separate arguments. Here is a demonstration of what happens:The script does have special handling for
-imacros,-include,-idirafter,-iprefixand-iwithprefixto work with spaces (or rather require spaces), but not-isysroot.When interacting with clang directly,
-isysrootworks both with and without spaces between-isysrootand the path. In fact, originally spaces weren't supported there either - that's a later innovation. MacPorts doesn't put a space after-isysrootfor that reason, albeit that older toolchain support isn't a concern in Homebrew.Considering
-isysrootwithout spaces still works today, I propose removing the space as that fixes all the problems without having to do specialcpphandling. Note that-isystemalready behaves this way.