Skip to content

Commit

Permalink
BREAKING: Modify path layout to fix relative RPATH lookups (#104)
Browse files Browse the repository at this point in the history
The executable of an executable transitioned rule is now created in a
subdirectory of the target's package, which allows it to match the path
depth under the execroot of the original executable and thus gets
relative RPATH entries to resolve.

Also copy any DLLs next to the new executable on Windows.

The new path layout can be disabled via
`--no@with_cfg.bzl//:incompatible_same_depth_path_layout`, e.g. if users
hardcoded rlocationpaths and don't want to update them now. It is
recommended to avoid this and use `$(rlocationpath ...)` instead.

Fixes #85
  • Loading branch information
fmeum authored Jul 24, 2024
1 parent b37e695 commit fdcaa33
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 9 deletions.
7 changes: 7 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")

bzl_library(
name = "with_cfg",
srcs = ["with_cfg.bzl"],
visibility = ["//visibility:public"],
deps = ["//with_cfg:defs"],
)

bool_flag(
name = "incompatible_same_depth_path_layout",
build_setting_default = True,
visibility = ["//visibility:public"],
)
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ module(
)

bazel_dep(name = "bazel_skylib", version = "1.4.1")
bazel_dep(name = "platforms", version = "0.0.6")

bazel_dep(name = "aspect_bazel_lib", version = "1.34.0", dev_dependency = True)
bazel_dep(name = "bazel_skylib_gazelle_plugin", version = "1.4.1", dev_dependency = True)
bazel_dep(name = "buildifier_prebuilt", version = "6.4.0", dev_dependency = True)
bazel_dep(name = "gazelle", version = "0.33.0", dev_dependency = True, repo_name = "bazel_gazelle")
bazel_dep(name = "platforms", version = "0.0.6", dev_dependency = True)
bazel_dep(name = "rules_testing", version = "0.4.0", dev_dependency = True)
bazel_dep(name = "stardoc", dev_dependency = True)
git_override(
Expand Down
3 changes: 3 additions & 0 deletions examples/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ startup --nowindows_enable_symlinks
# top-level configuration.
common --experimental_output_directory_naming_scheme=diff_against_dynamic_baseline

# Required with Bazel 6
common --experimental_cc_shared_library

# Prevent Java compiler warnings such as:
# [0.002s][warning][perf,memops] Cannot use file /tmp/hsperfdata_runner/2 because it is locked by another process (errno = 11)
common --incompatible_sandbox_hermetic_tmp
Expand Down
58 changes: 58 additions & 0 deletions examples/dynamic_deps/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
load("//dynamic_deps/rules:runfiles_dir.bzl", "runfiles_dir")
load(":cc_define_binary.bzl", "cc_define_binary")

cc_define_binary(
name = "bin",
srcs = ["bin.cpp"],
dynamic_deps = ["//dynamic_deps/lib:shared"],
deps = ["//dynamic_deps/lib"],
)

runfiles_dir(
name = "bin_runfiles",
executable = ":bin",
)

sh_test(
name = "bin_test_from_symlinked_runfiles",
srcs = ["bin_test.sh"],
args = ["$(rlocationpath :bin)"],
data = [":bin"],
deps = ["@bazel_tools//tools/bash/runfiles"],
)

sh_test(
name = "bin_test_direct",
srcs = ["bin_test.sh"],
args = select({
"@platforms//os:windows": ["$(rlocationpath :bin_runfiles)/bin.exe"],
"//conditions:default": ["$(rlocationpath :bin_runfiles)/bin"],
}),
data = [":bin_runfiles"],
deps = ["@bazel_tools//tools/bash/runfiles"],
target_compatible_with = select({
# TODO: Get this test to pass on Windows.
"@platforms//os:windows": ["@platforms//:incompatible"],
# TODO: Reenable on macOS after:
# 1. Bazel is released with the fix in https://github.com/bazelbuild/bazel/pull/23089.
# 2. Enabling --incompatible_macos_set_install_name.
# 3. Adding a dep on apple_support, as the default Unix toolchain only supports this flag
# after https://github.com/bazelbuild/bazel/pull/23090.
"@platforms//os:macos": ["@platforms//:incompatible"],
"//conditions:default": [],
}),
)

sh_test(
name = "bin_test_from_regular_runfiles",
srcs = ["bin_test.sh"],
args = select({
"@platforms//os:windows": ["$(rlocationpath :bin_runfiles)/bin.exe.runfiles/$(rlocationpath :bin)"],
"//conditions:default": ["$(rlocationpath :bin_runfiles)/bin.runfiles/$(rlocationpath :bin)"],
}),
data = [
":bin",
":bin_runfiles",
],
deps = ["@bazel_tools//tools/bash/runfiles"],
)
6 changes: 6 additions & 0 deletions examples/dynamic_deps/bin.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "dynamic_deps/lib/lib.h"

int main() {
PrintGreeting();
return 0;
}
21 changes: 21 additions & 0 deletions examples/dynamic_deps/bin_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/env bash

# --- begin runfiles.bash initialization v3 ---
# Copy-pasted from the Bazel Bash runfiles library v3.
set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash
# shellcheck disable=SC1090
source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
source "$0.runfiles/$f" 2>/dev/null || \
source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
{ echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
# --- end runfiles.bash initialization v3 ---

bin_path=$(rlocation "$1" || { >&2 echo "$1 not found"; exit 1; })

output=$("$bin_path" || { >&2 echo "Failed to run $bin_path"; exit 1; })
if [[ "$output" != "Hello,_world!" ]]; then
echo "Unexpected output: $output"
exit 1
fi
5 changes: 5 additions & 0 deletions examples/dynamic_deps/cc_define_binary.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
load("@with_cfg.bzl", "with_cfg")

_builder = with_cfg(native.cc_binary)
_builder.extend("copt", ["-DGREETING=\"Hello,_world!\""])
cc_define_binary, _cc_define_binary = _builder.build()
14 changes: 14 additions & 0 deletions examples/dynamic_deps/lib/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
cc_library(
name = "lib",
srcs = ["lib.cpp"],
hdrs = ["lib.h"],
visibility = ["//dynamic_deps:__pkg__"],
tags = ["manual"],
)

cc_shared_library(
name = "shared",
deps = [":lib"],
visibility = ["//dynamic_deps:__pkg__"],
tags = ["manual"],
)
8 changes: 8 additions & 0 deletions examples/dynamic_deps/lib/lib.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#define COMPILING_DLL
#include "lib.h"

#include <iostream>

void PrintGreeting() {
std::cout << GREETING << std::endl;
}
13 changes: 13 additions & 0 deletions examples/dynamic_deps/lib/lib.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#pragma once

#ifdef _WIN32
#ifdef COMPILING_DLL
#define DLLEXPORT __declspec(dllexport)
#else
#define DLLEXPORT __declspec(dllimport)
#endif
#else
#define DLLEXPORT
#endif

DLLEXPORT void PrintGreeting();
Empty file.
53 changes: 53 additions & 0 deletions examples/dynamic_deps/rules/runfiles_dir.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
def _runfiles_dir_impl(ctx):
# type: (ctx) -> list
out_dir = ctx.actions.declare_directory(ctx.label.name)
default_info = ctx.attr.executable[DefaultInfo]
executable = default_info.files_to_run.executable
name = executable.basename

args = ctx.actions.args()
args.add(out_dir.path)

ctx.actions.run_shell(
inputs = default_info.default_runfiles.files,
outputs = [out_dir],
command = """
mkdir -p {out_dir}/{name}.runfiles
cp {executable} {out_dir}/{name}
""".format(
name = name,
out_dir = out_dir.path,
executable = executable.path,
) + "\n".join([
"""
mkdir -p $(dirname {out_dir}/{name}.runfiles/{rlocationpath})
cp {path} {out_dir}/{name}.runfiles/{rlocationpath}
""".format(
name = name,
out_dir = out_dir.path,
path = f.path,
rlocationpath = _rlocationpath(ctx, f),
)
for f in default_info.default_runfiles.files.to_list()
]),
)

return [
DefaultInfo(files = depset([out_dir])),
]

def _rlocationpath(ctx, f):
if f.short_path.startswith("../"):
return f.short_path[3:]
else:
return ctx.workspace_name + "/" + f.short_path

runfiles_dir = rule(
implementation = _runfiles_dir_impl,
attrs = {
"executable": attr.label(
cfg = "target",
executable = True,
),
},
)
5 changes: 4 additions & 1 deletion with_cfg/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ bzl_library(
name = "frontend",
srcs = ["frontend.bzl"],
visibility = ["//with_cfg:__subpackages__"],
deps = [":providers"],
deps = [
":providers",
"@bazel_skylib//rules:common_settings",
],
)

bzl_library(
Expand Down
35 changes: 28 additions & 7 deletions with_cfg/private/frontend.bzl
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load(":providers.bzl", "FrontendInfo")

visibility("private")
Expand All @@ -11,18 +12,36 @@ def get_frontend(*, executable, test):
return _frontend_default

def _frontend_impl(ctx):
# type: (ctx) -> None
target = ctx.attr.exports

original_executable = target[FrontendInfo].executable
dirname, separator, _ = ctx.label.name.rpartition("/")
basename = original_executable.basename
executable = ctx.actions.declare_file(dirname + separator + basename)
executable_basename = original_executable.basename
incompatible_same_depth_path_layout = ctx.attr._incompatible_same_depth_path_layout[BuildSettingInfo].value
if incompatible_same_depth_path_layout:
# Create the executable in a subdirectory to ensure that its path depth below the exec root
# is the same as the original executable's. This is necessary to make relative RPATHS work.
executable = ctx.actions.declare_file(ctx.label.name + "/" + executable_basename)
else:
dirname, separator, _ = ctx.label.name.rpartition("/")
executable = ctx.actions.declare_file(dirname + separator + executable_basename)

additional_runfiles = [executable]
if CcInfo in target and ctx.target_platform_has_constraint(ctx.attr._windows[platform_common.ConstraintValueInfo]):
# DLLs need to be located next to the executable on Windows.
dlls_to_relocate = [
f
for f in target[DefaultInfo].default_runfiles.files.to_list()
if f.path.endswith(".dll") and f.dirname != executable.dirname and f.dirname == original_executable.dirname
]
for dll in dlls_to_relocate:
out_dll = ctx.actions.declare_file(dll.basename, sibling = executable)
ctx.actions.symlink(output = out_dll, target_file = dll)
additional_runfiles.append(out_dll)

# TODO: If this is a copy rather than a symlink, runfiles discovery will not work correctly.
# Fix this by using a wrapper script rather than a symlink.
ctx.actions.symlink(output = executable, target_file = original_executable)
data_runfiles = ctx.runfiles([executable]).merge(target[DefaultInfo].data_runfiles)
default_runfiles = ctx.runfiles([executable]).merge(target[DefaultInfo].default_runfiles)
data_runfiles = ctx.runfiles(additional_runfiles).merge(target[DefaultInfo].data_runfiles)
default_runfiles = ctx.runfiles(additional_runfiles).merge(target[DefaultInfo].default_runfiles)

run_environment_info = _clean_run_environment_info(
target[FrontendInfo].run_environment_info,
Expand Down Expand Up @@ -79,6 +98,8 @@ _frontend_attrs = {
mandatory = True,
providers = [FrontendInfo],
),
"_windows": attr.label(default = "@platforms//os:windows"),
"_incompatible_same_depth_path_layout": attr.label(default = "//:incompatible_same_depth_path_layout"),
}

_frontend_test_attrs = {
Expand Down

0 comments on commit fdcaa33

Please sign in to comment.