refactor: remove output_dir from run_binary and expand_variables (#588)

docs/expand_make_vars.md

@@ -54,7 +54,7 @@ The expanded path or the original path
 ## expand_variables
 
 <pre>
-expand_variables(<a href="#expand_variables-ctx">ctx</a>, <a href="#expand_variables-s">s</a>, <a href="#expand_variables-outs">outs</a>, <a href="#expand_variables-output_dir">output_dir</a>, <a href="#expand_variables-attribute_name">attribute_name</a>)
+expand_variables(<a href="#expand_variables-ctx">ctx</a>, <a href="#expand_variables-s">s</a>, <a href="#expand_variables-outs">outs</a>, <a href="#expand_variables-attribute_name">attribute_name</a>)
 </pre>
 
 Expand make variables and substitute like genrule does.
@@ -104,7 +104,6 @@ for more information of how these special variables are expanded.
 | <a id="expand_variables-ctx"></a>ctx |  starlark rule context   |  none |
 | <a id="expand_variables-s"></a>s |  expression to expand   |  none |
 | <a id="expand_variables-outs"></a>outs |  declared outputs of the rule, for expanding references to outputs   |  <code>[]</code> |
-| <a id="expand_variables-output_dir"></a>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.   |  <code>False</code> |
 | <a id="expand_variables-attribute_name"></a>attribute_name |  name of the attribute containing the expression. Used for error reporting.   |  <code>"args"</code> |
 
 **RETURNS**

docs/run_binary.md

@@ -11,7 +11,7 @@ This fork of bazel-skylib's run_binary adds directory output support and better
 
 <pre>
 run_binary(<a href="#run_binary-name">name</a>, <a href="#run_binary-tool">tool</a>, <a href="#run_binary-srcs">srcs</a>, <a href="#run_binary-args">args</a>, <a href="#run_binary-env">env</a>, <a href="#run_binary-outs">outs</a>, <a href="#run_binary-out_dirs">out_dirs</a>, <a href="#run_binary-mnemonic">mnemonic</a>, <a href="#run_binary-progress_message">progress_message</a>,
-           <a href="#run_binary-execution_requirements">execution_requirements</a>, <a href="#run_binary-stamp">stamp</a>, <a href="#run_binary-output_dir">output_dir</a>, <a href="#run_binary-kwargs">kwargs</a>)
+           <a href="#run_binary-execution_requirements">execution_requirements</a>, <a href="#run_binary-stamp">stamp</a>, <a href="#run_binary-kwargs">kwargs</a>)
 </pre>
 
 Runs a binary as a build action.
@@ -35,7 +35,6 @@ This rule does not require Bash (unlike `native.genrule`).
 | <a id="run_binary-progress_message"></a>progress_message |  Progress message to show to the user during the build, for example, "Compiling foo.cc to create foo.o". The message may contain %{label}, %{input}, or %{output} patterns, which are substituted with label string, first input, or output's path, respectively. Prefer to use patterns instead of static strings, because the former are more efficient.   |  <code>None</code> |
 | <a id="run_binary-execution_requirements"></a>execution_requirements |  Information for scheduling the action.<br><br>For example,<br><br><pre><code> execution_requirements = {     "no-cache": "1", }, </code></pre><br><br>See https://docs.bazel.build/versions/main/be/common-definitions.html#common.tags for useful keys.   |  <code>None</code> |
 | <a id="run_binary-stamp"></a>stamp |  Whether to include build status files as inputs to the tool. Possible values:<br><br>- <code>stamp = 0</code> (default): Never include build status files as inputs to the tool.     This gives good build result caching.     Most tools don't use the status files, so including them in <code>--stamp</code> builds makes those     builds have many needless cache misses.     (Note: this default is different from most rules with an integer-typed <code>stamp</code> attribute.) - <code>stamp = 1</code>: Always include build status files as inputs to the tool, even in     [--nostamp](https://docs.bazel.build/versions/main/user-manual.html#flag--stamp) builds.     This setting should be avoided, since it is non-deterministic.     It potentially causes remote cache misses for the target and     any downstream actions that depend on the result. - <code>stamp = -1</code>: Inclusion of build status files as inputs is controlled by the     [--[no]stamp](https://docs.bazel.build/versions/main/user-manual.html#flag--stamp) flag.     Stamped targets are not rebuilt unless their dependencies change.<br><br>When stamping is enabled, an additional two environment variables will be set for the action:     - <code>BAZEL_STABLE_STATUS_FILE</code>     - <code>BAZEL_VOLATILE_STATUS_FILE</code><br><br>These files can be read and parsed by the action, for example to pass some values to a linker.   |  <code>0</code> |
-| <a id="run_binary-output_dir"></a>output_dir |  If set to True then an output directory named the same as the target name is added to out_dirs.<br><br>Deprecated. For backward compatability with @aspect_bazel_lib 1.x. Use out_dirs instead.   |  <code>False</code> |
 | <a id="run_binary-kwargs"></a>kwargs |  Additional arguments   |  none |
 
 

lib/private/expand_variables.bzl

@@ -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)
@@ -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:
@@ -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 s.find("$@") != -1 or s.find("$(@)") != -1:
-    if output_dir:
+        if len(outs) > 1:
-        if s.find("$@") != -1 or s.find("$(@)") != -1:
+            fail("$@ substitution may only be used with a single out.")
-            fail("$@ substitution may only be used with output_dir=False.")
+    if len(outs) == 1:
-
+        additional_substitutions["@"] = outs[0].path
-        # We'll write into a newly created directory named after the rule
+        if outs[0].is_directory:
-        output_dir = _spaths.join(
+            output_dir = outs[0].path
-            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
         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

lib/private/run_binary.bzl

@@ -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.
 
@@ -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.
@@ -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,