Skip to content

Commit a0a52be

Browse files
capickettfacebook-github-bot
authored andcommitted
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 4d98a60 commit a0a52be

File tree

6 files changed

+132
-67
lines changed

6 files changed

+132
-67
lines changed

prelude/rust/build.bzl

Lines changed: 5 additions & 2 deletions
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.

prelude/rust/build_params.bzl

Lines changed: 52 additions & 11 deletions
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.

prelude/rust/link_info.bzl

Lines changed: 42 additions & 36 deletions
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(

prelude/rust/rust_binary.bzl

Lines changed: 3 additions & 1 deletion
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(

0 commit comments

Comments
 (0)