bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
388 stars 178 forks source link

run_binary expands $(locations …) to a single string instead of a list #319

Open phst opened 2 years ago

phst commented 2 years ago

Example:

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "bazel_skylib",
    sha256 = "fc64d71583f383157e3e5317d24e789f942bc83c76fde7e5981cadc097a3c3cc",
    strip_prefix = "bazel-skylib-1.1.1/",
    urls = [
        "https://github.com/bazelbuild/bazel-skylib/archive/refs/tags/1.1.1.zip",  # 2021-09-27
    ],
)

BUILD:

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

sh_binary(
    name = "cat",
    srcs = ["cat.sh"],
)

filegroup(
    name = "files",
    srcs = [
        "a.txt",
        "b.txt",
    ],
)

run_binary(
    name = "run_cat",
    srcs = [":files"],
    outs = ["cat.out"],
    args = [
        "$(location :cat.out)",
        "$(locations :files)",
    ],
    tool = ":cat",
)

cat.sh:

#!/bin/bash
out="$1"
shift
cat -- "$@" > "${out:?}"

a.txt and b.txt are arbitrary source files.


Running bazel build //:cat.out will then result in an error message:

cat: './a.txt ./b.txt': No such file or directory

This is because $(locations …) expands to a single string.

Not sure whether this can be fixed in Skylib alone – probably needs a Bazel/Starlark change for a variant of ctx.expand_location that works on an Args object.

tetromino commented 2 years ago

Clearly this is a bug: ctx.expand_location returns a string containing a list of shell-quoted paths joined with spaces, but skylib assumes that shell tokenization is not needed (see comment in _impl in rules/run_binary.bzl).

The question is how best to fix it.