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

rust_bindgen fails on macOS due to fatal error: 'TargetConditionals.h' file not found #899

Closed
kiron1 opened this issue Aug 18, 2021 · 9 comments · Fixed by #916
Closed
Assignees

Comments

@kiron1
Copy link
Contributor

kiron1 commented Aug 18, 2021

I have a Bazel project where I want to use rules_rust with rust_bindgen to be able to use Duktape in rust.

I have a cc_library(...) which can successfully compile Duktape, but the rust_bindgen(...) target fails with the following error:

duktape-sys/src/duk_config.h:407:10: fatal error: 'TargetConditionals.h' file not found
duktape-sys/src/duk_config.h:407:10: fatal error: 'TargetConditionals.h' file not found, err: true
thread 'main' panicked at 'Unable to generate bindings: ()', external/rules_rust_bindgen__bindgen__0_58_1/src/main.rs:54:36

duk_config.h:407

When I use clang from _bindgen_clang_repositories ($(bazel info execution_root)/external/bindgen_clang_osx/bin/clang -c duktape-sys/src/duktape.c ) I get indeed this error:

duktape-sys/src/duk_config.h:407:10: fatal error: 'TargetConditionals.h' file not found

However, when using xcrun clang -c duktape-sys/src/duktape.c, it compiles without problems.

Is there a way to teach rust_bindgen to use clang with the includes directories used by xcrun clang?

@kiron1
Copy link
Contributor Author

kiron1 commented Aug 19, 2021

A hacky way to fix the issue, is to teach rust_bindgen to also look in $(xcrun --show-sdk-path)/usr/include for headers using the clang_flags argument:

rust_bindgen(
    name = "bindings",
    bindgen_flags = [
        "--allowlist-function=duk_.*",
        "--allowlist-type=duk_.*",
        "--allowlist-var=DUK_.*",
    ],
    cc_lib = ":duktapelib",
    clang_flags = [
        "-isystem",
        "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include",
    ],
    header = ":src/duktape.h",
    visibility = ["//visibility:private"],
)

I hope there is a more bazel-style way of fixing it, since the CC Toolchain / cc_library knows the right path already.

@illicitonion
Copy link
Collaborator

@kiron1 Would it be possible to get a self-contained repro (either a git repo, or a WORKSPACE + BUILD file) that shows the issue?

@kiron1
Copy link
Contributor Author

kiron1 commented Aug 20, 2021

@illicitonion sure, here the minimal example: https://github.com/kiron1/rules_rust_bindgen

Bazel version: 4.2.0
macOS: 10.15.7
Xcode 12.4

@illicitonion
Copy link
Collaborator

Fantastic, thanks, that was very useful.

The problem here is that Bazel sets up a cc_toolchain with all of these extra flags, but the bindgen code completely ignores the toolchain and tries to bring along its own clang and such.

I don't have much bindgen context, but I can give a brain-dump of the Bazel side of this:

  • On macOS, cc_toolchains use a small binary which wraps clang as the compiler, instead of clang itself it dynamically looks up things like the SDK root, and replaces a bunch of template variables it sees in any flags it's passed, e.g. __BAZEL_XCODE_SDKROOT__.
  • The way for a rule to get this information is to look up parts of the cc_toolchain and either use those when constructing command lines, or just let the cc_toolchain do the command line construction entirely. AFAICT looking at bindgen, the need to do the former - mix in extra flags and config when constructing command lines.

I wired up the cc_toolchain to grab its clang-wrapper and flags, but this wasn't sufficient to make things Just Work - I'm guessing because of either some handling bindgen itself does, or because bindgen appears to use libclang directly and presumably that side-steps any of the substitution done by the wrapper.

Unfortunately my bindgen-fu is basically nil, but maybe the below diff can give someone a jumping off point for how to wire these things into bindgen:

diff --git a/bindgen/bindgen.bzl b/bindgen/bindgen.bzl
index cf28023..01fa248 100644
--- a/bindgen/bindgen.bzl
+++ b/bindgen/bindgen.bzl
@@ -16,7 +16,12 @@
 load("//rust:rust.bzl", "rust_library")
 
 # buildifier: disable=bzl-visibility
-load("//rust/private:utils.bzl", "find_toolchain", "get_preferred_artifact")
+load("//rust/private:utils.bzl", "find_cc_toolchain", "find_toolchain", "get_preferred_artifact")
+
+load(
+    "@bazel_tools//tools/build_defs/cc:action_names.bzl",
+    "CPP_COMPILE_ACTION_NAME",
+)
 
 # TODO(hlopko): use the more robust logic from rustc.bzl also here, through a reasonable API.
 def _get_libs_for_static_executable(dep):
@@ -121,6 +126,26 @@ def _rust_bindgen_impl(ctx):
     args.add_all(system_include_directories, before_each = "-isystem")
     args.add_all(clang_args)
 
+    cc_toolchain, feature_configuration = find_cc_toolchain(ctx)
+
+    # See https://docs.bazel.build/versions/4.2.0/skylark/lib/cc_common.html#create_compile_variables
+    # Instead of ourselves doing the -iquote and similar adding in the lines above,
+    # we could be passing these  as arguments to `crate_compile_variables` and it would set up the correct flags for us.
+    compile_variables = cc_common.create_compile_variables(
+        cc_toolchain = cc_toolchain,
+        feature_configuration = feature_configuration,
+    )
+
+    # This will include e.g. "-isysroot" "__BAZEL_XCODE_SDKROOT__"
+    compile_args_from_toolchain = cc_common.get_memory_inefficient_command_line(
+        feature_configuration = feature_configuration,
+        action_name = CPP_COMPILE_ACTION_NAME,
+        variables = compile_variables,
+    )
+
+    # This points at the clang-wrapper that does env var substitution.
+    clang_bin_from_toolchain = cc_common.get_tool_for_action(feature_configuration = feature_configuration, action_name = CPP_COMPILE_ACTION_NAME)
+
     env = {
         "CLANG_PATH": clang_bin.path,
         "LIBCLANG_PATH": libclang_dir,
@@ -194,12 +219,15 @@ rust_bindgen = rule(
             allow_single_file = True,
             cfg = "exec",
         ),
+        "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"),
     },
     outputs = {"out": "%{name}.rs"},
     toolchains = [
         str(Label("//bindgen:bindgen_toolchain")),
         str(Label("//rust:toolchain")),
+        "@rules_cc//cc:toolchain_type"
     ],
+    fragments = ["cpp"],
     incompatible_use_toolchain_transition = True,
 )

@sayrer
Copy link
Contributor

sayrer commented Aug 21, 2021

It looks like this issue is due to bindgen's use of the clang-sys crate. I think the way to improve this .bzl file is to add more environment variables. In this case, perhaps the cc_toolchain can populate one of the include path variables that Clang checks: https://clang.llvm.org/docs/CommandGuide/clang.html#environment

Not an uncommon issue, it seems: rust-lang/rust-bindgen#1226

That bindgen issue has links showing a bunch of other ways to solve this problem, like: https://github.com/apache/tvm/pull/6765/files

illicitonion pushed a commit that referenced this issue Aug 31, 2021
This fixes #899 by putting  `XCODE_VERSION_OVERRIDE`, `APPLE_SDK_VERSION_OVERRIDE`, `APPLE_SDK_PLATFORM` in the env.
@sayrer
Copy link
Contributor

sayrer commented Aug 31, 2021

@illicitonion sure, here the minimal example: https://github.com/kiron1/rules_rust_bindgen

Bazel version: 4.2.0
macOS: 10.15.7
Xcode 12.4

@kiron1 could you contribute this repo as an example here?

@kiron1
Copy link
Contributor Author

kiron1 commented Sep 1, 2021

@sayrer sure I can create a PR for bazelbuild/rules_rust with the example, but what do we want to demonstrate with the example?

a) That #include <TargetConditionals.h> can workr, or
b) how to configure Bazel und Cargo such that both work with bindgen and can use include_str!() for the generated source?

There is already examples/bindgen. Do we want to extend this example, or add a second example?

@sayrer
Copy link
Contributor

sayrer commented Sep 1, 2021

a) That #include <TargetConditionals.h> can work

I was thinking of that. I'd just do whatever's easiest in terms of where to put it.

@kiron1
Copy link
Contributor Author

kiron1 commented Sep 2, 2021

a) That #include <TargetConditionals.h> can work

I was thinking of that. I'd just do whatever's easiest in terms of where to put it.

Something like this #925 ?

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 a pull request may close this issue.

3 participants