Skip to content

Commit bb38e76

Browse files
authored
Use the libraries included with the clang distribution for bindgen (#820)
* Use the libraries included with the clang distribution for bindgen * Fall back to the system load path if libstdcxx is not specified * Regenerate documentation * Fix bugs and preserve backward compatibility with users of bindgen_clang_* * Buildifier * Regenerate documentation * Re-enable tests * Generate rpaths correctly * Skip rpath generation on Windows * Better comment about why test is disabled * Add comment about DYLD_LIBRARY_PATH * buildifier
1 parent c9345d3 commit bb38e76

File tree

7 files changed

+42
-60
lines changed

7 files changed

+42
-60
lines changed

.bazelci/presubmit.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,8 @@ tasks:
2929
- "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245
3030
- "..."
3131
- "@examples//..."
32-
# Skip tests for dylib support on osx, since we don't support it yet.
32+
# This test requires --incompatible_macos_set_install_name and Bazel 4.2.0+
3333
- "-@examples//ffi/rust_calling_c:matrix_dylib_test"
34-
- "-@examples//ffi/rust_calling_c:matrix_dynamically_linked"
35-
- "-@examples//ffi/rust_calling_c/simple/..."
3634
build_targets: *osx_targets
3735
test_targets: *osx_targets
3836
build_flags:

bindgen/BUILD.bazel

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@ rust_bindgen_toolchain(
1919
"//conditions:default": "@bindgen_clang_linux//:clang",
2020
}),
2121
libclang = select({
22-
"//rust/platform:osx": "@bindgen_clang_osx//:libclang.so",
23-
"//conditions:default": "@bindgen_clang_linux//:libclang.so",
22+
"//rust/platform:osx": "@bindgen_clang_osx//:libclang",
23+
"//conditions:default": "@bindgen_clang_linux//:libclang",
24+
}),
25+
libstdcxx = select({
26+
"//rust/platform:osx": "@bindgen_clang_osx//:libc++",
27+
"//conditions:default": None,
2428
}),
25-
libstdcxx = "@local_libstdcpp//:libstdc++",
2629
)
2730

2831
toolchain(

bindgen/bindgen.bzl

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,6 @@ def _rust_bindgen_impl(ctx):
8787
rustfmt_bin = toolchain.rustfmt or rust_toolchain.rustfmt
8888
clang_bin = toolchain.clang
8989
libclang = toolchain.libclang
90-
91-
# TODO: This rule shouldn't need to depend on libstdc++
92-
# This rule requires an explicit dependency on a libstdc++ because
93-
# 1. It is a runtime dependency of libclang.so
94-
# 2. We cannot locate it in the cc_toolchain yet
95-
# Depending on how libclang.so was compiled, it may try to locate its libstdc++ dependency
96-
# in a way that makes our handling here unnecessary (eg. system /usr/lib/x86_64-linux-gnu/libstdc++.so.6)
9790
libstdcxx = toolchain.libstdcxx
9891

9992
# rustfmt is not where bindgen expects to find it, so we format manually
@@ -130,8 +123,11 @@ def _rust_bindgen_impl(ctx):
130123
"RUST_BACKTRACE": "1",
131124
}
132125

126+
# Set the dynamic linker search path so that clang uses the libstdcxx from the toolchain.
127+
# DYLD_LIBRARY_PATH is LD_LIBRARY_PATH on macOS.
133128
if libstdcxx:
134129
env["LD_LIBRARY_PATH"] = ":".join([f.dirname for f in _get_libs_for_static_executable(libstdcxx).to_list()])
130+
env["DYLD_LIBRARY_PATH"] = env["LD_LIBRARY_PATH"]
135131

136132
ctx.actions.run(
137133
executable = bindgen_bin,
@@ -140,9 +136,9 @@ def _rust_bindgen_impl(ctx):
140136
transitive = [
141137
cc_lib[CcInfo].compilation_context.headers,
142138
_get_libs_for_static_executable(libclang),
143-
] + [
139+
] + ([
144140
_get_libs_for_static_executable(libstdcxx),
145-
] if libstdcxx else [],
141+
] if libstdcxx else []),
146142
),
147143
outputs = [unformatted_output],
148144
mnemonic = "RustBindgen",
@@ -232,9 +228,10 @@ rust_bindgen_toolchain = rule(
232228
providers = [CcInfo],
233229
),
234230
"libstdcxx": attr.label(
235-
doc = "A cc_library that satisfies libclang's libstdc++ dependency.",
231+
doc = "A cc_library that satisfies libclang's libstdc++ dependency. This is used to make the execution of clang hermetic. If None, system libraries will be used instead.",
236232
cfg = "exec",
237233
providers = [CcInfo],
234+
mandatory = False,
238235
),
239236
"rustfmt": attr.label(
240237
doc = "The label of a `rustfmt` executable. If this is provided, generated sources will be formatted.",

bindgen/repositories.bzl

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,6 @@ def rust_bindgen_repositories():
2424
# nb. The bindgen rule itself should work on any platform.
2525
_bindgen_clang_repositories()
2626

27-
maybe(
28-
_local_libstdcpp,
29-
name = "local_libstdcpp",
30-
)
31-
3227
rules_rust_bindgen_fetch_remote_crates()
3328

3429
native.register_toolchains(str(Label("//bindgen:default_bindgen_toolchain")))
@@ -48,19 +43,29 @@ http_archive(
4843
"""
4944

5045
_CLANG_BUILD_FILE = """\
51-
load("@rules_cc//cc:defs.bzl", "cc_library")
46+
load("@rules_cc//cc:defs.bzl", "cc_import")
5247
5348
package(default_visibility = ["//visibility:public"])
5449
5550
sh_binary(
5651
name = "clang",
5752
srcs = ["bin/clang"],
58-
data = glob(["lib/**"]),
5953
)
6054
61-
cc_library(
55+
cc_import(
56+
name = "libclang",
57+
shared_library = "lib/libclang.{suffix}",
58+
)
59+
60+
alias(
6261
name = "libclang.so",
63-
srcs = ["{}"],
62+
actual = ":libclang",
63+
deprecation = "Use :libclang instead",
64+
)
65+
66+
cc_import(
67+
name = "libc++",
68+
shared_library = "lib/libc++.{suffix}"
6469
)
6570
"""
6671

@@ -72,7 +77,7 @@ def _bindgen_clang_repositories():
7277
urls = ["https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0/clang+llvm-10.0.0-x86_64-linux-gnu-ubuntu-18.04.tar.xz"],
7378
strip_prefix = "clang+llvm-10.0.0-x86_64-linux-gnu-ubuntu-18.04",
7479
sha256 = "b25f592a0c00686f03e3b7db68ca6dc87418f681f4ead4df4745a01d9be63843",
75-
build_file_content = _CLANG_BUILD_FILE.format("lib/libclang.so"),
80+
build_file_content = _CLANG_BUILD_FILE.format(suffix = "so"),
7681
workspace_file_content = _COMMON_WORKSPACE.format("bindgen_clang_linux"),
7782
)
7883

@@ -82,32 +87,6 @@ def _bindgen_clang_repositories():
8287
urls = ["https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0/clang+llvm-10.0.0-x86_64-apple-darwin.tar.xz"],
8388
strip_prefix = "clang+llvm-10.0.0-x86_64-apple-darwin",
8489
sha256 = "633a833396bf2276094c126b072d52b59aca6249e7ce8eae14c728016edb5e61",
85-
build_file_content = _CLANG_BUILD_FILE.format("lib/libclang.dylib"),
90+
build_file_content = _CLANG_BUILD_FILE.format(suffix = "dylib"),
8691
workspace_file_content = _COMMON_WORKSPACE.format("bindgen_clang_osx"),
8792
)
88-
89-
_LIBSTDCPP_BUILD_FILE = """\
90-
load("@rules_cc//cc:defs.bzl", "cc_library")
91-
92-
cc_library(
93-
name = "libstdc++",
94-
srcs = ["{}"],
95-
visibility = ["//visibility:public"]
96-
)
97-
"""
98-
99-
def _local_libstdcpp_impl(repository_ctx):
100-
os = repository_ctx.os.name.lower()
101-
if os == "linux":
102-
repository_ctx.symlink("/usr/lib/x86_64-linux-gnu/libstdc++.so.6", "libstdc++.so.6")
103-
repository_ctx.file("BUILD.bazel", _LIBSTDCPP_BUILD_FILE.format("libstdc++.so.6"))
104-
elif os.startswith("mac"):
105-
repository_ctx.symlink("/usr/lib/libstdc++.6.dylib", "libstdc++.6.dylib")
106-
repository_ctx.file("BUILD.bazel", _LIBSTDCPP_BUILD_FILE.format("libstdc++.6.dylib"))
107-
else:
108-
fail(os + " is not supported.")
109-
repository_ctx.file("WORKSPACE.bazel", _COMMON_WORKSPACE.format(repository_ctx.name))
110-
111-
_local_libstdcpp = repository_rule(
112-
implementation = _local_libstdcpp_impl,
113-
)

docs/flatten.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ The tools required for the `rust_bindgen` rule.
367367
| <a id="rust_bindgen_toolchain-bindgen"></a>bindgen | The label of a <code>bindgen</code> executable. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
368368
| <a id="rust_bindgen_toolchain-clang"></a>clang | The label of a <code>clang</code> executable. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
369369
| <a id="rust_bindgen_toolchain-libclang"></a>libclang | A cc_library that provides bindgen's runtime dependency on libclang. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
370-
| <a id="rust_bindgen_toolchain-libstdcxx"></a>libstdcxx | A cc_library that satisfies libclang's libstdc++ dependency. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
370+
| <a id="rust_bindgen_toolchain-libstdcxx"></a>libstdcxx | A cc_library that satisfies libclang's libstdc++ dependency. This is used to make the execution of clang hermetic. If None, system libraries will be used instead. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
371371
| <a id="rust_bindgen_toolchain-rustfmt"></a>rustfmt | The label of a <code>rustfmt</code> executable. If this is provided, generated sources will be formatted. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
372372

373373

docs/rust_bindgen.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ The tools required for the `rust_bindgen` rule.
4747
| <a id="rust_bindgen_toolchain-bindgen"></a>bindgen | The label of a <code>bindgen</code> executable. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
4848
| <a id="rust_bindgen_toolchain-clang"></a>clang | The label of a <code>clang</code> executable. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
4949
| <a id="rust_bindgen_toolchain-libclang"></a>libclang | A cc_library that provides bindgen's runtime dependency on libclang. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
50-
| <a id="rust_bindgen_toolchain-libstdcxx"></a>libstdcxx | A cc_library that satisfies libclang's libstdc++ dependency. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
50+
| <a id="rust_bindgen_toolchain-libstdcxx"></a>libstdcxx | A cc_library that satisfies libclang's libstdc++ dependency. This is used to make the execution of clang hermetic. If None, system libraries will be used instead. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
5151
| <a id="rust_bindgen_toolchain-rustfmt"></a>rustfmt | The label of a <code>rustfmt</code> executable. If this is provided, generated sources will be formatted. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
5252

5353

rust/private/rustc.bzl

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -752,12 +752,17 @@ def _compute_rpaths(toolchain, output_dir, dep_info):
752752
Returns:
753753
depset: A set of relative paths from the output directory to each dependency
754754
"""
755-
preferreds = [get_preferred_artifact(lib) for linker_input in dep_info.transitive_noncrates.to_list() for lib in linker_input.libraries]
756755

757-
# TODO(augie): I don't understand why this can't just be filtering on
758-
# _is_dylib(lib), but doing that causes failures on darwin and windows
759-
# examples that otherwise work.
760-
dylibs = [lib for lib in preferreds if _has_dylib_ext(lib, toolchain.dylib_ext)]
756+
# Windows has no rpath equivalent, so always return an empty depset.
757+
if toolchain.os == "windows":
758+
return depset([])
759+
760+
dylibs = [
761+
get_preferred_artifact(lib)
762+
for linker_input in dep_info.transitive_noncrates.to_list()
763+
for lib in linker_input.libraries
764+
if _is_dylib(lib)
765+
]
761766
if not dylibs:
762767
return depset([])
763768

0 commit comments

Comments
 (0)