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

Fix iconv and clang version detection on OSX #6917

Merged
merged 3 commits into from Jan 5, 2019

Conversation

Projects
None yet
7 participants
@LordAro
Copy link
Member

LordAro commented Sep 24, 2018

Rationale:

Under OSX, clang is actually all compilers - cc, gcc & clang. We already grep the output of --version to get that gcc might actually be clang, but the version number is hidden. For reasons best known to themselves, Apple have decided to completely remove the "actual" clang version number and use their own.

This screws with the CFLAGS somewhat, as several of them are specific to compiler versions.

It turns out that gcc -v under OSX actually outputs

Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/c++/4.2.1
Apple LLVM version 9.1.0 (clang-902.0.39.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

which isn't exactly what we were expecting. Coupled with the fact that the clang "version number" is determined by taking the first 2 digits from the first line of output, and we get version... 10

This is solved by only taking the lines containing "version", which seems to be constant between all variants. We then get version "91", which is still inaccurate, but actually works for our purposes

As for iconv, for some reason on @andythenorth's system iconv.h is only in /usr/local/Cellar/libiconv/1.15/include/iconv.h, and the configure script only searches within /usr/include & /usr/local/include. This seems fairly arbitrary anyway, so some magic involving the compiler output (which seems to be consistent between gcc & clang) and awk gets us a list of search paths to ...search within

@LordAro LordAro force-pushed the LordAro:macosx-clang-iconv-fix branch 2 times, most recently from 5a714af to cfc9fde Sep 24, 2018

@LordAro LordAro requested a review from michicc Sep 24, 2018

Show resolved Hide resolved config.lib Outdated
Show resolved Hide resolved config.lib Outdated

@LordAro LordAro force-pushed the LordAro:macosx-clang-iconv-fix branch from cfc9fde to c53a7d9 Sep 30, 2018

updated

@LordAro

This comment has been minimized.

Copy link
Member

LordAro commented Oct 21, 2018

In the interests of actually fixing #6880, would it be reasonable to just "fake" the clang version if it's on OSX? arbitrarily set it to 5.0 or something

@LordAro LordAro force-pushed the LordAro:macosx-clang-iconv-fix branch from c53a7d9 to a72729d Nov 25, 2018

@LordAro

This comment has been minimized.

Copy link
Member

LordAro commented Nov 25, 2018

Given recent additions (C++11 enum types), I figure it is better to just force C++11 globally for all compilers. A bit of a stop-gap measure until CMake happens, but it does make it work...

@andythenorth

This comment has been minimized.

Copy link
Contributor

andythenorth commented Nov 25, 2018

Works on macOS 10.13.6 with Clang "Apple LLVM version 10.0.0 (clang-1000.11.45.5)"

@LordAro LordAro force-pushed the LordAro:macosx-clang-iconv-fix branch from a72729d to 30e05e5 Nov 25, 2018

@orudge

This comment has been minimized.

Copy link
Contributor

orudge commented Dec 3, 2018

Also works for me on Mac OS X 10.11.6 with Clang "Apple LLVM version 8.0.0 (clang-800.0.42.1).

@orudge
Copy link
Contributor

orudge left a comment

Can't see anything that looks obviously wrong to me; tested on OS X 10.11 with Apple LLVM 8 and works for me.

@nielsmh

This comment has been minimized.

Copy link
Contributor

nielsmh commented Dec 27, 2018

Oddly, this does not seem to cause problems with the macOS CI tests being run right now, even with no extra CXXFLAGS.

@LordAro LordAro force-pushed the LordAro:macosx-clang-iconv-fix branch from 30e05e5 to 7ed3153 Jan 5, 2019

@TrueBrain TrueBrain merged commit 4fbfe34 into OpenTTD:master Jan 5, 2019

@LordAro LordAro deleted the LordAro:macosx-clang-iconv-fix branch Jan 5, 2019

@JGRennison

This comment has been minimized.

Copy link
Contributor

JGRennison commented Jan 13, 2019

With this PR applied:
A version string of "Apple LLVM version 8.0.0 (clang-800.0.42.1)" produces a cc_version of 80.
A version string of "Apple LLVM version 10.0.0 (clang-1000.11.45.5)" produces a cc_version of 10, which doesn't seem correct. This causes some of the subsequent tests of the cc_version variable to be incorrect, though as these only control warning flags compilation still succeeds.

@LordAro

This comment has been minimized.

Copy link
Member

LordAro commented Jan 13, 2019

Yeah, this was known. It's unfortunate, but there wasn't really any other way around it, without hugely complicating everything. We decided it was better to have the version a bit wrong (and to force c++11) and not worry about it until someone rewrites the buildsystem in cmake or similar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment