Skip to content

Commit

Permalink
refactor: remove output_dir from run_binary and expand_variables (#588)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan authored and alexeagle committed Dec 23, 2023
1 parent 9097087 commit 819f3c8
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 39 deletions.
3 changes: 1 addition & 2 deletions docs/expand_make_vars.md

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

3 changes: 1 addition & 2 deletions docs/run_binary.md

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

38 changes: 11 additions & 27 deletions lib/private/expand_variables.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

load("@bazel_skylib//lib:paths.bzl", _spaths = "paths")

def expand_variables(ctx, s, outs = [], output_dir = False, attribute_name = "args"):
def expand_variables(ctx, s, outs = [], attribute_name = "args"):
"""Expand make variables and substitute like genrule does.
Bazel [pre-defined variables](https://bazel.build/reference/be/make-variables#predefined_variables)
Expand Down Expand Up @@ -45,9 +45,6 @@ def expand_variables(ctx, s, outs = [], output_dir = False, attribute_name = "ar
ctx: starlark rule context
s: expression to expand
outs: declared outputs of the rule, for expanding references to outputs
output_dir: whether the rule is expected to output a directory (TreeArtifact)
Deprecated. For backward compatability with @aspect_bazel_lib 1.x. Pass
output tree artifacts to outs instead.
attribute_name: name of the attribute containing the expression. Used for error reporting.
Returns:
Expand All @@ -60,30 +57,17 @@ def expand_variables(ctx, s, outs = [], output_dir = False, attribute_name = "ar
)
additional_substitutions = {}

# TODO(2.0): remove output_dir in 2.x release
if output_dir:
if s.find("$@") != -1 or s.find("$(@)") != -1:
fail("$@ substitution may only be used with output_dir=False.")

# We'll write into a newly created directory named after the rule
output_dir = _spaths.join(
ctx.bin_dir.path,
ctx.label.workspace_root,
ctx.label.package,
ctx.label.name,
)
else:
if s.find("$@") != -1 or s.find("$(@)") != -1:
if len(outs) > 1:
fail("$@ substitution may only be used with a single out.")
if len(outs) == 1:
additional_substitutions["@"] = outs[0].path
if outs[0].is_directory:
output_dir = outs[0].path
else:
output_dir = outs[0].dirname
if s.find("$@") != -1 or s.find("$(@)") != -1:
if len(outs) > 1:
fail("$@ substitution may only be used with a single out.")
if len(outs) == 1:
additional_substitutions["@"] = outs[0].path
if outs[0].is_directory:
output_dir = outs[0].path
else:
output_dir = rule_dir
output_dir = outs[0].dirname
else:
output_dir = rule_dir

additional_substitutions["@D"] = output_dir
additional_substitutions["RULEDIR"] = rule_dir
Expand Down
9 changes: 1 addition & 8 deletions lib/private/run_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ def run_binary(
progress_message = None,
execution_requirements = None,
stamp = 0,
# TODO: remove output_dir in 2.x release
output_dir = False,
**kwargs):
"""Runs a binary as a build action.
Expand Down Expand Up @@ -185,11 +183,6 @@ def run_binary(
See https://docs.bazel.build/versions/main/be/common-definitions.html#common.tags for useful keys.
output_dir: If set to True then an output directory named the same as the target name
is added to out_dirs.
Deprecated. For backward compatability with @aspect_bazel_lib 1.x. Use out_dirs instead.
stamp: Whether to include build status files as inputs to the tool. Possible values:
- `stamp = 0` (default): Never include build status files as inputs to the tool.
Expand Down Expand Up @@ -221,7 +214,7 @@ def run_binary(
args = args,
env = env,
outs = outs,
out_dirs = out_dirs + ([name] if output_dir else []),
out_dirs = out_dirs,
mnemonic = mnemonic,
progress_message = progress_message,
execution_requirements = execution_requirements,
Expand Down

0 comments on commit 819f3c8

Please sign in to comment.