Merge pull request #87 from lucidsoftware/various-things

Various fixes and improvements

Author: jjudd

.bazelrc_shared

@@ -28,6 +28,7 @@ common:tests --@rules_scala_annex//rules/scala:scala-toolchain=test_zinc_2_13
 
 # These are backwards incompatible options; we should check to see if their values have been flipped
 # when upgrading to new major Bazel version.
+common --incompatible_modify_execution_info_additive
 common --incompatible_auto_exec_groups
 common --incompatible_autoload_externally=sh_binary # sh_binary is used by rules_jvm_external
 common --incompatible_config_setting_private_default_visibility

rules/common/private/utils.bzl

@@ -173,21 +173,9 @@ def safe_name(value):
 def _short_path(file):
     return file.short_path
 
-# This propagates specific tags as execution requirements to be passed to an action
-# A fix to bazelbuild/bazel that will make this no longer necessary is underway; we can remove this once that's released and we've obtained it
-PROPAGATABLE_TAGS = ["no-remote", "no-cache", "no-sandbox", "no-remote-exec", "no-remote-cache"]
-
-def resolve_execution_reqs(ctx, base_exec_reqs):
-    exec_reqs = {}
-    for tag in ctx.attr.tags:
-        if tag in PROPAGATABLE_TAGS:
-            exec_reqs.update({tag: "1"})
-    exec_reqs.update(base_exec_reqs)
-    return exec_reqs
-
 def _format_resources_item(item):
     key, value = item
-    return "{}:{}".format(value.path, key)
+    return "\"{}\":\"{}\"".format(value.path, key)
 
 def action_singlejar(
         ctx,
@@ -215,21 +203,18 @@ def action_singlejar(
     args.add("--output", output)
     if main_class != None:
         args.add("--main_class", main_class)
-        args.set_param_file_format("multiline")
-        args.use_param_file("@%s", use_always = True)
+    args.set_param_file_format("multiline")
+    args.use_param_file("@%s", use_always = True)
 
     all_inputs = depset(resources.values(), transitive = [inputs])
 
     ctx.actions.run(
         arguments = [args],
         executable = ctx.executable._singlejar,
-        execution_requirements = resolve_execution_reqs(
-            ctx,
-            {
-                "supports-workers": "0",
-                "supports-path-mapping": "1",
-            },
-        ),
+        execution_requirements = {
+            "supports-workers": "0",
+            "supports-path-mapping": "1",
+        },
         inputs = all_inputs,
         mnemonic = _SINGLE_JAR_MNEMONIC,
         outputs = [output],

rules/private/phases/phase_bootstrap_compile.bzl

@@ -1,7 +1,6 @@
 load("@rules_java//java/common:java_common.bzl", "java_common")
 load(
     "//rules/common:private/utils.bzl",
-    _resolve_execution_reqs = "resolve_execution_reqs",
     _strip_margin = "strip_margin",
 )
 
@@ -121,12 +120,9 @@ def phase_bootstrap_compile(ctx, g):
     ctx.actions.run_shell(
         arguments = [args],
         command = command,
-        execution_requirements = _resolve_execution_reqs(
-            ctx,
-            {
-                "supports-path-mapping": "1",
-            },
-        ),
+        execution_requirements = {
+            "supports-path-mapping": "1",
+        },
         inputs = inputs,
         mnemonic = "BootstrapScalacompile",
         outputs = [g.classpaths.jar, tmp],

rules/private/phases/phase_coverage_jacoco.bzl

@@ -3,10 +3,6 @@ load(
     _CodeCoverageConfiguration = "CodeCoverageConfiguration",
     _JacocoInfo = "JacocoInfo",
 )
-load(
-    "@rules_scala_annex//rules/common:private/utils.bzl",
-    _resolve_execution_reqs = "resolve_execution_reqs",
-)
 load(
     "@rules_scala_annex//rules/private:coverage_replacements_provider.bzl",
     _coverage_replacements_provider = "coverage_replacements_provider",
@@ -34,16 +30,13 @@ def phase_coverage_jacoco(ctx, g):
     ctx.actions.run(
         arguments = [args],
         executable = toolchain.code_coverage_configuration.instrumentation_worker.files_to_run,
-        execution_requirements = _resolve_execution_reqs(
-            ctx,
-            {
-                "supports-multiplex-workers": "1",
-                "supports-workers": "1",
-                "supports-multiplex-sandboxing": "1",
-                "supports-worker-cancellation": "1",
-                "supports-path-mapping": "1",
-            },
-        ),
+        execution_requirements = {
+            "supports-multiplex-workers": "1",
+            "supports-workers": "1",
+            "supports-multiplex-sandboxing": "1",
+            "supports-worker-cancellation": "1",
+            "supports-path-mapping": "1",
+        },
         inputs = [in_out_pair[0] for in_out_pair in in_out_pairs],
         mnemonic = "JacocoInstrumenter",
         outputs = [in_out_pair[1] for in_out_pair in in_out_pairs],

rules/private/phases/phase_zinc_compile.bzl

@@ -8,10 +8,6 @@ load(
     _ZincDepInfo = "ZincDepInfo",
     _ZincInfo = "ZincInfo",
 )
-load(
-    "@rules_scala_annex//rules/common:private/utils.bzl",
-    _resolve_execution_reqs = "resolve_execution_reqs",
-)
 
 #
 # PHASE: compile
@@ -106,10 +102,7 @@ def phase_zinc_compile(ctx, g):
     ctx.actions.run(
         arguments = [args],
         executable = worker.files_to_run,
-        execution_requirements = _resolve_execution_reqs(
-            ctx,
-            execution_requirements_tags,
-        ),
+        execution_requirements = execution_requirements_tags,
         inputs = inputs,
         mnemonic = "ScalaCompile",
         outputs = outputs,

rules/private/phases/phase_zinc_depscheck.bzl

@@ -6,7 +6,6 @@ load(
 )
 load(
     "@rules_scala_annex//rules/common:private/utils.bzl",
-    _resolve_execution_reqs = "resolve_execution_reqs",
     _short_path = "short_path",
 )
 
@@ -44,16 +43,13 @@ def phase_zinc_depscheck(ctx, g):
         ctx.actions.run(
             arguments = [deps_args],
             executable = deps_configuration.worker.files_to_run,
-            execution_requirements = _resolve_execution_reqs(
-                ctx,
-                {
-                    "supports-multiplex-workers": "1",
-                    "supports-workers": "1",
-                    "supports-multiplex-sandboxing": "1",
-                    "supports-worker-cancellation": "1",
-                    "supports-path-mapping": "1",
-                },
-            ),
+            execution_requirements = {
+                "supports-multiplex-workers": "1",
+                "supports-workers": "1",
+                "supports-multiplex-sandboxing": "1",
+                "supports-worker-cancellation": "1",
+                "supports-path-mapping": "1",
+            },
             inputs = [g.compile.used],
             mnemonic = "ScalaCheckDeps",
             outputs = [deps_check],

rules/register_toolchain.bzl

@@ -215,16 +215,29 @@ register_zinc_toolchain = _make_register_toolchain(_zinc_configuration)
 def _scala_incoming_transition_impl(settings, attr):
     result = dict(settings)
 
-    if attr.scala_toolchain_name != "":
+    if attr.scala_toolchain_name != "" and attr.scala_toolchain_name != settings[scala_toolchain_setting]:
         # We set `original_scala_toolchain_setting` so we can reset the toolchain to its
         # original value in `scala_outgoing_transition`. That way, we can ensure every target is
         # built under a single toolchain, thus preventing duplicate builds.
         #
+        # We do not do this work when the toolchain name is set, but is no different than what is
+        # already set. By having that check we avoid the failure mode where the original toolchain
+        # name gets set equal to the current toolchain name and destroys whatever the actual original
+        # toolchain name was. For example
+        #  State 1:              State 2:          State 3:
+        #    Setting: A      =>    Setting: B  =>    Setting: B  => Game over
+        #    Original: Unset       Original: A       Original: B
+        #
+        # Note that the check described above should ideally not be required due to outgoing
+        # transitions but it is, so something is going wrong. As a result, the check is probably
+        # temporary, but who knows.
+        #
         # This is inspired by what the rules_go folks are doing.
         result[original_scala_toolchain_setting] = settings[scala_toolchain_setting]
         result[scala_toolchain_setting] = attr.scala_toolchain_name
 
-    if hasattr(attr, "scalafmt_toolchain_name") and attr.scalafmt_toolchain_name != "":
+    if (hasattr(attr, "scalafmt_toolchain_name") and attr.scalafmt_toolchain_name != "" and
+        attr.scalafmt_toolchain_name != settings[scalafmt_toolchain_setting]):
         result[original_scalafmt_toolchain_setting] = settings[scalafmt_toolchain_setting]
         result[scalafmt_toolchain_setting] = attr.scalafmt_toolchain_name
 

rules/scala.bzl

@@ -57,7 +57,7 @@ _compile_private_attributes = {
     ),
     "_singlejar": attr.label(
         cfg = "exec",
-        default = "@bazel_tools//tools/jdk:singlejar",
+        default = "@rules_java//toolchains:singlejar",
         executable = True,
     ),
 

rules/scala/private/doc.bzl

@@ -8,7 +8,6 @@ load(
 load(
     "//rules/common:private/utils.bzl",
     _collect = "collect",
-    _resolve_execution_reqs = "resolve_execution_reqs",
     _separate_src_jars_srcs_and_other = "separate_src_jars_srcs_and_other",
 )
 
@@ -55,16 +54,13 @@ def scaladoc_implementation(ctx):
     ctx.actions.run(
         arguments = [args],
         executable = ctx.attr._runner.files_to_run,
-        execution_requirements = _resolve_execution_reqs(
-            ctx,
-            {
-                "supports-multiplex-workers": "1",
-                "supports-workers": "1",
-                "supports-multiplex-sandboxing": "1",
-                "supports-worker-cancellation": "1",
-                "supports-path-mapping": "1",
-            },
-        ),
+        execution_requirements = {
+            "supports-multiplex-workers": "1",
+            "supports-workers": "1",
+            "supports-multiplex-sandboxing": "1",
+            "supports-worker-cancellation": "1",
+            "supports-path-mapping": "1",
+        },
         inputs = depset(
             src_jars + srcs + [toolchain.zinc_configuration.compiler_bridge],
             transitive = [classpath, compiler_classpath],

rules/scala_proto/private/core.bzl

@@ -1,7 +1,6 @@
 load("@protobuf//bazel/common:proto_info.bzl", "ProtoInfo")
 load(
     "//rules/common:private/utils.bzl",
-    _resolve_execution_reqs = "resolve_execution_reqs",
     _safe_name = "safe_name",
 )
 
@@ -44,16 +43,13 @@ def scala_proto_library_implementation(ctx):
     ctx.actions.run(
         arguments = [args],
         executable = compiler.compiler.files_to_run,
-        execution_requirements = _resolve_execution_reqs(
-            ctx,
-            {
-                "supports-multiplex-workers": supports_workers,
-                "supports-workers": supports_workers,
-                "supports-multiplex-sandboxing": supports_workers,
-                "supports-worker-cancellation": supports_workers,
-                "supports-path-mapping": supports_workers,
-            },
-        ),
+        execution_requirements = {
+            "supports-multiplex-workers": supports_workers,
+            "supports-workers": supports_workers,
+            "supports-multiplex-sandboxing": supports_workers,
+            "supports-worker-cancellation": supports_workers,
+            "supports-path-mapping": supports_workers,
+        },
         inputs = depset(direct = [], transitive = [transitive_sources]),
         mnemonic = "ScalaProtoCompile",
         outputs = [gendir],
@@ -71,9 +67,9 @@ def scala_proto_library_implementation(ctx):
     ctx.actions.run_shell(
         arguments = [shell_args],
         command = """$1 c $4 META-INF/= $(find -L $2 -type f | while read v; do echo ${v#"${2%$3}"}=$v; done)""",
-        execution_requirements = _resolve_execution_reqs(ctx, {
+        execution_requirements = {
             "supports-path-mapping": "1",
-        }),
+        },
         inputs = [gendir],
         mnemonic = "SrcJar",
         outputs = [srcjar],

rules/scalafmt/private/test.bzl

@@ -1,6 +1,5 @@
 load(
     "@rules_scala_annex//rules/common:private/utils.bzl",
-    _resolve_execution_reqs = "resolve_execution_reqs",
     _short_path = "short_path",
 )
 
@@ -49,16 +48,13 @@ def build_format(ctx):
             ctx.actions.run(
                 arguments = ["--jvm_flag=-Dfile.encoding=UTF-8", args],
                 executable = ctx.executable._fmt,
-                execution_requirements = _resolve_execution_reqs(
-                    ctx,
-                    {
-                        "supports-multiplex-workers": "1",
-                        "supports-workers": "1",
-                        "supports-multiplex-sandboxing": "1",
-                        "supports-worker-cancellation": "1",
-                        "supports-path-mapping": "1",
-                    },
-                ),
+                execution_requirements = {
+                    "supports-multiplex-workers": "1",
+                    "supports-workers": "1",
+                    "supports-multiplex-sandboxing": "1",
+                    "supports-worker-cancellation": "1",
+                    "supports-path-mapping": "1",
+                },
                 inputs = [config, src],
                 mnemonic = "ScalaFmt",
                 outputs = [file],
@@ -81,9 +77,9 @@ def format_runner(ctx, manifest, files):
     ctx.actions.run_shell(
         arguments = [args],
         command = "cat $1 | sed -e s#%workspace%#$2# -e s#%manifest%#$3# > $4",
-        execution_requirements = _resolve_execution_reqs(ctx, {
+        execution_requirements = {
             "supports-path-mapping": "1",
-        }),
+        },
         inputs = [ctx.file._runner, manifest] + files,
         mnemonic = "CreateScalaFmtRunner",
         outputs = [ctx.outputs.scalafmt_runner],
@@ -100,9 +96,9 @@ def format_tester(ctx, manifest, files):
     ctx.actions.run_shell(
         arguments = [args],
         command = "cat $1 | sed -e s#%workspace%#$2# -e s#%manifest%#$3# > $4",
-        execution_requirements = _resolve_execution_reqs(ctx, {
+        execution_requirements = {
             "supports-path-mapping": "1",
-        }),
+        },
         inputs = [ctx.file._testrunner, manifest] + files,
         mnemonic = "CreateScalaFmtTester",
         outputs = [ctx.outputs.scalafmt_testrunner],

tests/resources/special_characters/BUILD

@@ -0,0 +1,7 @@
+load("@rules_scala_annex//rules:scala.bzl", "scala_library")
+
+scala_library(
+    name = "special_characters",
+    resources = glob(["**/*.txt"]),
+    scala_toolchain_name = "test_zinc_2_13",
+)

tests/resources/special_characters/example/spaces in this file name.txt

@@ -0,0 +1 @@
+example

tests/resources/special_characters/test

@@ -0,0 +1,4 @@
+#!/bin/bash -e
+. "$(dirname "$0")"/../../common.sh
+
+bazel build :special_characters

tests/tagging/BUILD

@@ -1,14 +1,16 @@
 load("@rules_scala_annex//rules:scala.bzl", "scala_binary", "scala_library", "scala_test")
-load(
-    "@rules_scala_annex//rules/common:private/utils.bzl",
-    _PROPAGATABLE_TAGS = "PROPAGATABLE_TAGS",
-)
 
 scala_binary(
     name = "binary-all-propagatable",
     srcs = ["A.scala"],
     scala_toolchain_name = "test_zinc_2_13",
-    tags = ["manual"] + _PROPAGATABLE_TAGS,
+    tags = ["manual"] + [
+        "no-cache",
+        "no-remote",
+        "no-remote-cache",
+        "no-remote-exec",
+        "no-sandbox",
+    ],
 )
 
 scala_library(