Make binary rules run from workspace (#162)
Previously these just ran with normal `bazel run` locations. This is
do-able with bazel's `--run_under` or the new `--run_in_cwd` but I think
it makes sense to just make it work for these tools.
Fixes https://github.com/keith/buildifier-prebuilt/issues/9
diff --git a/BUILD b/BUILD
index b124b8b..be09df6 100644
--- a/BUILD
+++ b/BUILD
@@ -2,6 +2,8 @@
exports_files(
[
+ "binary_runner.bash.template",
+ "binary_runner.bat.template",
"runner.bash.template",
"runner.bat.template",
"WORKSPACE",
@@ -74,6 +76,8 @@
"BUILD",
"MODULE.bazel",
"WORKSPACE",
+ "binary_runner.bash.template",
+ "binary_runner.bat.template",
"runner.bash.template",
"runner.bat.template",
"//buildifier:all_files",
diff --git a/binary_runner.bash.template b/binary_runner.bash.template
new file mode 100644
index 0000000..a4ff078
--- /dev/null
+++ b/binary_runner.bash.template
@@ -0,0 +1,61 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+TOOL_NAME='{TOOL_NAME}'
+TOOL_SHORT_PATH='{TOOL_SHORT_PATH}'
+
+function resolve_runfile() {
+ local path=$1
+ local runfiles_path=${path#../}
+ local candidate
+ local manifest
+ local line
+
+ if [[ -e "$path" ]]; then
+ realpath "$path"
+ return 0
+ fi
+
+ if [[ -n "${RUNFILES_DIR:-}" ]]; then
+ candidate="${RUNFILES_DIR}/${runfiles_path}"
+ if [[ -e "$candidate" ]]; then
+ realpath "$candidate"
+ return 0
+ fi
+ fi
+
+ for manifest in \
+ "${RUNFILES_MANIFEST_FILE:-}" \
+ "$0.runfiles_manifest" \
+ "$0.exe.runfiles_manifest" \
+ "$0.runfiles/MANIFEST"; do
+ [[ -n "$manifest" && -f "$manifest" ]] || continue
+ while IFS= read -r line; do
+ if [[ "$line" == "${runfiles_path} "* ]]; then
+ printf '%s\n' "${line#* }"
+ return 0
+ elif [[ "$line" == "$runfiles_path" ]]; then
+ printf '%s\n' "$runfiles_path"
+ return 0
+ fi
+ done < "$manifest"
+ done
+
+ return 1
+}
+
+if ! tool_path=$(resolve_runfile "$TOOL_SHORT_PATH"); then
+ echo "Unable to locate $TOOL_NAME runfile: $TOOL_SHORT_PATH" >&2
+ exit 1
+fi
+
+working_directory=${BUILD_WORKING_DIRECTORY:-${BUILD_WORKSPACE_DIRECTORY:-}}
+if [[ -n "$working_directory" ]]; then
+ if ! cd "$working_directory"; then
+ echo "Unable to change working directory: $working_directory" >&2
+ exit 1
+ fi
+fi
+
+exec "$tool_path" "$@"
diff --git a/binary_runner.bat.template b/binary_runner.bat.template
new file mode 100644
index 0000000..0642fb5
--- /dev/null
+++ b/binary_runner.bat.template
@@ -0,0 +1,44 @@
+@echo off
+setlocal EnableDelayedExpansion
+
+set "TOOL_NAME={TOOL_NAME}"
+set "TOOL_FILENAME={TOOL_FILENAME}"
+set "TOOL_SHORT_PATH={TOOL_SHORT_PATH}"
+
+REM When runfiles are enabled, the tool is available through its short path.
+set "tool_path=%TOOL_SHORT_PATH:/=\%"
+for %%I in ("!tool_path!") do set "tool_path=%%~fI"
+
+REM Otherwise, locate the tool through the runfiles manifest.
+if not exist "!tool_path!" (
+ set "manifest_path=%RUNFILES_MANIFEST_FILE%"
+ if defined manifest_path if not exist "!manifest_path!" set "manifest_path="
+ if not defined manifest_path if exist "%~f0.runfiles_manifest" set "manifest_path=%~f0.runfiles_manifest"
+ if not defined manifest_path if exist "%~f0.exe.runfiles_manifest" set "manifest_path=%~f0.exe.runfiles_manifest"
+ if not defined manifest_path if exist "MANIFEST" set "manifest_path=MANIFEST"
+ if not defined manifest_path if exist "..\MANIFEST" set "manifest_path=..\MANIFEST"
+
+ if defined manifest_path (
+ for /F "tokens=1,* delims= " %%F IN ('findstr /l /c:"!TOOL_FILENAME!" "!manifest_path!"') DO (
+ set "tool_path=%%G"
+ )
+ )
+)
+
+if not exist "!tool_path!" (
+ echo Unable to locate !TOOL_NAME! runfile: !TOOL_SHORT_PATH! 1>&2
+ exit /b 1
+)
+
+set "working_directory=%BUILD_WORKSPACE_DIRECTORY%"
+if defined BUILD_WORKING_DIRECTORY set "working_directory=%BUILD_WORKING_DIRECTORY%"
+if defined working_directory (
+ cd /d "!working_directory!"
+ if errorlevel 1 (
+ echo Unable to change working directory: !working_directory! 1>&2
+ exit /b 1
+ )
+)
+
+"!tool_path!" %*
+exit /b !ERRORLEVEL!
diff --git a/buildifier/buildifier_binary.bzl b/buildifier/buildifier_binary.bzl
index 8213df8..753dc93 100644
--- a/buildifier/buildifier_binary.bzl
+++ b/buildifier/buildifier_binary.bzl
@@ -6,11 +6,19 @@
load("@platforms//host:constraints.bzl", "HOST_CONSTRAINTS")
def _buildifier_binary(ctx):
- buildifier = ctx.toolchains["@buildifier_prebuilt//buildifier:toolchain"]._tool
- script = ctx.actions.declare_file("buildifier")
- ctx.actions.symlink(
+ toolchain = ctx.toolchains["@buildifier_prebuilt//buildifier:toolchain"]
+ buildifier = toolchain._tool
+ runner = toolchain._binary_runner
+ out_ext = ".bash" if runner.label.name.endswith(".bash.template") else ".bat"
+ script = ctx.actions.declare_file(ctx.label.name + out_ext)
+ ctx.actions.expand_template(
+ template = runner.files.to_list()[0],
output = script,
- target_file = buildifier,
+ substitutions = {
+ "{TOOL_NAME}": "buildifier",
+ "{TOOL_FILENAME}": buildifier.basename,
+ "{TOOL_SHORT_PATH}": buildifier.short_path,
+ },
is_executable = True,
)
diff --git a/buildozer/buildozer_binary.bzl b/buildozer/buildozer_binary.bzl
index 25c56e6..87ff031 100644
--- a/buildozer/buildozer_binary.bzl
+++ b/buildozer/buildozer_binary.bzl
@@ -6,11 +6,19 @@
load("@platforms//host:constraints.bzl", "HOST_CONSTRAINTS")
def _buildozer_binary(ctx):
- buildozer = ctx.toolchains["@buildifier_prebuilt//buildozer:toolchain"]._tool
- script = ctx.actions.declare_file("buildozer")
- ctx.actions.symlink(
+ toolchain = ctx.toolchains["@buildifier_prebuilt//buildozer:toolchain"]
+ buildozer = toolchain._tool
+ runner = toolchain._binary_runner
+ out_ext = ".bash" if runner.label.name.endswith(".bash.template") else ".bat"
+ script = ctx.actions.declare_file(ctx.label.name + out_ext)
+ ctx.actions.expand_template(
+ template = runner.files.to_list()[0],
output = script,
- target_file = buildozer,
+ substitutions = {
+ "{TOOL_NAME}": "buildozer",
+ "{TOOL_FILENAME}": buildozer.basename,
+ "{TOOL_SHORT_PATH}": buildozer.short_path,
+ },
is_executable = True,
)
diff --git a/examples/modify_example_test_runner.sh b/examples/modify_example_test_runner.sh
index 568a1c7..be2e711 100755
--- a/examples/modify_example_test_runner.sh
+++ b/examples/modify_example_test_runner.sh
@@ -45,10 +45,12 @@
# Run the buildifier rule check.
"${bazel}" run //:buildifier.check || fail "Expected //:buildifier.check to succeed."
-# # Execute buildozer command
-# "${bazel}" run -- @buildifier_prebuilt//:buildozer \
-# 'comment Comment\ added\ by\ test.' //:buildifier.check || \
-# fail "Expected buildozer command to succeed."
+# Execute buildozer and verify that it modifies the source workspace.
+"${bazel}" run -- @buildifier_prebuilt//:buildozer \
+ 'comment Comment\ added\ by\ test.' //:buildifier.check || \
+ fail "Expected buildozer command to succeed."
+grep -Fq '# Comment added by test.' BUILD || \
+ fail "Expected buildozer to modify the workspace BUILD file."
# Execute buildifier directly
"${bazel}" run -- @buildifier_prebuilt//:buildifier -r "${PWD}" || \
@@ -57,4 +59,3 @@
# Ensure that the workspace works properly
"${bazel}" test //... || fail "Expected tests to succeed in the scratch directory."
-
diff --git a/tests/binary_runner/BUILD b/tests/binary_runner/BUILD
new file mode 100644
index 0000000..75cef31
--- /dev/null
+++ b/tests/binary_runner/BUILD
@@ -0,0 +1,14 @@
+load("@rules_shell//shell:sh_test.bzl", "sh_test")
+
+sh_test(
+ name = "binary_runner_test",
+ srcs = ["binary_runner_test.sh"],
+ args = [
+ "$(location //:buildifier)",
+ "$(location //:buildozer)",
+ ],
+ data = [
+ "//:buildifier",
+ "//:buildozer",
+ ],
+)
diff --git a/tests/binary_runner/binary_runner_test.sh b/tests/binary_runner/binary_runner_test.sh
new file mode 100755
index 0000000..770d652
--- /dev/null
+++ b/tests/binary_runner/binary_runner_test.sh
@@ -0,0 +1,68 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+buildifier=$(realpath "$1")
+buildozer=$(realpath "$2")
+workspace="${TEST_TMPDIR}/workspace"
+working_directory="${workspace}/pkg"
+decoy="${TEST_TMPDIR}/decoy"
+
+mkdir -p "$working_directory" "$decoy"
+
+cat >"${workspace}/MODULE.bazel" <<'EOF'
+module(name = "binary_runner_test")
+EOF
+
+cat >"${workspace}/BUILD" <<'EOF'
+filegroup(
+ name = "workspace_target",
+)
+EOF
+
+cat >"${working_directory}/BUILD" <<'EOF'
+filegroup(
+ name = "working_target",
+)
+EOF
+
+cat >"${working_directory}/working_only.bzl" <<'EOF'
+WORKING_ONLY = True
+EOF
+
+# A workspace marker here makes a raw buildtools binary treat this directory as
+# the workspace root. The runner must instead use the Bazel working-directory
+# environment variables.
+touch "${decoy}/MODULE.bazel"
+cd "$decoy"
+
+case "$(uname -s)" in
+CYGWIN* | MINGW32* | MSYS* | MINGW*)
+ BUILD_WORKSPACE_DIRECTORY=$(cygpath -w "$workspace")
+ BUILD_WORKING_DIRECTORY=$(cygpath -w "$working_directory")
+ ;;
+*)
+ BUILD_WORKSPACE_DIRECTORY=$workspace
+ BUILD_WORKING_DIRECTORY=$working_directory
+ ;;
+esac
+export BUILD_WORKING_DIRECTORY BUILD_WORKSPACE_DIRECTORY
+
+# BUILD_WORKING_DIRECTORY takes precedence over BUILD_WORKSPACE_DIRECTORY.
+"$buildifier" -mode=check working_only.bzl
+
+output=$("$buildozer" 'print name' :all)
+if [[ "$output" != "working_target" ]]; then
+ echo "Unexpected buildozer output from working directory: $output" >&2
+ exit 1
+fi
+
+# BUILD_WORKSPACE_DIRECTORY is used when BUILD_WORKING_DIRECTORY is absent.
+unset BUILD_WORKING_DIRECTORY
+"$buildifier" -mode=check BUILD
+
+output=$("$buildozer" 'print name' //:all)
+if [[ "$output" != "workspace_target" ]]; then
+ echo "Unexpected buildozer output from workspace directory: $output" >&2
+ exit 1
+fi
diff --git a/toolchain.bzl b/toolchain.bzl
index ec73ad2..0cc37aa 100644
--- a/toolchain.bzl
+++ b/toolchain.bzl
@@ -7,6 +7,7 @@
def _buildifier_toolchain(ctx):
return [
platform_common.ToolchainInfo(
+ _binary_runner = ctx.attr.binary_runner,
_tool = ctx.executable.tool,
_runner = ctx.attr.runner,
),
@@ -15,6 +16,11 @@
prebuilt_toolchain = rule(
_buildifier_toolchain,
attrs = {
+ "binary_runner": attr.label(
+ allow_single_file = True,
+ mandatory = True,
+ doc = "The templated runner script for invoking the tool directly",
+ ),
"tool": attr.label(
allow_single_file = True,
mandatory = True,
@@ -42,10 +48,12 @@
"""
name = buildtools.create_unique_name(name = tool_name, platform = os, arch = arch)
+ binary_runner = "@buildifier_prebuilt//:binary_runner.bat.template" if os == "windows" else "@buildifier_prebuilt//:binary_runner.bash.template"
buildifier_runner = "@buildifier_prebuilt//:runner.bat.template" if os == "windows" else "@buildifier_prebuilt//:runner.bash.template"
prebuilt_toolchain(
name = name,
+ binary_runner = binary_runner,
tool = tool,
runner = buildifier_runner if tool_name == "buildifier" else None,
)
