Skip to content

Commit e942d94

Browse files
authored
Tweaks to scalafmt rules/script to support bazel test wrappers (#1344)
With these changes, I can use a macro to place a wrapping `sh_test` target around this script, and change the phase ordering to put `phase_scalafmt` before the `runfiles` phase, and suddenly it works as intended! NB: If this change is acceptable to folks, then it might be worth changing the default phase placement for `ext_scalafmt` to place it before `runfiles` by default. Example use: In custom bzl file, I have: ``` _enable_format_targets_and_tests_by_default = True _ext_scalafmt = dicts.add( _base_ext_scalafmt, {"attrs": dicts.add( _base_ext_scalafmt["attrs"], {"format": attr.bool( default = _enable_format_targets_and_tests_by_default, doc = "Enable the check-format and auto-format synthetic run targets (foo.format-test and foo.format respectively)", )}, )}, {"phase_providers": [ "@//path/to/some/buildfile:phase_scalafmt", ]}, ) scala_library = make_scala_library(_ext_scalafmt) # etc for other rule types def maybe_add_format_test(**kwargs): if (kwargs.get("format", _enable_format_targets_and_tests_by_default)): name = kwargs["name"] sh_test( name = "{}.format-test.target".format(name), srcs = [":{}.format-test".format(name)], data = [ ":{}".format(name), ], local = True, ) def _scalafmt_singleton_implementation(ctx): return [ _ScalaRulePhase( custom_phases = [ ("-", "runfiles", "scalafmt", _phase_scalafmt), ], ), ] scalafmt_singleton = rule( implementation = _scalafmt_singleton_implementation, ) ``` then in `path/to/some/buildfile/BUILD.bazel`, I have: ``` load("//path/to/above/my_rules.bzl", "scalafmt_singleton") scalafmt_singleton( name = "phase_scalafmt", visibility = ["//visibility:public"], ) ```
1 parent 8eb948c commit e942d94

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

scala/private/phases/phase_scalafmt.bzl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,25 @@ load(
1010

1111
def phase_scalafmt(ctx, p):
1212
if ctx.attr.format:
13-
manifest, files = _build_format(ctx)
13+
manifest, files, srcs = _build_format(ctx)
1414
_formatter(ctx, manifest, files, ctx.file._runner, ctx.outputs.scalafmt_runner)
1515
_formatter(ctx, manifest, files, ctx.file._testrunner, ctx.outputs.scalafmt_testrunner)
16+
17+
# Return a depset containing all the relevant files, so a wrapping `sh_test` can successfully access them.
18+
return struct(runfiles = depset([manifest] + files + srcs))
1619
else:
1720
_write_empty_content(ctx, ctx.outputs.scalafmt_runner)
1821
_write_empty_content(ctx, ctx.outputs.scalafmt_testrunner)
22+
return None
1923

2024
def _build_format(ctx):
2125
files = []
26+
srcs = []
2227
manifest_content = []
2328
for src in ctx.files.srcs:
2429
# only format scala source files, not generated files
2530
if src.path.endswith(_scala_extension) and src.is_source:
31+
srcs.append(src)
2632
file = ctx.actions.declare_file("{}.fmt.output".format(src.short_path))
2733
files.append(file)
2834
ctx.actions.run(
@@ -40,7 +46,7 @@ def _build_format(ctx):
4046
manifest = ctx.actions.declare_file("format/{}/manifest.txt".format(ctx.label.name))
4147
ctx.actions.write(manifest, "\n".join(manifest_content) + "\n")
4248

43-
return manifest, files
49+
return manifest, files, srcs
4450

4551
def _formatter(ctx, manifest, files, template, output_runner):
4652
ctx.actions.run_shell(

scala/scalafmt/private/format-test.template.sh

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
#!/bin/bash -e
2-
WORKSPACE_ROOT="${1:-$BUILD_WORKSPACE_DIRECTORY}"
2+
3+
# Explanation: IF $BUILD_WORKSPACE_DIRECTORY is set to something (as it would be during a
4+
# `bazel run`), then append a trailing `/`. If it's not set (as it wouldn't be during
5+
# a `bazel test` invocation in a wrapping `sh_test` rule), then elide the trailing `/`, and
6+
# instead rely upon a relative path from the test's runtrees. The corresponding change
7+
# to `phase_scalafmt` places the source files into the `runfiles` set, so they'll be symlinked
8+
# correctly in the appropriate relative location.
9+
WORKSPACE_ROOT="${1:-${BUILD_WORKSPACE_DIRECTORY}${BUILD_WORKSPACE_DIRECTORY:+/}}"
10+
311
RUNPATH="${TEST_SRCDIR-$0.runfiles}"/%workspace%
412
RUNPATH=(${RUNPATH//bin/ })
513
RUNPATH="${RUNPATH[0]}"bin
614

715
EXIT=0
16+
817
while read original formatted; do
918
if [[ ! -z "$original" ]] && [[ ! -z "$formatted" ]]; then
10-
if ! cmp -s "$WORKSPACE_ROOT/$original" "$RUNPATH/$formatted"; then
19+
if ! cmp -s "${WORKSPACE_ROOT}$original" "$RUNPATH/$formatted"; then
1120
echo $original
12-
diff "$WORKSPACE_ROOT/$original" "$RUNPATH/$formatted" || true
21+
diff "${WORKSPACE_ROOT}$original" "$RUNPATH/$formatted" || true
1322
EXIT=1
1423
fi
1524
fi

0 commit comments

Comments
 (0)