Make variable substituion for `javacopts`

[protocol7]

The documentation for java_library and java_binary states that the javacopts attribute supports make variable substitution, e.g.

rules_java/java/common/rules/java_library.bzl

Line 202 in 624b208

"javacopts": attr.string_list(

. However, this does not seem to be the case. Here's a small test case:

  MODULE.bazel
  module(name = "make_var_bug_repro")

  bazel_dep(name = "rules_java", version = "8.6.3")

  java = use_extension("@rules_java//java:extensions.bzl", "java")
  java.toolchain(version = "21")

  make_var_provider.bzl
  "A simple rule that provides a make variable via TemplateVariableInfo."

  def _make_var_provider_impl(ctx):
      return [
          platform_common.TemplateVariableInfo({
              ctx.attr.var_name: ctx.attr.var_value,
          }),
      ]

  make_var_provider = rule(
      implementation = _make_var_provider_impl,
      attrs = {
          "var_name": attr.string(mandatory = True),
          "var_value": attr.string(mandatory = True),
      },
  )

  BUILD.bazel
  load("@rules_java//java:defs.bzl", "java_library")
  load(":make_var_provider.bzl", "make_var_provider")

  make_var_provider(
      name = "my_make_var",
      var_name = "MY_CUSTOM_OPT",
      var_value = "-Xlint:none",
  )

  # According to docs, javacopts supports "Make variable" substitution
  # but this fails with: error: invalid flag: $(MY_CUSTOM_OPT)
  java_library(
      name = "hello",
      srcs = ["Hello.java"],
      javacopts = ["$(MY_CUSTOM_OPT)"],
      toolchains = [":my_make_var"],
  )

  Hello.java
  public class Hello {
      public static void main(String[] args) {
          System.out.println("Hello, World!");
      }
  }

  Run with: bazel build //:hello --java_runtime_version=remotejdk_21

This fails with:

$ bazel build //:hello --java_runtime_version=remotejdk_21
INFO: Analyzed target //:hello (0 packages loaded, 0 targets configured).
ERROR: /private/tmp/claude/rules_java_make_var_bug/BUILD.bazel:14:13: Building libhello.jar (1 source file) failed: (Exit 1): java failed: error executing Javac command (from java_library rule target //:hello) external/rules_java++toolchains+remotejdk25_macos_aarch64/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
java.lang.IllegalArgumentException: error: invalid flag: $(MY_CUSTOM_OPT)
	at jdk.compiler/com.sun.tools.javac.main.Arguments.reportDiag(Arguments.java:914)
	at jdk.compiler/com.sun.tools.javac.main.Arguments.doProcessArgs(Arguments.java:409)
	at jdk.compiler/com.sun.tools.javac.main.Arguments.processArgs(Arguments.java:361)
	at jdk.compiler/com.sun.tools.javac.main.Arguments.init(Arguments.java:260)
	at jdk.compiler/com.sun.tools.javac.api.JavacTool.getTask(JavacTool.java:191)
	at com.google.devtools.build.buildjar.javac.BlazeJavacMain.compile(BlazeJavacMain.java:137)
	at com.google.devtools.build.buildjar.ReducedClasspathJavaLibraryBuilder.compileSources(ReducedClasspathJavaLibraryBuilder.java:57)
	at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.compileJavaLibrary(SimpleJavaLibraryBuilder.java:110)
	at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.run(SimpleJavaLibraryBuilder.java:118)
	at com.google.devtools.build.buildjar.BazelJavaBuilder.build(BazelJavaBuilder.java:111)
	at com.google.devtools.build.buildjar.BazelJavaBuilder.parseAndBuild(BazelJavaBuilder.java:91)
	at com.google.devtools.build.buildjar.BazelJavaBuilder.lambda$main$0(BazelJavaBuilder.java:52)
	at com.google.devtools.build.lib.worker.WorkRequestHandler$WorkRequestCallback.apply(WorkRequestHandler.java:252)
	at com.google.devtools.build.lib.worker.WorkRequestHandler.respondToRequest(WorkRequestHandler.java:480)
	at com.google.devtools.build.lib.worker.WorkRequestHandler.lambda$startResponseThread$0(WorkRequestHandler.java:433)
	at java.base/java.lang.Thread.run(Thread.java:1474)

This shows that the make variable has not been substitued before javac is invoked. Is this expected, and the docs should be updated, or should support for substitution be added?

[hvadehra]

Thanks for the report.

Looks like this was a regression introduced when we Starlarkified the rules. Unfortunately, we apparently had no test coverage for this, and no usages internally either. No one has complained about it since either. The fix itself is going to be trivial, so I'll do it.

[protocol7]

Thanks! We have a patch for this internally, providing it here if it's useful. This uses ctx.var since expand_make_variables is deprecated.

diff --git a/java/common/rules/impl/compile_action.bzl b/java/common/rules/impl/compile_action.bzl
--- a/java/common/rules/impl/compile_action.bzl
+++ b/java/common/rules/impl/compile_action.bzl
@@ -20,6 +20,17 @@ load("//java/private:java_common_internal.bzl", _compile_private_for_builtins =

 visibility("private")

+def _expand_make_vars(s, ctx):
+    """Expands make variables in a string using ctx.var.
+
+    This implements the documented "Make variable" substitution for javacopts.
+    """
+    result = s
+    for key, value in ctx.var.items():
+        result = result.replace("$(%s)" % key, value)
+    return result
+
+
 def _filter_strict_deps(mode):
     return "error" if mode in ["strict", "default"] else mode

@@ -151,7 +162,7 @@ def compile_action(
         plugins = plugins,
         deps = deps,
         native_libraries = native_libraries,
         runtime_deps = runtime_deps,
         exports = exports,
         exported_plugins = exported_plugins,
-        javac_opts = [ctx.expand_location(opt) for opt in javacopts],
+        javac_opts = [ctx.expand_location(_expand_make_vars(opt, ctx)) for opt in javacopts],
         neverlink = neverlink,
         output_source_jar = output_source_jar,

[hvadehra]

Thanks, but I'd rather we not invent yet another way to expand options. Using the deprecated API probably makes the most sense. If/when it's scheduled for removal, we should have a drop-in replacement available that we can switch to.

[hvadehra]

That aside, how long have you had this patch? Any reason you're only reporting this now? It would have been nice to have caught/fixed this when it was first noticed.

[protocol7]

That aside, how long have you had this patch? Any reason you're only reporting this now? It would have been nice to have caught/fixed this when it was first noticed.

Just this week, thus the report now.