Windows portability fixes (#1386)

* buildifier: auto-sorting

* Use native .run action over run_shell

Remove dependency on host shell
since bash is not available on Windows

* Add get_auth to repository_download

In case the content is e.g. behind a mirror with authentication, this is
necessary.

Note: get_auth was introduced in 7.1.0 and would break compatibility
with earlier versions

* Add unsorted-dict-items to the buildifier warnigns

Added fixes with:
```
bazel run //:buildifier.fix
```

* Fix buildifier deprecation

When running `//:buildifier.check` this deprecation warning is issued
```
buildifier: selecting diff program with the BUILDIFIER_DIFF, BUILDIFIER_MULTIDIFF, and DISPLAY environment variables is deprecated, use flags -diff_command and -multi_diff instead
```

* Fix order of sorted flags after _TEST_OPTS got sorted

Since the order of the opts changed, the order in which the flags are
produced changes as well

* Add the unsorted-dict-items checks to ci and pre-commit-config

* Bump buildifier to same version as .bazelci version

bazelci uses 8.2.0

Author: FaBrand

.bazelci/presubmit.yml

@@ -20,7 +20,7 @@ buildifier:
   version: 8.2.0
   # no lint warnings for the moment. They are basically a smoke alarm in hell.
   # keep this argument in sync with .pre-commit-config.yaml
-  warnings: "-confusing-name,-constant-glob,-duplicated-name,-function-docstring,-function-docstring-args,-function-docstring-header,-module-docstring,-name-conventions,-no-effect,-constant-glob,-provider-params,-print,-rule-impl-return,-bzl-visibility,-unnamed-macro,-uninitialized,-unreachable"
+  warnings: "+unsorted-dict-items,-confusing-name,-constant-glob,-duplicated-name,-function-docstring,-function-docstring-args,-function-docstring-header,-module-docstring,-name-conventions,-no-effect,-constant-glob,-provider-params,-print,-rule-impl-return,-bzl-visibility,-unnamed-macro,-uninitialized,-unreachable"
 tasks:
   unittests:
     name: "Unit Tests"

.pre-commit-config.yaml

@@ -8,6 +8,6 @@ repos:
       - id: buildifier
         args: &args
           # Keep this argument in sync with .bazelci/config.yaml
-          - --warnings=-confusing-name,-constant-glob,-duplicated-name,-function-docstring,-function-docstring-args,-function-docstring-header,-module-docstring,-name-conventions,-no-effect,-constant-glob,-provider-params,-print,-rule-impl-return,-bzl-visibility,-unnamed-macro,-uninitialized,-unreachable
+          - --warnings=+unsorted-dict-items,-confusing-name,-constant-glob,-duplicated-name,-function-docstring,-function-docstring-args,-function-docstring-header,-module-docstring,-name-conventions,-no-effect,-constant-glob,-provider-params,-print,-rule-impl-return,-bzl-visibility,-unnamed-macro,-uninitialized,-unreachable
       - id: buildifier-lint
         args: *args

BUILD

@@ -91,6 +91,7 @@ buildifier(
     ],
     lint_mode = "warn",
     lint_warnings = [
+        "+unsorted-dict-items",
         "-confusing-name",
         "-constant-glob",
         "-duplicated-name",
@@ -109,7 +110,6 @@ buildifier(
         "-uninitialized",
         "-unreachable",
     ],
-    mode = "diff",
 )
 
 buildifier(
@@ -118,4 +118,7 @@ buildifier(
         "./.git/*",
     ],
     lint_mode = "fix",
+    lint_warnings = [
+        "+unsorted-dict-items",
+    ],
 )

MODULE.bazel

@@ -17,7 +17,7 @@ bazel_dep(name = "rules_android", version = "0.6.4")
 bazel_dep(name = "bazel_features", version = "1.25.0")
 bazel_dep(name = "rules_shell", version = "0.4.1")
 
-bazel_dep(name = "buildifier_prebuilt", version = "8.0.3", dev_dependency = True)
+bazel_dep(name = "buildifier_prebuilt", version = "8.2.0.2", dev_dependency = True)
 
 rules_java_toolchains = use_extension("@rules_java//java:extensions.bzl", "toolchains")
 use_repo(rules_java_toolchains, "remote_java_tools")

examples/anvil/third_party/BUILD.bazel

@@ -62,8 +62,8 @@ kt_compiler_plugin(
     compile_phase = True,
     id = "com.squareup.anvil.compiler",
     options = {
-        "src-gen-dir": "{generatedSources}",
         "generate-dagger-factories": "true",
+        "src-gen-dir": "{generatedSources}",
     },
     stubs_phase = True,
     target_embedded_compiler = True,

examples/ksp/third_party/BUILD.bazel

@@ -14,30 +14,30 @@ kt_compiler_plugin(
     compile_phase = True,
     id = "com.google.devtools.ksp.symbol-processing",
     options = {
+        "allWarningsAsErrors": "false",
         "apclasspath": "{classpath}",
-        # projectBaseDir shouldn't matter because incremental is disabled
-        "projectBaseDir": "{temp}",
-        # Disable incremental mode
-        "incremental": "false",
+        # Directory to write KSP caches. Shouldn't matter because incremental is disabled
+        "cachesDir": "{temp}",
         # Directory where class files are written to. Files written to this directory are class
         # files being written directly from the annotation processor, not Kotlinc
         "classOutputDir": "{generatedClasses}",
+        # Disable incremental mode
+        "incremental": "false",
         # Directory where generated Java sources files are written to
         "javaOutputDir": "{generatedSources}",
         # Directory where generated Kotlin sources files are written to
         "kotlinOutputDir": "{generatedSources}",
+        # TODO(bencodes) Not sure what this directory is yet.
+        "kspOutputDir": "{temp}",
+        # projectBaseDir shouldn't matter because incremental is disabled
+        "projectBaseDir": "{temp}",
         # Directory where META-INF data is written to. This might not be the most ideal place to
         # write this. Maybe just directly to the classes directory?
         "resourceOutputDir": "{generatedSources}",
-        # TODO(bencodes) Not sure what this directory is yet.
-        "kspOutputDir": "{temp}",
-        # Directory to write KSP caches. Shouldn't matter because incremental is disabled
-        "cachesDir": "{temp}",
-        # Include in compilation as an example. This should be processed in the stubs phase.
-        "withCompilation": "true",
         # Set returnOkOnError to false because we want to fail the build if there are any errors
         "returnOkOnError": "false",
-        "allWarningsAsErrors": "false",
+        # Include in compilation as an example. This should be processed in the stubs phase.
+        "withCompilation": "true",
     },
     visibility = ["//visibility:public"],
     deps = [

kotlin/internal/jvm/compile.bzl

@@ -305,16 +305,16 @@ def _build_resourcejar_action(ctx, extra_resources = {}):
     """
     resources_jar_output = ctx.actions.declare_file(ctx.label.name + "-resources.jar")
     zipper_args = _resourcejar_args_action(ctx, extra_resources)
-    ctx.actions.run_shell(
+    ctx.actions.run(
         mnemonic = "KotlinZipResourceJar",
+        executable = ctx.executable._zipper,
         inputs = ctx.files.resources + extra_resources.values() + [zipper_args],
-        tools = [ctx.executable._zipper],
         outputs = [resources_jar_output],
-        command = "{zipper} c {resources_jar_output} @{path}".format(
-            path = zipper_args.path,
-            resources_jar_output = resources_jar_output.path,
-            zipper = ctx.executable._zipper.path,
-        ),
+        arguments = [
+            "c",
+            resources_jar_output.path,
+            "@" + zipper_args.path,
+        ],
         progress_message = "Creating intermediate resource jar %{label}",
     )
     return resources_jar_output
@@ -390,8 +390,8 @@ def _run_kapt_builder_actions(
         plugins = plugins,
         outputs = {
             "generated_java_srcjar": ap_generated_src_jar,
-            "kapt_generated_stub_jar": kapt_generated_stub_jar,
             "kapt_generated_class_jar": kapt_generated_class_jar,
+            "kapt_generated_stub_jar": kapt_generated_stub_jar,
         },
         build_kotlin = False,
         mnemonic = "KotlinKapt",
@@ -433,8 +433,8 @@ def _run_ksp_builder_actions(
         transitive_runtime_jars = transitive_runtime_jars,
         plugins = plugins,
         outputs = {
-            "ksp_generated_java_srcjar": ksp_generated_java_srcjar,
             "ksp_generated_classes_jar": ksp_generated_classes_jar,
+            "ksp_generated_java_srcjar": ksp_generated_java_srcjar,
         },
         build_kotlin = False,
         mnemonic = "KotlinKsp",
@@ -832,8 +832,8 @@ def _run_kt_java_builder_actions(
         if not "kt_abi_plugin_incompatible" in ctx.attr.tags and toolchains.kt.experimental_use_abi_jars == True:
             kt_compile_jar = ctx.actions.declare_file(ctx.label.name + "-kt.abi.jar")
             outputs = {
-                "output": kt_runtime_jar,
                 "abi_jar": kt_compile_jar,
+                "output": kt_runtime_jar,
             }
         else:
             kt_compile_jar = kt_runtime_jar

kotlin/internal/jvm/impl.bzl

@@ -105,17 +105,17 @@ def _write_launcher_action(ctx, rjars, main_class, jvm_flags):
             output = ctx.outputs.executable,
             substitutions = {
                 "%classpath%": classpath,
-                "%runfiles_manifest_only%": "",
                 "%java_start_class%": "com.google.testing.coverage.JacocoCoverageRunner",
                 "%javabin%": java_bin,
                 "%jvm_flags%": jvm_flags,
-                "%set_jacoco_metadata%": "export JACOCO_METADATA_JAR=\"$JAVA_RUNFILES/{}/{}\"".format(ctx.workspace_name, jacoco_metadata_file.short_path),
-                "%set_jacoco_main_class%": """export JACOCO_MAIN_CLASS={}""".format(main_class),
+                "%needs_runfiles%": "0" if _is_absolute(java_bin_path) else "1",
+                "%runfiles_manifest_only%": "",
                 "%set_jacoco_java_runfiles_root%": """export JACOCO_JAVA_RUNFILES_ROOT=$JAVA_RUNFILES/{}/""".format(ctx.workspace_name),
+                "%set_jacoco_main_class%": """export JACOCO_MAIN_CLASS={}""".format(main_class),
+                "%set_jacoco_metadata%": "export JACOCO_METADATA_JAR=\"$JAVA_RUNFILES/{}/{}\"".format(ctx.workspace_name, jacoco_metadata_file.short_path),
                 "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=YES""",
-                "%workspace_prefix%": ctx.workspace_name + "/",
                 "%test_runtime_classpath_file%": "export TEST_RUNTIME_CLASSPATH_FILE=${JAVA_RUNFILES}",
-                "%needs_runfiles%": "0" if _is_absolute(java_bin_path) else "1",
+                "%workspace_prefix%": ctx.workspace_name + "/",
             },
             is_executable = True,
         )
@@ -130,17 +130,17 @@ def _write_launcher_action(ctx, rjars, main_class, jvm_flags):
         output = ctx.outputs.executable,
         substitutions = {
             "%classpath%": classpath,
-            "%runfiles_manifest_only%": "",
             "%java_start_class%": main_class,
             "%javabin%": java_bin,
             "%jvm_flags%": jvm_flags,
-            "%set_jacoco_metadata%": "",
-            "%set_jacoco_main_class%": "",
+            "%needs_runfiles%": "0" if _is_absolute(java_bin_path) else "1",
+            "%runfiles_manifest_only%": "",
             "%set_jacoco_java_runfiles_root%": "",
+            "%set_jacoco_main_class%": "",
+            "%set_jacoco_metadata%": "",
             "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO""",
-            "%workspace_prefix%": ctx.workspace_name + "/",
             "%test_runtime_classpath_file%": "export TEST_RUNTIME_CLASSPATH_FILE=${JAVA_RUNFILES}",
-            "%needs_runfiles%": "0" if _is_absolute(java_bin_path) else "1",
+            "%workspace_prefix%": ctx.workspace_name + "/",
         },
         is_executable = True,
     )
@@ -333,8 +333,8 @@ def kt_jvm_junit_test_impl(ctx):
 
 _KtCompilerPluginClasspathInfo = provider(
     fields = {
-        "reshaded_infos": "list reshaded JavaInfos of a compiler library",
         "infos": "list JavaInfos of a compiler library",
+        "reshaded_infos": "list reshaded JavaInfos of a compiler library",
     },
 )