Skip to content

Commit

Permalink
Update cargo_build_script to work without runfiles support. (#2887)
Browse files Browse the repository at this point in the history
Partially addresses #1156

This pull-request implements an additional change to enable this
behavior which aggregates all transitive `BuildInfo.compile_data` into
`Rustc` actions. While this seems to bloat these actions with
unnecessary data, it addresses a catastrophic flaw in how Windows works
at all.

As of Bazel 7.3.1, Windows does not run any actions in a sandbox
(bazelbuild/bazel#18401), this means that
references to the current working directory will be consistent since
they always refer to the execroot. On top of this fact,
`cargo_build_script` will assign
[CARGO_MANIFEST_DIR](https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates)
to a [path within the runfiles directory of the build
script](https://github.com/bazelbuild/rules_rust/blame/d9ee6172d1dfc2801ba809441668e60e797f89de/cargo/private/cargo_build_script.bzl#L218).
The combination of these two facts leads crates like
[windows_x86_64_msvc](https://crates.io/crates/windows_x86_64_msvc),
which assign a linker path using `CARGO_MANIFEST_DIR`
([@windows_x86_64_msvc//build.rs](https://github.com/microsoft/windows-rs/blob/0.59.0/crates/targets/x86_64_msvc/build.rs#L1-L8))
to introduce un-tracked dependencies into dependent `Rustc` actions.
This then leads to build failures if the `windows_x86_64_msvc` crate is
ever a remote cache hit for the dependents as runfiles (or
`.cargo_runfiles` in the case of this PR) are not fetched and will not
exist on the host at link time.

This change addresses this issue of untracked dependencies by ensuring
the runfiles of `cargo_build_script.script` targets are aggregated into
`Rustc` actions.
  • Loading branch information
UebelAndre authored Sep 19, 2024
1 parent 6e97b2f commit 59464fe
Show file tree
Hide file tree
Showing 236 changed files with 616 additions and 24 deletions.
37 changes: 37 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ default_windows_targets: &default_windows_targets
- "//..."
- "-//test/proto/..."
- "-//test/unit/pipelined_compilation/..."
default_windows_no_runfiles_targets: &default_windows_no_runfiles_targets
- "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245
- "//..."
# TODO: https://github.com/bazelbuild/rules_rust/issues/1156
- "-//crate_universe/..."
- "-//test/chained_direct_deps:mod3_doc_test"
- "-//test/out_dir_in_tests:demo_lib_doc_test"
- "-//test/proto/..."
- "-//test/rustc_env_files:output_test"
- "-//test/test_env_launcher:test"
- "-//test/test_env:test_manifest_dir"
- "-//test/test_env:test_run"
- "-//test/unit/pipelined_compilation/..."
- "-//test/unit/rustdoc/..."
- "-//tools/runfiles/..."
crate_universe_vendor_example_targets: &crate_universe_vendor_example_targets
- "//vendor_external:crates_vendor"
- "//vendor_local_manifests:crates_vendor"
Expand Down Expand Up @@ -78,6 +93,15 @@ tasks:
windows:
build_targets: *default_windows_targets
test_targets: *default_windows_targets
windows_no_runfiles:
name: No Runfiles
platform: windows
build_flags:
- "--noenable_runfiles"
test_flags:
- "--noenable_runfiles"
build_targets: *default_windows_no_runfiles_targets
test_targets: *default_windows_no_runfiles_targets
ubuntu2004_split_coverage_postprocessing:
name: Split Coverage Postprocessing
platform: ubuntu2004
Expand Down Expand Up @@ -171,6 +195,19 @@ tasks:
- "--config=clippy"
build_targets: *default_windows_targets
test_targets: *default_windows_targets
windows_no_runfiles_with_aspects:
name: No Runfiles With Aspects
platform: windows
build_flags:
- "--noenable_runfiles"
- "--config=rustfmt"
- "--config=clippy"
test_flags:
- "--noenable_runfiles"
- "--config=rustfmt"
- "--config=clippy"
build_targets: *default_windows_no_runfiles_targets
test_targets: *default_windows_no_runfiles_targets
windows_rolling_with_aspects:
name: "Windows Rolling Bazel Version With Aspects"
platform: windows
Expand Down
1 change: 1 addition & 0 deletions bindgen/3rdparty/crates/BUILD.bindgen-0.70.1.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bindgen/3rdparty/crates/BUILD.clang-sys-1.8.1.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bindgen/3rdparty/crates/BUILD.libc-0.2.158.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bindgen/3rdparty/crates/BUILD.winapi-0.3.9.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions cargo/private/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load(":runfiles_enabled.bzl", "runfiles_enabled_build_setting")

bzl_library(
name = "bzl_lib",
srcs = glob(["**/*.bzl"]),
visibility = ["//:__subpackages__"],
)

runfiles_enabled_build_setting(
name = "runfiles_enabled",
visibility = ["//visibility:public"],
)
77 changes: 64 additions & 13 deletions cargo/private/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ load(
"find_toolchain",
_name_to_crate_name = "name_to_crate_name",
)
load(":runfiles_enabled.bzl", "is_runfiles_enabled", "runfiles_enabled_attr")

# Reexport for cargo_build_script_wrapper.bzl
name_to_crate_name = _name_to_crate_name
Expand Down Expand Up @@ -198,6 +199,38 @@ def _feature_enabled(ctx, feature_name, default = False):

return default

def _rlocationpath(file, workspace_name):
if file.short_path.startswith("../"):
return file.short_path[len("../"):]

return "{}/{}".format(workspace_name, file.short_path)

def _create_runfiles_dir(ctx, script):
runfiles_dir = ctx.actions.declare_directory("{}.cargo_runfiles".format(ctx.label.name))

# External repos always fall into the `../` branch of `_rlocationpath`.
workspace_name = ctx.workspace_name

def _runfiles_map(file):
return "{}={}".format(file.path, _rlocationpath(file, workspace_name))

runfiles = script[DefaultInfo].default_runfiles

args = ctx.actions.args()
args.use_param_file("@%s", use_always = True)
args.add(runfiles_dir.path)
args.add_all(runfiles.files, map_each = _runfiles_map, allow_closure = True)

ctx.actions.run(
mnemonic = "CargoBuildScriptRunfilesDir",
executable = ctx.executable._runfiles_maker,
arguments = [args],
inputs = runfiles.files,
outputs = [runfiles_dir],
)

return runfiles_dir

def _cargo_build_script_impl(ctx):
"""The implementation for the `cargo_build_script` rule.
Expand All @@ -208,16 +241,37 @@ def _cargo_build_script_impl(ctx):
list: A list containing a BuildInfo provider
"""
script = ctx.executable.script
script_info = ctx.attr.script[CargoBuildScriptRunfilesInfo]
toolchain = find_toolchain(ctx)
out_dir = ctx.actions.declare_directory(ctx.label.name + ".out_dir")
env_out = ctx.actions.declare_file(ctx.label.name + ".env")
dep_env_out = ctx.actions.declare_file(ctx.label.name + ".depenv")
flags_out = ctx.actions.declare_file(ctx.label.name + ".flags")
link_flags = ctx.actions.declare_file(ctx.label.name + ".linkflags")
link_search_paths = ctx.actions.declare_file(ctx.label.name + ".linksearchpaths") # rustc-link-search, propagated from transitive dependencies
manifest_dir = "%s.runfiles/%s/%s" % (script.path, ctx.label.workspace_name or ctx.workspace_name, ctx.label.package)
compilation_mode_opt_level = get_compilation_mode_opts(ctx, toolchain).opt_level

script_tools = []
script_data = []
for target in script_info.data:
script_data.append(target[DefaultInfo].files)
script_data.append(target[DefaultInfo].default_runfiles.files)
for target in script_info.tools:
script_tools.append(target[DefaultInfo].files)
script_tools.append(target[DefaultInfo].default_runfiles.files)

workspace_name = ctx.label.workspace_name
if not workspace_name:
workspace_name = ctx.workspace_name

if not is_runfiles_enabled(ctx.attr):
runfiles_dir = _create_runfiles_dir(ctx, ctx.attr.script)
script_data.append(depset([runfiles_dir]))
manifest_dir = "{}/{}/{}".format(runfiles_dir.path, workspace_name, ctx.label.package)
else:
script_data.append(ctx.attr.script[DefaultInfo].default_runfiles.files)
manifest_dir = "{}.runfiles/{}/{}".format(script.path, workspace_name, ctx.label.package)

streams = struct(
stdout = ctx.actions.declare_file(ctx.label.name + ".stdout.log"),
stderr = ctx.actions.declare_file(ctx.label.name + ".stderr.log"),
Expand Down Expand Up @@ -331,8 +385,6 @@ def _cargo_build_script_impl(ctx):
variables = getattr(target[platform_common.TemplateVariableInfo], "variables", depset([]))
env.update(variables)

script_info = ctx.attr.script[CargoBuildScriptRunfilesInfo]

_merge_env_dict(env, expand_dict_value_locations(
ctx,
ctx.attr.build_script_env,
Expand All @@ -343,15 +395,6 @@ def _cargo_build_script_impl(ctx):
script_info.tools,
))

script_tools = []
script_data = []
for target in script_info.data:
script_data.append(target[DefaultInfo].files)
script_data.append(target[DefaultInfo].default_runfiles.files)
for target in script_info.tools:
script_tools.append(target[DefaultInfo].files)
script_tools.append(target[DefaultInfo].default_runfiles.files)

tools = depset(
direct = [
script,
Expand Down Expand Up @@ -379,6 +422,7 @@ def _cargo_build_script_impl(ctx):
args.add(ctx.attr.rundir)

build_script_inputs = []

for dep in ctx.attr.link_deps:
if rust_common.dep_info in dep and dep[rust_common.dep_info].dep_env:
dep_env_file = dep[rust_common.dep_info].dep_env
Expand Down Expand Up @@ -520,7 +564,14 @@ cargo_build_script = rule(
"_experimental_symlink_execroot": attr.label(
default = Label("//cargo/settings:experimental_symlink_execroot"),
),
},
"_runfiles_maker": attr.label(
cfg = "exec",
executable = True,
default = Label("//cargo/private/runfiles_maker"),
),
} | runfiles_enabled_attr(
default = Label("//cargo/private:runfiles_enabled"),
),
fragments = ["cpp"],
toolchains = [
str(Label("//rust:toolchain_type")),
Expand Down
Loading

0 comments on commit 59464fe

Please sign in to comment.