Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix genrule usage with runfiles #95

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fix genrule usage with runfiles #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sh/posix.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ def _sh_posix_make_variables_impl(ctx):
toolchain.sh_binaries_info,
)

executable = ctx.actions.declare_file(ctx.label.name)
default_info = mk_default_info_with_files_to_run(
ctx,
ctx.label.name,
executable,
toolchain.tool[DefaultInfo].files,
toolchain.tool[DefaultInfo].default_runfiles,
)
Expand Down
3 changes: 1 addition & 2 deletions sh/private/defs.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ def mk_template_variable_info(name, sh_binaries_info):
},
))

def mk_default_info_with_files_to_run(ctx, name, files, runfiles):
def mk_default_info_with_files_to_run(ctx, executable, files, runfiles):
# Create a dummy executable to trigger the generation of a FilesToRun
# provider which can be used in custom rules depending on this bundle to
# input the needed runfiles into build actions.
# This is a workaround for https://github.com/bazelbuild/bazel/issues/15486
executable = ctx.actions.declare_file(name)
ctx.actions.write(executable, "", is_executable = True)
return DefaultInfo(
executable = executable,
Expand Down
114 changes: 54 additions & 60 deletions sh/sh.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,37 @@ ShBinariesInfo = provider(

_WINDOWS_EXE_EXTENSIONS = [".exe", ".cmd", ".bat", ".ps1"]

def _sh_binaries_from_srcs(ctx, srcs, is_windows):
_POSIX_WRAPPER_TEMPLATE = """\
#!/bin/sh
if [[ -z ${{RUNFILES_MANIFEST_FILE+x}} && -z ${{RUNFILES_DIR+x}} ]]; then
if [[ -f "{main_executable}.runfiles_manifest" ]]; then
export RUNFILES_MANIFEST_FILE="{main_executable}.runfiles_manifest"
elif [[ -d "{main_executable}.runfiles" ]]; then
export RUNFILES_DIR="{main_executable}.runfiles"
else
echo "ERROR: Runfiles not found for bundle {main_executable}" >&2
exit 1
fi
fi
exec "{original_executable}" "$@"
"""

_WINDOWS_WRAPPER_TEMPLATE = """\
@echo off
if not defined RUNFILES_MANIFEST_FILE if not defined RUNFILES_DIR (
if exist "{main_executable}.runfiles_manifest" (
set RUNFILES_MANIFEST_FILE="{main_executable}.runfiles_manifest"
) else if exist "{main_executable}.runfiles" (
set RUNFILES_DIR="{main_executable}.runfiles"
) else (
echo ERROR: Runfiles not found for bundle {main_executable} >&2
exit /b 1
)
)
"{original_executable}" %*
"""

def _sh_binaries_from_srcs(ctx, srcs, is_windows, main_executable):
executable_files = []
runfiles = ctx.runfiles()
executables_dict = dict()
Expand All @@ -27,10 +57,10 @@ def _sh_binaries_from_srcs(ctx, srcs, is_windows):
if src[DefaultInfo].files_to_run == None or src[DefaultInfo].files_to_run.executable == None:
fail("srcs must be executable, but '{}' is not.".format(src.label))

executable = src[DefaultInfo].files_to_run.executable
name = executable.basename
original_executable = src[DefaultInfo].files_to_run.executable
name = original_executable.basename
if is_windows:
(noext, ext) = paths.split_extension(executable.basename)
(noext, ext) = paths.split_extension(original_executable.basename)
if ext in _WINDOWS_EXE_EXTENSIONS:
name = noext

Expand All @@ -41,8 +71,21 @@ def _sh_binaries_from_srcs(ctx, srcs, is_windows):
src.label,
))

if src[DefaultInfo].default_runfiles:
executable = ctx.actions.declare_file(ctx.label.name + ".path/" + (name + ".bat" if is_windows else name))
ctx.actions.write(
executable,
(_WINDOWS_WRAPPER_TEMPLATE if is_windows else _POSIX_WRAPPER_TEMPLATE).format(
main_executable = main_executable.path,
original_executable = original_executable.path,
),
is_executable = True,
)
executable_files.append(original_executable)
runfiles = runfiles.merge(src[DefaultInfo].default_runfiles)
else:
executable = original_executable
executable_files.append(executable)
runfiles = runfiles.merge(src[DefaultInfo].default_runfiles)
executables_dict[name] = executable
executable_paths.append(executable.dirname)

Expand All @@ -64,6 +107,9 @@ def _sh_binaries_from_deps(ctx, deps):
fail("deps must be sh_binaries targets, but '{}' is not.".format(dep.label))

executable_files.append(dep[DefaultInfo].files)
# TODO: Handle tools with runfiles in deps correctly. They need to be wrapped in a new
# wrapper script as in _sh_binaries_from_srcs since the dummy executable they reference
# is no longer the sibling of the runfiles tree.
runfiles = runfiles.merge(dep[DefaultInfo].default_runfiles)
executables_dict.update(dep[ShBinariesInfo].executables)
executable_paths.append(dep[ShBinariesInfo].paths)
Expand Down Expand Up @@ -101,15 +147,16 @@ def _mk_sh_binaries_info(direct, transitive):

def _sh_binaries_impl(ctx):
is_windows = ctx.attr._is_windows[ConstantInfo].value
direct = _sh_binaries_from_srcs(ctx, ctx.attr.srcs, is_windows)
executable = ctx.actions.declare_file(ctx.label.name)
direct = _sh_binaries_from_srcs(ctx, ctx.attr.srcs, is_windows, executable)
transitive = _sh_binaries_from_deps(ctx, ctx.attr.deps)
data_runfiles = _runfiles_from_data(ctx, ctx.attr.data)

sh_binaries_info = _mk_sh_binaries_info(direct, transitive)
template_variable_info = mk_template_variable_info(ctx.label.name, sh_binaries_info)
default_info = mk_default_info_with_files_to_run(
ctx,
ctx.label.name,
executable,
depset(direct = direct.executable_files, transitive = transitive.executable_files),
direct.runfiles.merge(transitive.runfiles).merge(data_runfiles),
)
Expand Down Expand Up @@ -283,58 +330,5 @@ def _custom_rule_impl(ctx):
...
)
```

*Caveat: Using Binaries that require Runfiles*

Note, support for binaries that require runfiles is limited due to limitations
imposed by Bazel's Starlark API, see [#15486][issue-15486]. In order for these
to work you will need to define the `RUNFILES_DIR` or `RUNFILES_MANIFEST_FILE`
environment variables for the action using tools from the bundle.

(Use `RUNFILES_MANIFEST_FILE` if your operating system and configuration does
not support a runfiles tree and instead only provides a runfiles manifest file,
as is commonly the case on Windows.)

You can achieve this in a `genrule` as follows:

```bzl
genrule(
name = "runfiles-genrule",
toolchains = [":a-bundle"],
cmd = "\\n".join([
# The explicit RUNFILES_DIR/RUNFILES_MANIFEST_FILE is a workaround for
# https://github.com/bazelbuild/bazel/issues/15486
"RUNFILES_DIR=$(execpath :a-bundle).runfiles",
"RUNFILES_MANIFEST_FILE=$(execpath :a-bundle).runfiles_manifest",
"$(A_BUNDLE_BINARY_A)",
"$(A_BUNDLE_BINARY_B)",
]),
outs = [...],
)
```

And in a custom rule as follows:

```bzl
def _custom_rule_impl(ctx):
tools = ctx.attr.tools[ShBinariesInfo]
(tools_inputs, tools_manifest) = ctx.resolve_tools(tools = [ctx.attr.tools])
# The explicit RUNFILES_DIR/RUNFILES_MANIFEST_FILE is a workaround for
# https://github.com/bazelbuild/bazel/issues/15486
tools_env = {
"RUNFILES_DIR": ctx.attr.tools[DefaultInfo].files_to_run.runfiles_manifest.dirname,
"RUNFILES_MANIFEST_FILE": ctx.attr.tools[DefaultInfo].files_to_run.runfiles_manifest.path,
}

ctx.actions.run(
executable = tools.executables["binary-a"],
env = tools_env, # Pass the environment into the action.
inputs = tools_inputs,
input_manifests = tools_manifest,
...
)
```

[issue-15486]: https://github.com/bazelbuild/bazel/issues/15486
""",
)
22 changes: 2 additions & 20 deletions tests/sh_binaries/sh_binaries_test.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,29 +571,11 @@ def _test_genrule():
],
cmd = """\
$(GENRULE_BUNDLE_HELLO_WORLD) >$(execpath genrule_output_world)

IS_WINDOWS=
case "$$OSTYPE" in
cygwin|msys|win32) IS_WINDOWS=1;;
esac

with_runfiles() {
# The explicit RUNFILES_DIR|RUNFILES_MANIFEST_FILE is a workaround for
# https://github.com/bazelbuild/bazel/issues/15486
if [[ -n $$IS_WINDOWS ]]; then
RUNFILES_MANIFEST_FILE=$(execpath :genrule_bundle).runfiles_manifest \\
eval "$$@"
else
RUNFILES_DIR=$(execpath :genrule_bundle).runfiles \\
eval "$$@"
fi
}

with_runfiles $(GENRULE_BUNDLE_HELLO_DATA) >$(execpath genrule_output_data)
$(GENRULE_BUNDLE_HELLO_DATA) >$(execpath genrule_output_data)

PATH="$(_GENRULE_BUNDLE_PATH):$$PATH"
hello_world >$(execpath genrule_output_by_path)
with_runfiles hello_data >>$(execpath genrule_output_by_path)
hello_data >>$(execpath genrule_output_by_path)
""",
toolchains = [":genrule_bundle"],
)
Expand Down
Loading