Skip to content

Commit 0f59341

Browse files
authored
Use toolchain for tree artifact rpath rewriting (#53580)
### What does this PR do? Adds a TreeArtifact/directory tool to the `rpath_rewriter` toolchains and applies it to replace explicit os-based switching. ### Motivation Continuation of #53450 work to leverage toolchains for adapting rpath rewriting to each platform. ### Describe how you validated your changes Green CI, including passing health check and no visible package changes. ### Additional Notes This drops the `otool` toolchain entirely, as these changes have moved the `otool` portion of the rewriting to simply calling the tool from the full path where the tool is meant to exist on macos systems. If we ever want to bring it back it should probably be included with the macos rewriting tool and not be made into a separate toolchain. This change also made the remapping semantics of `dd_cc_packaged` consistent for files and for directories, simplifying the logic of the implementation and also making it easier to understand at the call site – this explains the changes to openssl and cpython BUILD files. Co-authored-by: alex.lopez <alex.lopez@datadoghq.com>
1 parent 94988ca commit 0f59341

22 files changed

Lines changed: 266 additions & 407 deletions

MODULE.bazel

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -159,15 +159,6 @@ multitool.hub(lockfile = "//bazel:prebuilt_buildtools.json")
159159
multitool.hub(lockfile = "//bazel:prebuilt_jq.json")
160160
use_repo(multitool, "multitool")
161161

162-
## Use local object file tools
163-
find_system_otool = use_extension("//bazel/toolchains/otool:otool_configure.bzl", "find_system_otool", dev_dependency = True)
164-
use_repo(find_system_otool, "otool")
165-
166-
register_toolchains(
167-
"@otool//:all",
168-
dev_dependency = True,
169-
)
170-
171162
register_toolchains(
172163
"//bazel/toolchains/patchelf:patchelf_built_toolchain",
173164
dev_dependency = True,

MODULE.bazel.lock

Lines changed: 0 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bazel/rules/dd_packaging/dd_cc_packaged.bzl

Lines changed: 6 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,15 @@ load("@rules_pkg//pkg:mappings.bzl", "pkg_files")
77
load("@rules_pkg//pkg:providers.bzl", "PackageFilegroupInfo", "PackageFilesInfo")
88
load("//bazel/rules:so_symlink.bzl", "so_symlink")
99
load("//bazel/rules/dd_packaging:dd_packaging_info.bzl", "DdPackagingInfo")
10-
load("//bazel/rules/rewrite_rpath:rewrite_rpath.bzl", "otool_dir_action", "patchelf_dir_action", "rewrite_rpath", "rewrite_rpaths_for_files")
11-
12-
def _is_os(ctx, constraint):
13-
return ctx.target_platform_has_constraint(constraint[platform_common.ConstraintValueInfo])
10+
load("//bazel/rules/rewrite_rpath:rewrite_rpath.bzl", "rewrite_rpath", "rewrite_rpaths")
1411

1512
def _dd_packaged_files_impl(ctx):
16-
is_linux = _is_os(ctx, ctx.attr._linux_constraint)
17-
is_macos = _is_os(ctx, ctx.attr._macos_constraint)
18-
1913
rpath = ctx.attr.rpath.format(install_dir = ctx.attr._install_dir[BuildSettingInfo].value)
2014
dest_src_map = {}
2115

2216
for src, prefix in ctx.attr.srcs.items():
23-
for f in src.files.to_list():
24-
if f.is_directory:
25-
if is_linux:
26-
out = ctx.actions.declare_directory("patched_dirs/" + f.basename)
27-
patchelf_dir_action(ctx, f, out, rpath)
28-
elif is_macos:
29-
out = ctx.actions.declare_directory("patched_dirs/" + f.basename)
30-
otool_dir_action(ctx, f, out, rpath)
31-
else:
32-
out = f
33-
dest = prefix
34-
else:
35-
out = rewrite_rpaths_for_files(ctx, inputs = [f], rpath = rpath)[0]
36-
dest = (prefix + "/" + f.basename) if prefix else f.basename
17+
for out in rewrite_rpaths(ctx, inputs = src.files.to_list(), rpath = rpath):
18+
dest = (prefix + "/" + out.basename) if prefix else out.basename
3719
dest_src_map[dest] = out
3820

3921
return [PackageFilesInfo(
@@ -51,29 +33,10 @@ _dd_packaged_files_rule = rule(
5133
"rpath": attr.string(
5234
default = "{install_dir}/embedded/lib",
5335
),
54-
"_linux_constraint": attr.label(default = "@platforms//os:linux"),
55-
"_macos_constraint": attr.label(default = "@platforms//os:macos"),
56-
"_script": attr.label(
57-
default = "@@//bazel/rules/rewrite_rpath:macos.sh",
58-
allow_single_file = True,
59-
cfg = "exec",
60-
),
61-
"_dir_script": attr.label(
62-
default = "@@//bazel/rules/rewrite_rpath:macos_dir.sh",
63-
allow_single_file = True,
64-
cfg = "exec",
65-
),
66-
"_install_name_tool": attr.label(
67-
default = "@@//bazel/tools:install_name_tool",
68-
executable = True,
69-
cfg = "exec",
70-
),
7136
"_install_dir": attr.label(default = "@@//:install_dir"),
7237
},
7338
toolchains = [
7439
"//bazel/toolchains/rpath_rewriter",
75-
"@@//bazel/toolchains/patchelf:patchelf_toolchain_type",
76-
"@@//bazel/toolchains/otool:otool_toolchain_type",
7740
],
7841
)
7942

@@ -175,10 +138,9 @@ dd_cc_packaged = macro(
175138
depends, directly or indirectly, on the wrapped binary.
176139
177140
If installed_executables is provided, each entry is rpath-patched and
178-
installed with mode 0755. Individual files are installed as prefix/basename;
179-
directory artifacts are installed as the prefix itself (contents copied into it).
180-
Use this instead of wrapping files in pkg_files and passing them via
181-
installed_files, so that rpath rewriting is not skipped.
141+
installed with mode 0755. Files and directory artifacts are installed as
142+
prefix/basename. Use this instead of wrapping files in pkg_files and passing
143+
them via installed_files, so that rpath rewriting is not skipped.
182144
183145
If a version is provided and the input is a cc_shared_library, the library
184146
will be installed along with the versioned symlink (see so_symlink).

bazel/rules/dd_packaging/dd_packaging_test.bzl

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
"""Tests for dd_collect_dependencies and dd_cc_packaged."""
22

3+
load("@bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory_bin_action")
34
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_shared_library")
45
load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo")
56
load("@rules_pkg//pkg:mappings.bzl", "pkg_files")
6-
load("@rules_pkg//pkg:providers.bzl", "PackageFilegroupInfo")
7+
load("@rules_pkg//pkg:providers.bzl", "PackageFilegroupInfo", "PackageFilesInfo")
78
load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite")
89
load("@rules_testing//lib:truth.bzl", "matching")
910
load("@rules_testing//lib:util.bzl", "util")
@@ -60,6 +61,24 @@ _duplicate_destinations = rule(
6061
attrs = {"dep": attr.label(providers = [PackageFilegroupInfo])},
6162
)
6263

64+
def _tree_artifact_impl(ctx):
65+
tree = ctx.actions.declare_directory("tree_dir")
66+
placeholder = ctx.actions.declare_file("tree_artifact_placeholder/file")
67+
ctx.actions.write(placeholder, "")
68+
copy_to_directory_bin_action(
69+
ctx,
70+
name = ctx.label.name,
71+
dst = tree,
72+
copy_to_directory_bin = ctx.toolchains["@bazel_lib//lib:copy_to_directory_toolchain_type"].copy_to_directory_info.bin,
73+
files = [placeholder],
74+
)
75+
return DefaultInfo(files = depset([tree]))
76+
77+
_tree_artifact = rule(
78+
implementation = _tree_artifact_impl,
79+
toolchains = ["@bazel_lib//lib:copy_to_directory_toolchain_type"],
80+
)
81+
6382
# ── Test cases ───────────────────────────────────────────────────────────────
6483

6584
# Test 1: a cc_shared_library without a dd_cc_packaged wrapper has no
@@ -434,6 +453,42 @@ def _test_diamond_no_duplicates(name):
434453
def _test_diamond_no_duplicates_impl(env, target):
435454
_outputs_of(env, target).contains_exactly([])
436455

456+
# Test 10: installed_executables treats the dict value as a prefix for both
457+
# files and TreeArtifacts, so artifacts land under prefix/basename.
458+
def _test_installed_executables_use_prefix(name):
459+
cc_binary(
460+
name = name + "_bin",
461+
srcs = ["testdata/main.c"],
462+
)
463+
native.genrule(
464+
name = name + "_file",
465+
outs = ["installed_executable_file"],
466+
cmd = "touch $@",
467+
)
468+
_tree_artifact(
469+
name = name + "_tree",
470+
)
471+
dd_cc_packaged(
472+
name = name + "_packaged",
473+
input = ":" + name + "_bin",
474+
installed_executables = {
475+
":" + name + "_file": "bin",
476+
":" + name + "_tree": "share",
477+
},
478+
)
479+
analysis_test(
480+
name = name,
481+
impl = _test_installed_executables_use_prefix_impl,
482+
target = name + "_packaged_exec_files",
483+
)
484+
485+
def _test_installed_executables_use_prefix_impl(env, target):
486+
dest_src_map = target[PackageFilesInfo].dest_src_map
487+
env.expect.that_dict(dest_src_map).keys().contains_exactly([
488+
"bin/installed_executable_file",
489+
"share/tree_dir",
490+
])
491+
437492
# ── Suite ────────────────────────────────────────────────────────────────────
438493

439494
def dd_packaging_test_suite(name):
@@ -449,5 +504,6 @@ def dd_packaging_test_suite(name):
449504
_test_cc_binary_collected,
450505
_test_cc_binary_no_cc_shared_library_info,
451506
_test_diamond_no_duplicates,
507+
_test_installed_executables_use_prefix,
452508
],
453509
)
Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +0,0 @@
1-
exports_files([
2-
"macos.sh",
3-
"macos_dir.sh",
4-
])

bazel/rules/rewrite_rpath/rewrite_rpath.bzl

Lines changed: 30 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -2,100 +2,60 @@
22

33
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
44

5-
def patchelf_dir_action(ctx, input_dir, output_dir, rpath):
6-
"""Registers a patchelf action to rewrite the rpath of all shared libraries inside a directory.
5+
_RPATH_REWRITER_TOOLCHAIN = "//bazel/toolchains/rpath_rewriter"
76

8-
Args:
9-
ctx: the rule context.
10-
input_dir: the source directory artifact to patch.
11-
output_dir: the output directory artifact to write.
12-
rpath: the rpath string to set.
13-
"""
14-
toolchain = ctx.toolchains["@@//bazel/toolchains/patchelf:patchelf_toolchain_type"].patchelf
15-
patchelf = toolchain.label[DefaultInfo].files_to_run
16-
ctx.actions.run_shell(
17-
inputs = [input_dir],
18-
tools = [patchelf],
19-
outputs = [output_dir],
20-
# /. copies the contents of input rather than nesting it under output
21-
# (Bazel pre-creates output via declare_directory). chmod restores
22-
# owner-write so patchelf can rewrite files installed as 0555.
23-
command = (
24-
"cp -rL '{input}/.' '{output}' && " +
25-
"chmod -R u+w '{output}' && " +
26-
"find '{output}' -type f \\( -name '*.so' -o -name '*.so.*' \\) " +
27-
"-exec '{patchelf}' --set-rpath '{rpath}' --force-rpath {{}} \\;"
28-
).format(
29-
input = input_dir.path,
30-
output = output_dir.path,
31-
patchelf = patchelf.executable.path,
32-
rpath = rpath,
33-
),
34-
)
7+
def rewrite_rpaths(ctx, inputs, rpath):
8+
"""Creates actions to apply an rpath rewriter to files and TreeArtifacts.
359
36-
def otool_dir_action(ctx, input_dir, output_dir, rpath):
37-
"""Registers install_name_tool actions to rewrite the rpath of all dylibs inside a directory.
10+
The selected rpath rewriter toolchain provides separate tools for regular
11+
files and TreeArtifacts. A `None` tool means rpath rewriting is not
12+
applicable for that artifact kind on the target platform; in that case, the
13+
original input is returned unchanged and no copy action is registered.
3814
3915
Args:
4016
ctx: the rule context.
41-
input_dir: the source directory artifact to patch.
42-
output_dir: the output directory artifact to write.
43-
rpath: the rpath string to set.
44-
"""
45-
otool = ctx.toolchains["@@//bazel/toolchains/otool:otool_toolchain_type"].otool
46-
args = ctx.actions.args()
47-
args.add(ctx.file._script.path)
48-
args.add(ctx.executable._install_name_tool.path)
49-
args.add(otool.path)
50-
args.add(rpath)
51-
args.add(input_dir.path)
52-
args.add(output_dir.path)
53-
ctx.actions.run(
54-
inputs = [input_dir, ctx.file._script],
55-
tools = [ctx.executable._install_name_tool],
56-
outputs = [output_dir],
57-
executable = ctx.file._dir_script,
58-
arguments = [args],
59-
)
60-
61-
def rewrite_rpaths_for_files(ctx, inputs, rpath):
62-
"""Creates actions to apply an rpath rewriter to the inputs.
63-
64-
Args:
65-
ctx: the rule context.
66-
inputs: the files to patch.
17+
inputs: the files or TreeArtifacts to patch.
6718
rpath: the rpath to set.
6819
6920
Returns:
70-
A list of the generated outputs
21+
A list of rewritten outputs, in the same order as inputs. Rewritten
22+
artifacts preserve the input basename. On platforms where rewriting is a
23+
no-op, some or all entries may be the original inputs.
7124
"""
72-
toolchain = ctx.toolchains["//bazel/toolchains/rpath_rewriter"]
73-
74-
# No-op: just pass the inputs through.
75-
if toolchain.rewriter_tool == None:
76-
return inputs
25+
toolchain = ctx.toolchains[_RPATH_REWRITER_TOOLCHAIN]
7726

7827
outputs = []
7928
for input in inputs:
80-
output = ctx.actions.declare_file("patched/" + input.basename)
29+
tool = toolchain.tree_rewriter_tool if input.is_directory else toolchain.rewriter_tool
30+
31+
# No-op: just pass this input through.
32+
if tool == None:
33+
outputs.append(input)
34+
continue
35+
36+
if input.is_directory:
37+
output = ctx.actions.declare_directory("patched_dirs/" + input.basename)
38+
else:
39+
output = ctx.actions.declare_file("patched/" + input.basename)
40+
8141
args = ctx.actions.args()
82-
args.add(input)
42+
args.add(input.path)
8343
args.add(rpath)
84-
args.add(output)
44+
args.add(output.path)
8545
ctx.actions.run(
8646
inputs = [input],
8747
outputs = [output],
8848
arguments = [args],
89-
executable = toolchain.rewriter_tool,
90-
toolchain = "//bazel/toolchains/rpath_rewriter",
49+
executable = tool,
50+
toolchain = _RPATH_REWRITER_TOOLCHAIN,
9151
)
9252
outputs.append(output)
9353

9454
return outputs
9555

9656
def _rewrite_rpath_impl(ctx):
9757
rpath = ctx.attr.rpath.format(install_dir = ctx.attr._install_dir[BuildSettingInfo].value)
98-
return DefaultInfo(files = depset(rewrite_rpaths_for_files(ctx, inputs = ctx.files.inputs, rpath = rpath)))
58+
return DefaultInfo(files = depset(rewrite_rpaths(ctx, inputs = ctx.files.inputs, rpath = rpath)))
9959

10060
rewrite_rpath = rule(
10161
implementation = _rewrite_rpath_impl,
@@ -117,5 +77,5 @@ rewrite_rpath = rule(
11777
default = "@@//:install_dir",
11878
),
11979
},
120-
toolchains = ["//bazel/toolchains/rpath_rewriter"],
80+
toolchains = [_RPATH_REWRITER_TOOLCHAIN],
12181
)

bazel/rules/rewrite_rpath/tests/BUILD.bazel

Lines changed: 0 additions & 44 deletions
This file was deleted.

0 commit comments

Comments
 (0)