Add $(location_dirname) template for resolving the directory of a label

[jmillikin]

Description of the feature request:

Allow $(location_dirname //some:target) to resolve to the directory that contains //some:target.

For example, if $(location //foo:bar) resolves to bazel-out/k8-fastbuild/bin/foo/bar.txt, then $(location_dirname //foo:bar) should resolve to bazel-out/k8-fastbuild/bin/foo.

Which category does this issue belong to?

Rules API

What underlying problem are you trying to solve with this feature?

This is useful when interacting with tools that write outputs to a named directory. Currently wrapping such a tool requires writing a custom build rule, but a $(location_dirname) template expanded by ctx.expand_location() would let them be wrapped in a genrule or Skylib's run_binary().

load("@bazel_skylib//rules:run_binary.bzl", "run_binary")

run_binary(
    name = "javacc_stage1_out",
    tool = "//bootstrap:javacc_stage1",
    srcs = ["src/main/javacc/JavaCC.jj"],
    outs = [
        "src/main/java/org/javacc/parser/JavaCCParser.java",
        "src/main/java/org/javacc/parser/JavaCCParserConstants.java"
    ],
    args = [
        # vvv New feature used here to specify the output directory vvv
        "-OUTPUT_DIRECTORY:$(location_dirname src/main/java/org/javacc/parser/JavaCCParser.java)",
        "$(location src/main/javacc/JavaCC.jj)",
    ],
)

[fmeum]

In the case of an output directory, you should be able to use this instead of a custom rule today:

"-OUTPUT_DIRECTORY:$(BINDIR)/{}/src/main/java/org/javacc/parser".format(package_name())

I would still like to see a way to support expansions to directory paths more easily, especially for multiple files.

[jmillikin]

I just tried the $(BINDIR) approach, and it doesn't work because @bazel_skylib//rules:run_binary.bzl only expands locations, not make variables.

run_binary(
    name = "javacc_java_jj",
    tool = "@javacc//:javacc",
    srcs = ["src/main/javacc/java.jj"],
    outs = [
        "GeneratedJavaParserConstants.java",
        "GeneratedJavaParser.java",
        "GeneratedJavaParserTokenManager.java",
    ],
    args = [
        "-OUTPUT_DIRECTORY:$(BINDIR)/javaparser-core",
        "$(location src/main/javacc/java.jj)",
    ],
)

(cd /home/john/.cache/bazel/_bazel_john/d1588efe31a7420c307ba88a28892df0/execroot/_main && \
  exec env - \
  bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/javacc~/javacc '-OUTPUT_DIRECTORY:$(BINDIR)/javaparser-core' javaparser-core/src/main/javacc/java.jj)

There is a comment to this effect within the skylib sources, so it seems like the behavior is intentional-ish:

# To keep the rule simple, do not expand Make Variables (like *_binary.args usually would).
# (We can add this feature later if users ask for it.)

[comius]

location_dirname makes sense.

Also expanding make variables in run_binary makes sense. If you prepare a PR I'll review it.

[jmillikin]

The proposed PR to Bazel is #23518 -- it adds $(execpath_dirname) and $(rootpath_dirname). After filing this issue, I discovered that $(location) appears to be deprecated in favor of $(execpath) and $(rootpath).

I'm hesitant to add make variable expansions in places where they're currently not enabled, because I've had builds break in the past from unexpected expansions. I'll leave the skylib changes to someone else who might have stronger preference for it.