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

Add rules_foreign_cc example #24

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

keith
Copy link
Member

@keith keith commented Sep 22, 2022

No description provided.

@keith
Copy link
Member Author

keith commented Sep 22, 2022

this is failing on macOS, which I lightly started debugging, not sure what the issue is, but there's clearly a mismatch in toolchain selection or something so the cmake invocation is getting macOS args

@keith
Copy link
Member Author

keith commented Sep 22, 2022

related bazel-contrib/rules_foreign_cc#289, but I don't know why this works fine with the bazel core NDK infra and not this repo

@ahumesky
Copy link
Collaborator

this is failing on macOS, which I lightly started debugging, not sure what the issue is, but there's clearly a mismatch in toolchain selection or something so the cmake invocation is getting macOS args

I'll see if anyone might have an idea of what's going on

@keith
Copy link
Member Author

keith commented Nov 29, 2022

thanks that's where I'm surprised that something changed here, but I haven't dug super deep. I imagine someone more familiar with these toolchain setups would know the issue off the top of their head

@keith
Copy link
Member Author

keith commented Dec 8, 2022

@ahumesky you hear any useful tips?

@ahumesky
Copy link
Collaborator

ahumesky commented Dec 8, 2022

Let me follow up on some threads.
I took a quick look at the errors, and yeah I'm not sure where -search_paths_first is coming from, it's not part of rules_android_ndk, and I didn't find it in bazel's codebase either

@keith
Copy link
Member Author

keith commented Dec 8, 2022

cmake default https://github.com/Kitware/CMake/blob/239beca1e3743ec693643869621cd0e41ed10a16/Modules/Platform/Darwin.cmake#L60 added since it's picking the wrong platform for sure

@ahumesky
Copy link
Collaborator

ahumesky commented Dec 8, 2022

and you're not able to repro this locally i assume right?

@keith
Copy link
Member Author

keith commented Dec 8, 2022

oh i think it repros locally too yea. it's definitely a difference in the toolchain configuration somewhere

@ahumesky
Copy link
Collaborator

ahumesky commented Dec 8, 2022

ah ok that's good -- much harder if it just repros on CI

@ahumesky
Copy link
Collaborator

ahumesky commented Dec 8, 2022

I think the problem basically comes down to this:
https://github.com/Kitware/CMake/blob/239beca1e3743ec693643869621cd0e41ed10a16/Modules/Platform/Darwin.cmake#L31-L38

rules_foreign_cc / cmake / cmake() is just going to evaluate that and add the flag based on just the version of the OS. There's nothing connecting that to the fact that this is actually an android build with a different compiler. Is there a way to define HAVE_FLAG_SEARCH_PATHS_FIRST with a value of 0 in the cmake() target?

@keith
Copy link
Member Author

keith commented Dec 8, 2022

it's not just about that flag though, you can see it's adding many macOS specific flags like -isysroot /Applications/Xcode13.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk too. I think the general problem is that rules_foreign_cc tries to use something from the toolchain to indicate the platform and whatever that is differs with this vs the native one

@ahumesky
Copy link
Collaborator

ahumesky commented Dec 8, 2022

oh i didn't realize there were more problematic values in there.

the cmake() rule is grabbing the cpp toolchain:
https://github.com/bazelbuild/rules_foreign_cc/blob/2c6262f8f487cd3481db27e2c509d9e6d30bfe53/foreign_cc/private/framework.bzl#L464

and I believe this is all going through the legacy toolchain resolution (especially with --noincompatible_enable_cc_toolchain_resolution still being used here) -- which I think ends up using --crosstool_top, can you try setting that in addition to --android_crosstool_top? (you may also then have to set --host_crosstool_top so that the host tools don't try to use the android crosstool as well)

@ahumesky
Copy link
Collaborator

ahumesky commented Dec 8, 2022

hmm, although, there's a split transition on android_binary which should have translated --android_crosstool_top to --crosstool_top for its dependencies -- including the cmake() target... so even if setting --crosstool_top makes this work, then there's still more to discover here

@keith keith force-pushed the ks/add-rules_foreign_cc-example branch from 1ddc420 to 890ee63 Compare January 5, 2023 19:56
@keith
Copy link
Member Author

keith commented Jan 5, 2023

yea avoiding with crosstool_top doesn't work and actually seems to cause more issues:

% bazel build //java/com/app:app --crosstool_top=@androidndk//:toolchain --host_crosstool_top=@bazel_tools//tools/cpp:toolchain
ERROR: /private/var/tmp/_bazel_ksmiley/55ec32113415a74e2950047844b9eb9d/external/androidndk/toolchains/llvm/prebuilt/darwin-x86_64/BUILD:20:19: in cc_toolchain_suite rule @androidndk//toolchains/llvm/prebuilt/darwin-x86_64:cc_toolchain_suite: cc_toolchain_suite '@androidndk//toolchains/llvm/prebuilt/darwin-x86_64:cc_toolchain_suite' does not contain a toolchain for cpu 'darwin_arm64'
ERROR: /private/var/tmp/_bazel_ksmiley/55ec32113415a74e2950047844b9eb9d/external/androidndk/toolchains/llvm/prebuilt/darwin-x86_64/BUILD:20:19: Analysis of target '@androidndk//toolchains/llvm/prebuilt/darwin-x86_64:cc_toolchain_suite' failed
ERROR: Analysis of target '//java/com/app:app' failed; build aborted:
INFO: Elapsed time: 0.099s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 0 targets configured)

@keith
Copy link
Member Author

keith commented Jan 6, 2023

Ok actually I think this is just a rules_foreign_cc issue that wasn't a huge deal before but was uncovered because of switching to lld in the NDK. Tracking here bazel-contrib/rules_foreign_cc#289

@keith keith force-pushed the ks/add-rules_foreign_cc-example branch from 890ee63 to 8cde187 Compare March 9, 2023 22:28
@keith keith marked this pull request as ready for review March 9, 2023 22:43
@keith keith requested a review from ahumesky March 9, 2023 22:43
@keith
Copy link
Member Author

keith commented Apr 11, 2023

@ahumesky can you review this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants