Simplify test_dependency_versions.sh

[mbland]

Description

Removes almost all of the copied deps/test sources and targets in favor of invoking @rules_scala and @multi_frameworks_toolchain tests directly. Moves targets depending on @rules_python and @rules_shell to new packages so test packages don't break when @rules_scala isn't the main module.

Introduces RULES_SCALA_TARGETS, MULTI_FRAMEWORKS_TOOLCHAIN_TARGETS, and ALL_TARGETS arrays to easily specify compatibility test targets instead of copying them. Only keeps a single ScalafmtTest target within deps/test/BUILD.bazel.test, which is a special case (described below) and executes successfully on Windows.

Though these are new packages, they're comprised of existing files and targets from their parent packages:

• //test/jmh/runtime
• //test/sh_tests
• //test/src/main/scala/scalarules/test/twitter_scrooge/strings

Also:

• Marks every bazel_dep for @latest_dependencies with dev_dependency = True.

• Replaces $(location ...) instances in moved targets with $(rootpath ...) (and in one case, with $(execpath ...)) per Bazel 8 $(location) expands to $(execpath) under bazel build bazelbuild/bazel#25198.

Motivation

While drafting a blog post describing test_dependency_versions.sh, I revisited the decision to copy targets and files in #1729 and #1738. Executing test targets from @rules_scala directly from the test module actually works, making the test far less complex, and making for better testing advice.

This required moving targets that referenced dev dependencies to their own packages, fixing @rules_scala test package breakages. Making the @latest_dependencies module a dev dependency eliminated module version warnings from @multi_frameworks_toolchain, since the test sets older dependency versions. Then making @latest_dependencies a dev dependency everywhere, instead of only in @multi_frameworks_toolchain, seemed more consistent.

The test retains the //:ScalafmtTest target because rules_scala doesn't actually provide Scalafmt rules, but only provides ext_scalafmt to create custom rules, per:

• docs/phase_scalafmt.md
• docs/customizable_phase.md

I discovered this after going down a rabbit hole regarding the implicitly defined (and ostensibly deprecated) .format and .format-test predeclared outputs for Scalafmt rules. Long story short, implicitly defined predeclared outputs have been "deprecated" since 2018-03-05, predating Bazel 0.28.0:

• https://bazel.build/versions/8.3.0/extending/rules#requesting_output_files
• https://bazel.build/versions/8.3.0/extending/rules#deprecated_predeclared_outputs
• https://bazel.build/versions/8.3.0/rules/lib/globals/bzl#rule.outputs
• 2018-03-05: Output Map Madness
• 2019-04-08: incompatible_no_rule_outputs_param: disable outputs param of rule function bazelbuild/bazel#7977
• 2019-06-07: Rollforward "Disable outputs param of rule function" with fix (introduced --incompatible_no_rule_outputs_param in Bazel 0.28.0): bazelbuild/bazel@36c70a6
• 2019-09-04: Document the replacements for the outputs parameter to the rule() function. (Bazel 1.0.0): bazelbuild/bazel@e29ddda

However, I learned that the .format and .format-test targets are Bash scripts run after the original rule executes Scalafmt. The generated scripts don't depend on the //scala/scalafmt:scalafmt binary at all. Plus, they only work when invoked within the main module, since they reference relative paths within BUILD_WORKSPACE_DIRECTORY. And finally, bazel test @rules_scala//test/scalafmt:formatted-test fails because it tries to declare output files in a nonexistent directory.

ERROR: .../external/rules_scala~/test/scalafmt/BUILD:43:20:
  in scalafmt_scala_test rule
  @@rules_scala~//test/scalafmt:formatted-test:

Traceback (most recent call last):
  File ".../external/rules_scala~/scala/private/rules/scala_test.bzl",
    line 38, column 22, in _scala_test_impl
      return run_phases(
  File ".../external/rules_scala~/scala/private/phases/api.bzl",
    line 45, column 23, in run_phases
      return _run_phases(ctx, builtin_customizable_phases, target = None)
  File ".../external/rules_scala~/scala/private/phases/api.bzl",
    line 77, column 32, in _run_phases
      new_provider = function(ctx, current_provider)
  File ".../external/rules_scala~/scala/private/phases/phase_scalafmt.bzl",
    line 10, column 46, in phase_scalafmt
      manifest, files, srcs = _build_format(ctx)
  File ".../external/rules_scala~/scala/private/phases/phase_scalafmt.bzl",
    line 30, column 44, in _build_format
      file = ctx.actions.declare_file("{}.fmt.output".format(src.short_path))

Error in declare_file:
  the output artifact
  'external/rules_scala~/test/rules_scala~/test/scalafmt/formatted/formatted-test.scala.fmt.output'
  is not under package directory
  'external/rules_scala~/test/scalafmt'
  for target '@@rules_scala~//test/scalafmt:formatted-test'

ERROR: .../external/rules_scala~/test/scalafmt/BUILD:43:20:
  Analysis of target '@@rules_scala~//test/scalafmt:formatted-test'
  failed

So invoking bazel test //:ScalafmtTest (via /... from ALL_TARGETS) is sufficient, since we're only validating that the toolchains execute properly. (test_scalafmt.sh tests the behavior of the .format and .format-test scripts, including on Windows via --run_under=bash.) This also means we can invoke this target on Windows, instead of having special case logic to avoid invoking the previous bazel run command.

[mbland]

@simuons @liucijus This has a slight conflict with #1756 in //test:data_location_expansion, since it updates the existing jvm_flags attribute to use $(rootpaths ...), and the other PR updates that attribute to use @bazel_worker_java. I've resolved this in a local branch and verified that it succeeds with the last_green build (1b120ef3e303e95e8f7041ceb096e95d42ec88d3). I can resolve it quickly after whichever pull request lands first, or you're welcome to do it.

Also, now that @WojciechMazur filed #1757, it seems like it would be good to include that with #1755, #1756, and one more dependency bump pull request before releasing v7.1.0. This pull request is purely internal, so it isn't critical to include before v7.1.0, but it would be nice.

[simuons]

Thanks @mbland, I'm merging this before #1756 as it got conflict probably after merging #1755