Skip to content
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

Use CMake's built-in toolchain detection #23998

Conversation

Projects
None yet
3 participants
@jkoritzinsky
Copy link
Member

jkoritzinsky commented Apr 15, 2019

Try removing manual resolution of toolchain for non-cross builds and let CMake resolve the toolchain itself.

Try removing manual resolution of toolchain for non-cross builds and …
…let CMake resolve the toolchain itself.

@jkoritzinsky jkoritzinsky changed the title WIP: Use CMake's built-in toolchain detection Use CMake's built-in toolchain detection Apr 15, 2019

@jkoritzinsky jkoritzinsky requested a review from jkotas Apr 15, 2019

@jkoritzinsky jkoritzinsky marked this pull request as ready for review Apr 15, 2019

@jkotas jkotas requested a review from janvorli Apr 15, 2019

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Apr 15, 2019

@janvorli understands the cmake plumbing

@janvorli

This comment has been minimized.

Copy link
Member

janvorli commented Apr 16, 2019

I've looked why we have added the stuff that is now being removed. It comes from the ancient #35 and it looks like it was fixing a problem related to building with clang 3.4 or 3.5.
But I want to verify that it always picks the same version of the llvm-xxx tools as the clang. I have many llvm versions installed on my Linux box side by side, so I can do that.

@jkoritzinsky

This comment has been minimized.

Copy link
Member Author

jkoritzinsky commented Apr 16, 2019

@janvorli sounds good. If you can take a look at that when you have some free cycles that'd be great!

@janvorli

This comment has been minimized.

Copy link
Member

janvorli commented Apr 16, 2019

Unfortunately, the results of my testing show this change is problematic. With your change, the cmake variables are set as follows for all versions of clang:

CMAKE_AR: /usr/bin/ar
CMAKE_LINKER: /usr/bin/ld
CMAKE_NM: /usr/bin/nm
CMAKE_OBJDUMP /usr/bin/objdump

Without your change, the correct llvm versions of the tools are used. Example - clang 4.0:

CMAKE_AR: /usr/bin/llvm-ar-4.0
CMAKE_LINKER: /usr/bin/llvm-link-4.0
CMAKE_NM: /usr/bin/llvm-nm-4.0
CMAKE_OBJDUMP /usr/bin/llvm-objdump-4.0
@jkoritzinsky

This comment has been minimized.

Copy link
Member Author

jkoritzinsky commented Apr 16, 2019

Ah well. Guess this isn’t gonna work without some additional work to preserve the behavior. I’ll close this for now and take another stab at it some time.

@janvorli

This comment has been minimized.

Copy link
Member

janvorli commented Apr 16, 2019

It causes the same issue for cross build. For example alpine arm32 build

CMAKE_AR: /usr/bin/armv6-alpine-linux-musleabihf-ar
CMAKE_LINKER: /usr/bin/armv6-alpine-linux-musleabihf-ld
CMAKE_NM: /usr/bin/armv6-alpine-linux-musleabihf-nm
CMAKE_OBJDUMP /usr/bin/armv6-alpine-linux-musleabihf-objdump

while the state before your change was the same as for x64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.