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

[NOMERGE] llvm: fix cross-compilation, alternative approach #40711

Closed
wants to merge 4 commits into from

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented May 18, 2018

This should be "ready" functionally (AFAIK),
but needs discussion to sort out how we want to do things.

Cleaned up version of the mess I made over on #40604 (again, sorry!)
providing a different way of reaching those goals.

Not sure if this is my preferred approach,
but hopefully is interesting and helps
move things along.

LMK what you think, thanks! :)


Submitted against 'master' since that's how I developed it
and because staging would only make evaluating this more difficult.
("is build failing because of this PR or because this is staging?" :P)

"-DLLVM_HOST_TRIPLE=${stdenv.hostPlatform.config}"
"-DLLVM_DEFAULT_TARGET_TRIPLE=${stdenv.targetPlatform.config}"
"-DTARGET_TRIPLE=${stdenv.targetPlatform.config}"
]
++ stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
"-DCMAKE_CROSSCOMPILING=ON" # inferred when setting CMAKE_SYSTEM_NAME, but is satisfying to do so explicitly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the cmake setup hook be setting this already? If not sounds like a bug.

@dtzWill
Copy link
Member Author

dtzWill commented May 18, 2018

(cc #25477)

I don't think so? Not sure what setting this is "supposed" to be interpreted as anyway, for example: https://gitlab.kitware.com/cmake/cmake/issues/17653 .

Basically it appears to be true when explicitly set to true (as we do here) or when CMAKE_SYSTEM_NAME was set directly or indirectly.

Perhaps it's best to just avoid bothering with it at all?

@dezgeg
Copy link
Contributor

dezgeg commented May 18, 2018

It should be set because various cmake users will check for it, e.g https://github.com/alisw/clang/blob/master/CMakeLists.txt#L433 (and https://codesearch.debian.net/search?q=CMAKE_CROSSCOMPILING in general)

@dtzWill
Copy link
Member Author

dtzWill commented May 22, 2018

It should be set because various cmake users will check for it, e.g https://github.com/alisw/clang/blob/master/CMakeLists.txt#L433 (and https://codesearch.debian.net/search?q=CMAKE_CROSSCOMPILING in general)

Well we should probably be setting CMAKE_SYSTEM if not CMAKE_CROSSCOMPILING.
Would be interesting to see how that goes when we try to cross-compiling projects that check for this.

FWIW upstream cmake added documentation about it's limitations[1] which is why I'm reluctant to say we should clearly set it. But maybe the right answer is to just set it since that seems right and look at the fallout and handle the cases where those limitations cause problems instead of ignoring it entirely O:).

Alright, you've convinced me :).

Regardless doing that is a big endeavor that may be worth handling separately.

[1] https://gitlab.kitware.com/cmake/cmake/merge_requests/1672/diffs

@dezgeg
Copy link
Contributor

dezgeg commented May 22, 2018

Here's the logic Buildroot uses for CMake packages:

They seem to use a toolchain file that eventually sets set(CMAKE_SYSTEM_NAME Buildroot).

@dtzWill
Copy link
Member Author

dtzWill commented Jun 17, 2018

Nevermind for now, we can revisit if/when have more time for this :).

@dtzWill dtzWill closed this Jun 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants