Skip to content

Commit 0923558

Browse files
capickettfacebook-github-bot
authored andcommittedOct 31, 2023
Allow unbundled linking of rust libraries when using buck2
Summary: This diff ports the `native_unbundle_deps` feature from buck1 to buck2. The effect of this change is to make all `cxx_library` -> `rust_library` edges in the dep graph result in linking `rlib` artifacts rather than `staticlib` artifacts. The benefit being that we can then avoid issues with duplicate deps across libraries. rust-lang/rust#73632 has much more detail about this approach. Reviewed By: zertosh Differential Revision: D50523279 fbshipit-source-id: 16ee22db9140dba1e882d86aa376a03e010ef352
1 parent 387e683 commit 0923558

6 files changed

+132
-67
lines changed
 

‎rust/build.bzl

+5-2
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ def generate_rustdoc_test(
213213
exec_is_windows = ctx.attrs._exec_os_type[OsLookup].platform == "windows"
214214

215215
toolchain_info = compile_ctx.toolchain_info
216+
native_unbundle_deps = toolchain_info.native_unbundle_deps
216217

217218
resources = create_resource_db(
218219
ctx = ctx,
@@ -230,7 +231,7 @@ def generate_rustdoc_test(
230231
if link_style == LinkStyle("shared"):
231232
shlib_info = merge_shared_libraries(
232233
ctx.actions,
233-
deps = inherited_shared_libs(ctx, include_doc_deps = True),
234+
deps = inherited_shared_libs(ctx, native_unbundle_deps, include_doc_deps = True),
234235
)
235236
for soname, shared_lib in traverse_shared_library_info(shlib_info).items():
236237
shared_libs[soname] = shared_lib.lib
@@ -258,7 +259,7 @@ def generate_rustdoc_test(
258259
LinkArgs(flags = extra_link_args),
259260
get_link_args_for_strategy(
260261
ctx,
261-
inherited_merged_link_infos(ctx, include_doc_deps = True),
262+
inherited_merged_link_infos(ctx, native_unbundle_deps, include_doc_deps = True),
262263
# TODO(cjhopman): It's unclear how rust is using link_style. I'm not sure if it's intended to be a LibOutputStyle or a LinkStrategy.
263264
to_link_strategy(link_style),
264265
),
@@ -374,6 +375,7 @@ def rust_compile(
374375
exec_is_windows = ctx.attrs._exec_os_type[OsLookup].platform == "windows"
375376

376377
toolchain_info = compile_ctx.toolchain_info
378+
native_unbundle_deps = toolchain_info.native_unbundle_deps
377379

378380
lints, clippy_lints = _lint_flags(compile_ctx)
379381

@@ -444,6 +446,7 @@ def rust_compile(
444446
ctx,
445447
inherited_merged_link_infos(
446448
ctx,
449+
native_unbundle_deps,
447450
include_doc_deps = False,
448451
),
449452
# TODO(cjhopman): It's unclear how rust is using link_style. I'm not sure if it's intended to be a LibOutputStyle or a LinkStrategy.

‎rust/build_params.bzl

+52-11
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,37 @@ def output_filename(cratename: str, emit: Emit, buildparams: BuildParams, extra:
124124
# Rule type - 'binary' also covers 'test'
125125
RuleType = enum("binary", "library")
126126

127-
# What language we're generating artifacts to be linked with
128-
LinkageLang = enum("rust", "c++")
127+
# Controls how we build our rust libraries, largely dependent on whether rustc
128+
# or buck is driving the final linking and whether we are linking the artifact
129+
# into other rust targets.
130+
#
131+
# Rust: In this mode, we build rust libraries as rlibs. This is the primary
132+
# approach for building rust targets when the final link step is driven by
133+
# rustc (e.g. rust_binary, rust_unittest, etc).
134+
#
135+
# Native: In this mode, we build rust libraries as staticlibs, where rustc
136+
# will bundle all of this target's rust dependencies into a single library
137+
# artifact. This approach is the most standardized way to build rust libraries
138+
# for linkage in non-rust code.
139+
#
140+
# NOTE: This approach does not scale well. It's possible to end up with
141+
# non-rust target A depending on two rust targets B and C, which can cause
142+
# duplicate symbols if B and C share common rust dependencies.
143+
#
144+
# Native Unbundled: In this mode, we revert back to building as rlibs. This
145+
# approach mitigates the duplicate symbol downside of the "Native" approach.
146+
# However, this option is not formally supported by rustc, and depends on an
147+
# implementation detail of rlibs (they're effectively .a archives and can be
148+
# linked with other native code using the CXX linker).
149+
#
150+
# See https://github.com/rust-lang/rust/issues/73632 for more details on
151+
# stabilizing this approach.
152+
153+
LinkageLang = enum(
154+
"rust",
155+
"native",
156+
"native-unbundled",
157+
)
129158

130159
_BINARY_SHARED = 0
131160
_BINARY_PIE = 1
@@ -231,10 +260,15 @@ _INPUTS = {
231260
("binary", False, "static", "shared", "rust"): _BINARY_NON_PIE,
232261
("binary", False, "static", "static", "rust"): _BINARY_NON_PIE,
233262
# Native linkable shared object
234-
("library", False, "shared", "any", "c++"): _NATIVE_LINKABLE_SHARED_OBJECT,
235-
("library", False, "shared", "shared", "c++"): _NATIVE_LINKABLE_SHARED_OBJECT,
236-
("library", False, "static", "shared", "c++"): _NATIVE_LINKABLE_SHARED_OBJECT,
237-
("library", False, "static_pic", "shared", "c++"): _NATIVE_LINKABLE_SHARED_OBJECT,
263+
("library", False, "shared", "any", "native"): _NATIVE_LINKABLE_SHARED_OBJECT,
264+
("library", False, "shared", "shared", "native"): _NATIVE_LINKABLE_SHARED_OBJECT,
265+
("library", False, "static", "shared", "native"): _NATIVE_LINKABLE_SHARED_OBJECT,
266+
("library", False, "static_pic", "shared", "native"): _NATIVE_LINKABLE_SHARED_OBJECT,
267+
# Native unbundled linkable shared object
268+
("library", False, "shared", "any", "native-unbundled"): _RUST_DYLIB_SHARED,
269+
("library", False, "shared", "shared", "native-unbundled"): _RUST_DYLIB_SHARED,
270+
("library", False, "static", "shared", "native-unbundled"): _RUST_DYLIB_SHARED,
271+
("library", False, "static_pic", "shared", "native-unbundled"): _RUST_DYLIB_SHARED,
238272
# Rust dylib shared object
239273
("library", False, "shared", "any", "rust"): _RUST_DYLIB_SHARED,
240274
("library", False, "shared", "shared", "rust"): _RUST_DYLIB_SHARED,
@@ -258,12 +292,19 @@ _INPUTS = {
258292
("library", False, "static", "any", "rust"): _RUST_STATIC_NON_PIC_LIBRARY,
259293
("library", False, "static", "static", "rust"): _RUST_STATIC_NON_PIC_LIBRARY,
260294
# Native linkable static_pic
261-
("library", False, "shared", "static", "c++"): _NATIVE_LINKABLE_STATIC_PIC,
262-
("library", False, "static_pic", "any", "c++"): _NATIVE_LINKABLE_STATIC_PIC,
263-
("library", False, "static_pic", "static", "c++"): _NATIVE_LINKABLE_STATIC_PIC,
295+
("library", False, "shared", "static", "native"): _NATIVE_LINKABLE_STATIC_PIC,
296+
("library", False, "static_pic", "any", "native"): _NATIVE_LINKABLE_STATIC_PIC,
297+
("library", False, "static_pic", "static", "native"): _NATIVE_LINKABLE_STATIC_PIC,
264298
# Native linkable static non-pic
265-
("library", False, "static", "any", "c++"): _NATIVE_LINKABLE_STATIC_NON_PIC,
266-
("library", False, "static", "static", "c++"): _NATIVE_LINKABLE_STATIC_NON_PIC,
299+
("library", False, "static", "any", "native"): _NATIVE_LINKABLE_STATIC_NON_PIC,
300+
("library", False, "static", "static", "native"): _NATIVE_LINKABLE_STATIC_NON_PIC,
301+
# Native Unbundled static_pic library
302+
("library", False, "shared", "static", "native-unbundled"): _RUST_STATIC_PIC_LIBRARY,
303+
("library", False, "static_pic", "any", "native-unbundled"): _RUST_STATIC_PIC_LIBRARY,
304+
("library", False, "static_pic", "static", "native-unbundled"): _RUST_STATIC_PIC_LIBRARY,
305+
# Native Unbundled static (non-pic) library
306+
("library", False, "static", "any", "native-unbundled"): _RUST_STATIC_NON_PIC_LIBRARY,
307+
("library", False, "static", "static", "native-unbundled"): _RUST_STATIC_NON_PIC_LIBRARY,
267308
}
268309

269310
# Check types of _INPUTS, writing these out as types is too verbose, but let's make sure we don't have any typos.

‎rust/link_info.bzl

+42-36
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ RustCxxLinkGroupInfo = record(
172172
link_group_libs = field(dict[str, [LinkGroupLib, None]]),
173173
# mapping from target labels to the corresponding link group link_info
174174
labels_to_links_map = field(dict[Label, LinkGroupLinkInfo]),
175-
# prefrred linkage mode for link group libraries
175+
# preferred linkage mode for link group libraries
176176
link_group_preferred_linkage = field(dict[Label, Linkage]),
177177
)
178178

@@ -265,8 +265,9 @@ def _create_linkable_graph(
265265
return linkable_graph
266266

267267
# Returns native link dependencies.
268-
def _non_rust_link_deps(
268+
def _native_link_dependencies(
269269
ctx: AnalysisContext,
270+
native_unbundle_deps: bool,
270271
include_doc_deps: bool = False) -> list[Dependency]:
271272
"""
272273
Return all first-order native linkable dependencies of all transitive Rust
@@ -276,30 +277,31 @@ def _non_rust_link_deps(
276277
looking for non-Rust native link infos (and terminating the search there).
277278
"""
278279
first_order_deps = [dep.dep for dep in resolve_deps(ctx, include_doc_deps)]
279-
return [
280-
d
281-
for d in first_order_deps
282-
if RustLinkInfo not in d and MergedLinkInfo in d
283-
]
280+
281+
if native_unbundle_deps:
282+
return [d for d in first_order_deps if MergedLinkInfo in d]
283+
else:
284+
return [
285+
d
286+
for d in first_order_deps
287+
if RustLinkInfo not in d and MergedLinkInfo in d
288+
]
284289

285290
# Returns native link dependencies.
286-
def _non_rust_link_infos(
291+
def _native_link_infos(
287292
ctx: AnalysisContext,
293+
native_unbundle_deps: bool,
288294
include_doc_deps: bool = False) -> list[MergedLinkInfo]:
289295
"""
290296
Return all first-order native link infos of all transitive Rust libraries.
291-
292-
This emulates v1's graph walk, where it traverses through Rust libraries
293-
looking for non-Rust native link infos (and terminating the search there).
294-
MergedLinkInfo is a mapping from link style to all the transitive deps
295-
rolled up in a tset.
296297
"""
297-
link_deps = _non_rust_link_deps(ctx, include_doc_deps)
298+
link_deps = _native_link_dependencies(ctx, native_unbundle_deps, include_doc_deps)
298299
return [d[MergedLinkInfo] for d in link_deps]
299300

300301
# Returns native link dependencies.
301-
def _non_rust_shared_lib_infos(
302+
def _native_shared_lib_infos(
302303
ctx: AnalysisContext,
304+
native_unbundle_deps: bool,
303305
include_doc_deps: bool = False) -> list[SharedLibraryInfo]:
304306
"""
305307
Return all transitive shared libraries for non-Rust native linkabes.
@@ -308,11 +310,15 @@ def _non_rust_shared_lib_infos(
308310
Rust libraries to collect all transitive shared libraries.
309311
"""
310312
first_order_deps = [dep.dep for dep in resolve_deps(ctx, include_doc_deps)]
311-
return [
312-
d[SharedLibraryInfo]
313-
for d in first_order_deps
314-
if RustLinkInfo not in d and SharedLibraryInfo in d
315-
]
313+
314+
if native_unbundle_deps:
315+
return [d[SharedLibraryInfo] for d in first_order_deps if SharedLibraryInfo in d]
316+
else:
317+
return [
318+
d[SharedLibraryInfo]
319+
for d in first_order_deps
320+
if RustLinkInfo not in d and SharedLibraryInfo in d
321+
]
316322

317323
# Returns native link dependencies.
318324
def _rust_link_infos(
@@ -323,21 +329,21 @@ def _rust_link_infos(
323329
def normalize_crate(label: str) -> str:
324330
return label.replace("-", "_")
325331

326-
# TODO(pickett): Currently this assumes the library target is being built as a
327-
# staticlib or cdylib.
328-
def inherited_exported_link_deps(ctx: AnalysisContext) -> list[Dependency]:
332+
def inherited_exported_link_deps(ctx: AnalysisContext, native_unbundle_deps: bool) -> list[Dependency]:
329333
deps = {}
330-
for dep in _non_rust_link_deps(ctx):
334+
for dep in _native_link_dependencies(ctx, native_unbundle_deps):
331335
deps[dep.label] = dep
332-
for info in _rust_link_infos(ctx):
333-
for dep in info.exported_link_deps:
334-
deps[dep.label] = dep
336+
if not native_unbundle_deps:
337+
for info in _rust_link_infos(ctx):
338+
for dep in info.exported_link_deps:
339+
deps[dep.label] = dep
335340
return deps.values()
336341

337342
def inherited_rust_cxx_link_group_info(
338343
ctx: AnalysisContext,
344+
native_unbundle_deps: bool,
339345
link_style: [LinkStyle, None] = None) -> RustCxxLinkGroupInfo:
340-
link_deps = inherited_exported_link_deps(ctx)
346+
link_deps = inherited_exported_link_deps(ctx, native_unbundle_deps)
341347

342348
# Assume a rust executable wants to use link groups if a link group map
343349
# is present
@@ -415,24 +421,24 @@ def inherited_rust_cxx_link_group_info(
415421
link_group_preferred_linkage = link_group_preferred_linkage,
416422
)
417423

418-
# TODO(pickett): Currently this assumes the library target is being built as a
419-
# staticlib or cdylib.
420424
def inherited_merged_link_infos(
421425
ctx: AnalysisContext,
426+
native_unbundle_deps: bool,
422427
include_doc_deps: bool = False) -> list[MergedLinkInfo]:
423428
infos = []
424-
infos.extend(_non_rust_link_infos(ctx, include_doc_deps))
425-
infos.extend([d.merged_link_info for d in _rust_link_infos(ctx, include_doc_deps) if d.merged_link_info])
429+
infos.extend(_native_link_infos(ctx, native_unbundle_deps, include_doc_deps))
430+
if not native_unbundle_deps:
431+
infos.extend([d.merged_link_info for d in _rust_link_infos(ctx, include_doc_deps) if d.merged_link_info])
426432
return infos
427433

428-
# TODO(pickett): Currently this assumes the library target is being built as a
429-
# staticlib or cdylib.
430434
def inherited_shared_libs(
431435
ctx: AnalysisContext,
436+
native_unbundle_deps: bool,
432437
include_doc_deps: bool = False) -> list[SharedLibraryInfo]:
433438
infos = []
434-
infos.extend(_non_rust_shared_lib_infos(ctx, include_doc_deps))
435-
infos.extend([d.shared_libs for d in _rust_link_infos(ctx, include_doc_deps)])
439+
infos.extend(_native_shared_lib_infos(ctx, native_unbundle_deps, include_doc_deps))
440+
if not native_unbundle_deps:
441+
infos.extend([d.shared_libs for d in _rust_link_infos(ctx, include_doc_deps)])
436442
return infos
437443

438444
def inherited_external_debug_info(

‎rust/rust_binary.bzl

+3-1
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ def _rust_binary_common(
9797

9898
target_os_type = ctx.attrs._target_os_type[OsLookup]
9999
linker_type = compile_ctx.cxx_toolchain_info.linker_info.type
100+
native_unbundle_deps = compile_ctx.toolchain_info.native_unbundle_deps
100101

101102
resources = flatten_dict(gather_resources(
102103
label = ctx.label,
@@ -133,6 +134,7 @@ def _rust_binary_common(
133134
if enable_link_groups(ctx, link_style, specified_link_style, is_binary = True):
134135
rust_cxx_link_group_info = inherited_rust_cxx_link_group_info(
135136
ctx,
137+
native_unbundle_deps,
136138
link_style = link_style,
137139
)
138140
link_group_mappings = rust_cxx_link_group_info.link_group_info.mappings
@@ -147,7 +149,7 @@ def _rust_binary_common(
147149
if link_style == LinkStyle("shared") or rust_cxx_link_group_info != None:
148150
shlib_info = merge_shared_libraries(
149151
ctx.actions,
150-
deps = inherited_shared_libs(ctx),
152+
deps = inherited_shared_libs(ctx, native_unbundle_deps),
151153
)
152154

153155
link_group_ctx = LinkGroupContext(

‎rust/rust_library.bzl

+26-17
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,19 @@ def prebuilt_rust_library_impl(ctx: AnalysisContext) -> list[Provider]:
138138
pdb = None,
139139
external_debug_info = external_debug_info,
140140
)
141+
142+
# Prebuilt libraries only work in unbundled mode, as they only support `rlib`
143+
# files today.
144+
native_unbundle_deps = True
141145
providers.append(
142146
RustLinkInfo(
143147
crate = crate,
144148
styles = styles,
145-
exported_link_deps = inherited_exported_link_deps(ctx),
146-
merged_link_info = create_merged_link_info_for_propagation(ctx, inherited_merged_link_infos(ctx)),
149+
exported_link_deps = inherited_exported_link_deps(ctx, native_unbundle_deps),
150+
merged_link_info = create_merged_link_info_for_propagation(ctx, inherited_merged_link_infos(ctx, native_unbundle_deps)),
147151
shared_libs = merge_shared_libraries(
148152
ctx.actions,
149-
deps = inherited_shared_libs(ctx),
153+
deps = inherited_shared_libs(ctx, native_unbundle_deps),
150154
),
151155
),
152156
)
@@ -237,7 +241,7 @@ def rust_library_impl(ctx: AnalysisContext) -> list[Provider]:
237241
check_artifacts.update(meta.diag)
238242

239243
rust_param_artifact[params] = _handle_rust_artifact(ctx, params, link, meta)
240-
if LinkageLang("c++") in param_lang[params]:
244+
if LinkageLang("native") in param_lang[params] or LinkageLang("native-unbundled") in param_lang[params]:
241245
native_param_artifact[params] = link
242246

243247
# Among {rustdoc, doctests, macro expand}, doctests are the only one which
@@ -317,6 +321,7 @@ def rust_library_impl(ctx: AnalysisContext) -> list[Provider]:
317321
)
318322
providers += _rust_providers(
319323
ctx = ctx,
324+
compile_ctx = compile_ctx,
320325
lang_style_param = lang_style_param,
321326
param_artifact = rust_param_artifact,
322327
)
@@ -364,8 +369,8 @@ def _build_params_for_styles(
364369

365370
# Styles+lang linkage to params
366371
for linkage_lang in LinkageLang:
367-
# Skip proc_macro + c++ combination
368-
if ctx.attrs.proc_macro and linkage_lang == LinkageLang("c++"):
372+
# Skip proc_macro + non-rust combinations
373+
if ctx.attrs.proc_macro and linkage_lang != LinkageLang("rust"):
369374
continue
370375

371376
for link_style in LinkStyle:
@@ -516,12 +521,14 @@ def _default_providers(
516521

517522
def _rust_providers(
518523
ctx: AnalysisContext,
524+
compile_ctx: CompileContext,
519525
lang_style_param: dict[(LinkageLang, LinkStyle), BuildParams],
520526
param_artifact: dict[BuildParams, RustLinkStyleInfo]) -> list[Provider]:
521527
"""
522528
Return the set of providers for Rust linkage.
523529
"""
524530
crate = attr_crate(ctx)
531+
native_unbundle_deps = compile_ctx.toolchain_info.native_unbundle_deps
525532

526533
style_info = {
527534
link_style: param_artifact[lang_style_param[(LinkageLang("rust"), link_style)]]
@@ -532,9 +539,9 @@ def _rust_providers(
532539
# non-Rust rules, found by walking through -- and ignoring -- Rust libraries
533540
# to find non-Rust native linkables and libraries.
534541
if not ctx.attrs.proc_macro:
535-
inherited_link_deps = inherited_exported_link_deps(ctx)
536-
inherited_link_infos = inherited_merged_link_infos(ctx)
537-
inherited_shlibs = inherited_shared_libs(ctx)
542+
inherited_link_deps = inherited_exported_link_deps(ctx, native_unbundle_deps)
543+
inherited_link_infos = inherited_merged_link_infos(ctx, native_unbundle_deps)
544+
inherited_shlibs = inherited_shared_libs(ctx, native_unbundle_deps)
538545
else:
539546
# proc-macros are just used by the compiler and shouldn't propagate
540547
# their native deps to the link line of the target.
@@ -566,14 +573,16 @@ def _native_providers(
566573
"""
567574
Return the set of providers needed to link Rust as a dependency for native
568575
(ie C/C++) code, along with relevant dependencies.
569-
570-
TODO: This currently assumes `staticlib`/`cdylib` behaviour, where all
571-
dependencies are bundled into the Rust crate itself. We need to break out of
572-
this mode of operation.
573576
"""
574-
inherited_link_deps = inherited_exported_link_deps(ctx)
575-
inherited_link_infos = inherited_merged_link_infos(ctx)
576-
inherited_shlibs = inherited_shared_libs(ctx)
577+
578+
# If native_unbundle_deps is set on the the rust toolchain, then build this artifact
579+
# using the "native-unbundled" linkage language. See LinkageLang docs for more details
580+
native_unbundle_deps = compile_ctx.toolchain_info.native_unbundle_deps
581+
lang = LinkageLang("native-unbundled") if native_unbundle_deps else LinkageLang("native")
582+
583+
inherited_link_deps = inherited_exported_link_deps(ctx, native_unbundle_deps)
584+
inherited_link_infos = inherited_merged_link_infos(ctx, native_unbundle_deps)
585+
inherited_shlibs = inherited_shared_libs(ctx, native_unbundle_deps)
577586
linker_info = compile_ctx.cxx_toolchain_info.linker_info
578587
linker_type = linker_info.type
579588

@@ -593,7 +602,7 @@ def _native_providers(
593602
external_debug_infos = {}
594603
for output_style in LibOutputStyle:
595604
legacy_link_style = legacy_output_style_to_link_style(output_style)
596-
params = lang_style_param[(LinkageLang("c++"), legacy_link_style)]
605+
params = lang_style_param[(lang, legacy_link_style)]
597606
lib = param_artifact[params]
598607
libraries[output_style] = lib
599608

‎rust/rust_toolchain.bzl

+4
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ _rust_toolchain_attrs = {
6868
# Utilities used for building flagfiles containing dynamic crate names
6969
"concat_tool": None,
7070
"transitive_dependency_symlinks_tool": None,
71+
# Passing true here enables the unstable feature using `rlib` format
72+
# instead of `staticlib` when linking rust targets into native (e.g.
73+
# C/C++) targets.
74+
"native_unbundle_deps": False,
7175
}
7276

7377
RustToolchainInfo = provider(fields = _rust_toolchain_attrs.keys())

0 commit comments

Comments
 (0)
Please sign in to comment.