From 5b5f84ce18f8c845b234e5f6f8ed67b9d8e553e4 Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Fri, 5 Apr 2019 17:27:19 +0200 Subject: [PATCH 01/11] Add native windows launcher --- scala/private/rule_impls.bzl | 181 +++++++++++------- scala/scala.bzl | 20 +- src/java/io/bazel/rulesscala/exe/BUILD | 24 +++ .../io/bazel/rulesscala/exe/LaunchInfo.java | 170 ++++++++++++++++ .../rulesscala/exe/LauncherFileWriter.java | 56 ++++++ .../jmh_support/BenchmarkGenerator.scala | 6 +- 6 files changed, 388 insertions(+), 69 deletions(-) create mode 100644 src/java/io/bazel/rulesscala/exe/BUILD create mode 100644 src/java/io/bazel/rulesscala/exe/LaunchInfo.java create mode 100644 src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 863f6da8d..b6e245dad 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -590,76 +590,99 @@ def _jar_path_based_on_java_bin(ctx): jar_path = java_bin.rpartition("/")[0] + "/jar" return jar_path -def _write_executable(ctx, rjars, main_class, jvm_flags, wrapper, use_jacoco): - template = ctx.attr._java_stub_template.files.to_list()[0] - - jvm_flags = " ".join( - [ctx.expand_location(f, ctx.attr.data) for f in jvm_flags], - ) - - javabin = "export REAL_EXTERNAL_JAVA_BIN=${JAVABIN};JAVABIN=%s/%s" % ( - _runfiles_root(ctx), - wrapper.short_path, - ) - - if use_jacoco and _coverage_replacements_provider.is_enabled(ctx): - classpath = ":".join( - ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list() + ctx.files._jacocorunner + ctx.files._lcov_merger], - ) - jacoco_metadata_file = ctx.actions.declare_file( - "%s.jacoco_metadata.txt" % ctx.attr.name, - sibling = ctx.outputs.executable, +def _write_executable(ctx, executable, rjars, main_class, jvm_flags, wrapper, use_jacoco): + if (_is_windows(ctx)): + classpath = ";".join( + [("external/%s" % (j.short_path[3:]) if j.short_path.startswith("../") else j.short_path) for j in rjars.to_list()], ) - ctx.actions.write(jacoco_metadata_file, "\n".join([ - jar.short_path.replace("../", "external/") - for jar in rjars - ])) - ctx.actions.expand_template( - template = template, - output = ctx.outputs.executable, - substitutions = { - "%classpath%": classpath, - "%javabin%": javabin, - "%jarbin%": _jar_path_based_on_java_bin(ctx), - "%jvm_flags%": jvm_flags, - "%needs_runfiles%": "", - "%runfiles_manifest_only%": "", - "%workspace_prefix%": ctx.workspace_name + "/", - "%java_start_class%": "com.google.testing.coverage.JacocoCoverageRunner", - "%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), - "%set_jacoco_java_runfiles_root%": """export JACOCO_JAVA_RUNFILES_ROOT=$JAVA_RUNFILES/{}/""".format(ctx.workspace_name), - "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=YES""", - }, - is_executable = True, + jvm_flags_str = ";".join(jvm_flags) + java_for_exe = "%s/%s" % (ctx.workspace_name, str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path)) + + ctx.actions.run( + outputs = [executable], + inputs = [], + executable = ctx.attr._exe.files_to_run.executable, + arguments = [executable.path, ctx.workspace_name, java_for_exe, main_class, classpath, jvm_flags_str], + mnemonic = "ExeLauncher", + progress_message = "Creating exe launcher", ) - return [jacoco_metadata_file] + return [] else: - # RUNPATH is defined here: - # https://github.com/bazelbuild/bazel/blob/0.4.5/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L227 - classpath = ":".join( - ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list()], + template = ctx.attr._java_stub_template.files.to_list()[0] + + jvm_flags = " ".join( + [ctx.expand_location(f, ctx.attr.data) for f in jvm_flags], ) - ctx.actions.expand_template( - template = template, - output = ctx.outputs.executable, - substitutions = { - "%classpath%": classpath, - "%java_start_class%": main_class, - "%javabin%": javabin, - "%jarbin%": _jar_path_based_on_java_bin(ctx), - "%jvm_flags%": jvm_flags, - "%needs_runfiles%": "", - "%runfiles_manifest_only%": "", - "%set_jacoco_metadata%": "", - "%set_jacoco_main_class%": "", - "%set_jacoco_java_runfiles_root%": "", - "%workspace_prefix%": ctx.workspace_name + "/", - "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO""", - }, - is_executable = True, + + javabin = "export REAL_EXTERNAL_JAVA_BIN=${JAVABIN};JAVABIN=%s/%s" % ( + _runfiles_root(ctx), + wrapper.short_path, ) - return [] + + if use_jacoco and _coverage_replacements_provider.is_enabled(ctx): + classpath = ":".join( + ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list() + ctx.files._jacocorunner + ctx.files._lcov_merger], + ) + jacoco_metadata_file = ctx.actions.declare_file( + "%s.jacoco_metadata.txt" % ctx.attr.name, + sibling = executable, + ) + ctx.actions.write(jacoco_metadata_file, "\n".join([ + jar.short_path.replace("../", "external/") + for jar in rjars + ])) + ctx.actions.expand_template( + template = template, + output = executable, + substitutions = { + "%classpath%": classpath, + "%javabin%": javabin, + "%jarbin%": _jar_path_based_on_java_bin(ctx), + "%jvm_flags%": jvm_flags, + "%needs_runfiles%": "", + "%runfiles_manifest_only%": "", + "%workspace_prefix%": ctx.workspace_name + "/", + "%java_start_class%": "com.google.testing.coverage.JacocoCoverageRunner", + "%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), + "%set_jacoco_java_runfiles_root%": """export JACOCO_JAVA_RUNFILES_ROOT=$JAVA_RUNFILES/{}/""".format(ctx.workspace_name), + "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=YES""", + }, + is_executable = True, + ) + return [jacoco_metadata_file] + else: + # RUNPATH is defined here: + # https://github.com/bazelbuild/bazel/blob/0.4.5/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L227 + classpath = ":".join( + ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list()], + ) + ctx.actions.expand_template( + template = template, + output = executable, + substitutions = { + "%classpath%": classpath, + "%java_start_class%": main_class, + "%javabin%": javabin, + "%jarbin%": _jar_path_based_on_java_bin(ctx), + "%jvm_flags%": jvm_flags, + "%needs_runfiles%": "", + "%runfiles_manifest_only%": "", + "%set_jacoco_metadata%": "", + "%set_jacoco_main_class%": "", + "%set_jacoco_java_runfiles_root%": "", + "%workspace_prefix%": ctx.workspace_name + "/", + "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO""", + }, + is_executable = True, + ) + return [] + +def _declare_executable(ctx): + if (_is_windows(ctx)): + return ctx.actions.declare_file("%s.exe" % ctx.label.name) + else: + return ctx.actions.declare_file(ctx.label.name) def _collect_runtime_jars(dep_targets): runtime_jars = [] @@ -850,6 +873,7 @@ def scala_macro_library_impl(ctx): # Common code shared by all scala binary implementations. def _scala_binary_common( ctx, + executable, cjars, rjars, transitive_compile_time_jars, @@ -878,7 +902,7 @@ def _scala_binary_common( runfiles = ctx.runfiles( transitive_files = depset( - [ctx.outputs.executable, java_wrapper] + ctx.files._java_runtime, + [executable, java_wrapper] + ctx.files._java_runtime, transitive = [rjars], ), collect_data = True, @@ -900,8 +924,9 @@ def _scala_binary_common( java_provider = create_java_provider(scalaattr, transitive_compile_time_jars) return struct( + executable = executable, coverage = outputs.coverage, - files = depset([ctx.outputs.executable, ctx.outputs.jar]), + files = depset([executable, ctx.outputs.jar]), instrumented_files = outputs.coverage.instrumented_files, providers = [java_provider, jars2labels] + outputs.coverage.providers, runfiles = runfiles, @@ -952,8 +977,12 @@ def scala_binary_impl(ctx): (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) wrapper = _write_java_wrapper(ctx, "", "") + + executable = _declare_executable(ctx) + out = _scala_binary_common( ctx, + executable, cjars, transitive_rjars, jars.transitive_compile_jars, @@ -968,6 +997,7 @@ def scala_binary_impl(ctx): ) _write_executable( ctx = ctx, + executable = executable, jvm_flags = ctx.attr.jvm_flags, main_class = ctx.attr.main_class, rjars = out.transitive_rjars, @@ -991,6 +1021,9 @@ def scala_repl_impl(ctx): (cjars, transitive_rjars) = (jars.compile_jars, jars.transitive_runtime_jars) args = " ".join(ctx.attr.scalacopts) + + executable = _declare_executable(ctx) + wrapper = _write_java_wrapper( ctx, args, @@ -1012,6 +1045,7 @@ trap finish EXIT out = _scala_binary_common( ctx, + executable, cjars, transitive_rjars, jars.transitive_compile_jars, @@ -1026,6 +1060,7 @@ trap finish EXIT ) _write_executable( ctx = ctx, + executable = executable, jvm_flags = ["-Dscala.usejavacp=true"] + ctx.attr.jvm_flags, main_class = "scala.tools.nsc.MainGenericRunner", rjars = out.transitive_rjars, @@ -1088,10 +1123,13 @@ def scala_test_impl(ctx): "-C io.bazel.rules.scala.JUnitXmlReporter ", ]) + executable = _declare_executable(ctx) + # main_class almost has to be "org.scalatest.tools.Runner" due to args.... wrapper = _write_java_wrapper(ctx, args, "") out = _scala_binary_common( ctx, + executable, cjars, transitive_rjars, transitive_compile_jars, @@ -1119,6 +1157,7 @@ def scala_test_impl(ctx): coverage_runfiles.extend(_write_executable( ctx = ctx, + executable = executable, jvm_flags = ctx.attr.jvm_flags, main_class = ctx.attr.main_class, rjars = rjars, @@ -1127,6 +1166,7 @@ def scala_test_impl(ctx): )) return struct( + executable = executable, files = out.files, instrumented_files = out.instrumented_files, providers = out.providers, @@ -1204,9 +1244,12 @@ def scala_junit_test_impl(ctx): ctx.attr._hamcrest, ] + executable = _declare_executable(ctx) + wrapper = _write_java_wrapper(ctx, "", "") out = _scala_binary_common( ctx, + executable, cjars, transitive_rjars, jars.transitive_compile_jars, @@ -1239,6 +1282,7 @@ def scala_junit_test_impl(ctx): ] _write_executable( ctx = ctx, + executable = executable, jvm_flags = launcherJvmFlags + ctx.attr.jvm_flags, main_class = "com.google.testing.junit.runner.BazelTestRunner", rjars = out.transitive_rjars, @@ -1295,3 +1339,6 @@ def _jacoco_offline_instrument(ctx, input_jar): def _jacoco_offline_instrument_format_each(in_out_pair): return (["%s=%s" % (in_out_pair[0].path, in_out_pair[1].path)]) + +def _is_windows(ctx): + return ctx.configuration.host_path_separator == ";" diff --git a/scala/scala.bzl b/scala/scala.bzl index d201f3015..98761c594 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -67,6 +67,11 @@ _implicit_deps = { "@io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac", ), ), + "_exe": attr.label( + executable = True, + cfg = "host", + default = Label("@io_bazel_rules_scala//src/java/io/bazel/rulesscala/exe:exe"), + ), } # Single dep to allow IDEs to pickup all the implicit dependencies. @@ -472,6 +477,14 @@ def scala_repositories( server_urls = maven_servers, ) + _scala_maven_import_external( + name = "io_bazel_rules_scala_guava", + artifact = "com.google.guava:guava:21.0", + jar_sha256 = "972139718abc8a4893fa78cba8cf7b2c903f35c97aaf44fa3031b0669948b480", + licenses = ["notice"], + server_urls = maven_servers, + ) + _scala_maven_import_external( name = "io_bazel_rules_scala_org_jacoco_org_jacoco_core", artifact = "org.jacoco:org.jacoco.core:0.7.5.201505241946", @@ -487,7 +500,7 @@ def scala_repositories( licenses = ["notice"], server_urls = maven_servers, ) - + # Using this and not the bazel regular one due to issue when classpath is too long # until https://github.com/bazelbuild/bazel/issues/6955 is resolved if native.existing_rule("java_stub_template") == None: @@ -538,6 +551,11 @@ def scala_repositories( actual = "@io_bazel_rules_scala_scala_parser_combinators", ) + native.bind( + name = "io_bazel_rules_scala/dependency/scala/guava", + actual = "@io_bazel_rules_scala_guava", + ) + def _sanitize_string_for_usage(s): res_array = [] for idx in range(len(s)): diff --git a/src/java/io/bazel/rulesscala/exe/BUILD b/src/java/io/bazel/rulesscala/exe/BUILD new file mode 100644 index 000000000..3cea7c9a9 --- /dev/null +++ b/src/java/io/bazel/rulesscala/exe/BUILD @@ -0,0 +1,24 @@ +java_library( + name = "exe-lib", + srcs = [ + "LauncherFileWriter.java", + "LaunchInfo.java", + ], + deps = [ + "@bazel_tools//tools/java/runfiles:runfiles", + "//external:io_bazel_rules_scala/dependency/scala/guava", + ], + visibility = ["//visibility:public"], +) + +java_binary( + name = "exe", + main_class = "io.bazel.rulesscala.exe.LauncherFileWriter", + visibility = ["//visibility:public"], + runtime_deps = [ + ":exe-lib", + ], + data = [ + "@bazel_tools//tools/launcher:launcher", + ], +) diff --git a/src/java/io/bazel/rulesscala/exe/LaunchInfo.java b/src/java/io/bazel/rulesscala/exe/LaunchInfo.java new file mode 100644 index 000000000..8931a3a41 --- /dev/null +++ b/src/java/io/bazel/rulesscala/exe/LaunchInfo.java @@ -0,0 +1,170 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package io.bazel.rulesscala.exe; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; + + +/** + * Metadata that describes the payload of the native launcher binary. + * + *

This object constructs the binary metadata lazily, to save memory. + */ +public final class LaunchInfo { + + private final ImmutableList entries; + + private LaunchInfo(ImmutableList entries) { + this.entries = entries; + } + + /** Creates a new {@link Builder}. */ + public static Builder builder() { + return new Builder(); + } + + /** Writes this object's entries to {@code out}, returns the total written amount in bytes. */ + @VisibleForTesting + long write(OutputStream out) throws IOException { + long len = 0; + for (Entry e : entries) { + len += e.write(out); + out.write('\0'); + ++len; + } + return len; + } + + /** Writes {@code s} to {@code out} encoded as UTF-8, returns the written length in bytes. */ + private static long writeString(String s, OutputStream out) throws IOException { + byte[] b = s.getBytes(StandardCharsets.UTF_8); + out.write(b); + return b.length; + } + + /** Represents one entry in {@link LaunchInfo.entries}. */ + private static interface Entry { + /** Writes this entry to {@code out}, returns the written length in bytes. */ + long write(OutputStream out) throws IOException; + } + + /** A key-value pair entry. */ + private static final class KeyValuePair implements Entry { + private final String key; + private final String value; + + public KeyValuePair(String key, String value) { + this.key = Preconditions.checkNotNull(key); + this.value = value; + } + + @Override + public long write(OutputStream out) throws IOException { + long len = writeString(key, out); + len += writeString("=", out); + if (value != null && !value.isEmpty()) { + len += writeString(value, out); + } + return len; + } + } + + /** A pair of a key and a delimiter-joined list of values. */ + private static final class JoinedValues implements Entry { + private final String key; + private final String delimiter; + private final Iterable values; + + public JoinedValues(String key, String delimiter, Iterable values) { + this.key = Preconditions.checkNotNull(key); + this.delimiter = Preconditions.checkNotNull(delimiter); + this.values = values; + } + + @Override + public long write(OutputStream out) throws IOException { + long len = writeString(key, out); + len += writeString("=", out); + if (values != null) { + boolean first = true; + for (String v : values) { + if (first) { + first = false; + } else { + len += writeString(delimiter, out); + } + len += writeString(v, out); + } + } + return len; + } + } + + /** Builder for {@link LaunchInfo} instances. */ + public static final class Builder { + private ImmutableList.Builder entries = ImmutableList.builder(); + + /** Builds a {@link LaunchInfo} from this builder. This builder may be reused. */ + public LaunchInfo build() { + return new LaunchInfo(entries.build()); + } + + /** + * Adds a key-value pair entry. + * + *

Examples: + * + *

+ */ + public Builder addKeyValuePair(String key, String value) { + Preconditions.checkNotNull(key); + if (!key.isEmpty()) { + entries.add(new KeyValuePair(key, value)); + } + return this; + } + + /** + * Adds a key and list of lazily-joined values. + * + *

Examples: + * + *

+ */ + public Builder addJoinedValues( + String key, String delimiter, Iterable values) { + Preconditions.checkNotNull(key); + Preconditions.checkNotNull(delimiter); + if (!key.isEmpty()) { + entries.add(new JoinedValues(key, delimiter, values)); + } + return this; + } + } +} \ No newline at end of file diff --git a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java new file mode 100644 index 000000000..1e7c09b69 --- /dev/null +++ b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java @@ -0,0 +1,56 @@ +package io.bazel.rulesscala.exe; + +import com.google.common.base.Preconditions; +import com.google.common.io.ByteStreams; +import com.google.devtools.build.runfiles.Runfiles; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.file.*; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Arrays; +import java.util.List; + + +public class LauncherFileWriter { + public static void main(String[] args) throws IOException { + Preconditions.checkArgument(args.length == 6); + + final String location = args[0]; + final String workspaceName = args[1]; + final String javaBinPath = args[2]; + final String jarBinPath = javaBinPath.substring(0, javaBinPath.lastIndexOf('/')) + "/jar.exe"; + final String javaStartClass = args[3]; + final String classpath = args[4]; + final List jvmFlags = Arrays.asList(args[5].split(";")); + + LaunchInfo launchInfo = + LaunchInfo.builder() + .addKeyValuePair("binary_type", "Java") + .addKeyValuePair("workspace_name", workspaceName) + .addKeyValuePair("symlink_runfiles_enabled", "0") + .addKeyValuePair("java_bin_path", javaBinPath) + .addKeyValuePair("jar_bin_path", jarBinPath) + .addKeyValuePair("java_start_class", javaStartClass) + .addKeyValuePair("classpath", classpath) + .addJoinedValues("jvm_flags", " ", jvmFlags) + .build(); + + Path launcher = Paths.get(Runfiles.create().rlocation("bazel_tools/tools/launcher/launcher.exe")); + Path outPath = Paths.get(location); + + try (InputStream in = Files.newInputStream(launcher); OutputStream out = Files.newOutputStream(outPath)) { + ByteStreams.copy(in, out); + + long dataLength = launchInfo.write(out); + ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES); + buffer.order(ByteOrder.LITTLE_ENDIAN); + buffer.putLong(dataLength); + out.write(buffer.array()); + + out.flush(); + } + } +} diff --git a/src/scala/io/bazel/rules_scala/jmh_support/BenchmarkGenerator.scala b/src/scala/io/bazel/rules_scala/jmh_support/BenchmarkGenerator.scala index 579e8ddab..d367ff424 100644 --- a/src/scala/io/bazel/rules_scala/jmh_support/BenchmarkGenerator.scala +++ b/src/scala/io/bazel/rules_scala/jmh_support/BenchmarkGenerator.scala @@ -87,7 +87,11 @@ object BenchmarkGenerator { } private def collectClassesFromJar(root: Path): List[Path] = { - val uri = new URI("jar:file", null, root.toFile.getAbsolutePath, null) + val path = if (System.getProperty("os.name").toLowerCase().contains("windows")) + "/" + root.toFile.getAbsolutePath.replaceAll("\\\\", "/") + else + root.toFile.getAbsolutePath + val uri = new URI("jar:file", null, path, null) val fs = FileSystems.newFileSystem(uri, Map.empty[String, String].asJava) fs.getRootDirectories.asScala.toList.flatMap { rootDir => listFilesRecursively(rootDir) { (path: Path) => From 373f501e4012ca73d3d4a595fae0b55adf777d26 Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Fri, 5 Apr 2019 17:27:47 +0200 Subject: [PATCH 02/11] Fix UnusedDependencyChecker Windows issue --- .../unuseddependencychecker/UnusedDependencyChecker.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/third_party/unused_dependency_checker/src/main/io/bazel/rulesscala/unuseddependencychecker/UnusedDependencyChecker.scala b/third_party/unused_dependency_checker/src/main/io/bazel/rulesscala/unuseddependencychecker/UnusedDependencyChecker.scala index 0334ec0b6..469db9093 100644 --- a/third_party/unused_dependency_checker/src/main/io/bazel/rulesscala/unuseddependencychecker/UnusedDependencyChecker.scala +++ b/third_party/unused_dependency_checker/src/main/io/bazel/rulesscala/unuseddependencychecker/UnusedDependencyChecker.scala @@ -16,6 +16,8 @@ class UnusedDependencyChecker(val global: Global) extends Plugin { self => var analyzerMode: AnalyzerMode = Error var currentTarget: String = "NA" + val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("windows") + override def init(options: List[String], error: (String) => Unit): Boolean = { var directJars: Seq[String] = Seq.empty var directTargets: Seq[String] = Seq.empty @@ -66,7 +68,7 @@ class UnusedDependencyChecker(val global: Global) extends Plugin { self => private def unusedDependenciesFound: Set[String] = { val usedJars: Set[AbstractFile] = findUsedJars val directJarPaths = direct.keys.toSet - val usedJarPaths = usedJars.map(_.path) + val usedJarPaths = if (!isWindows) usedJars.map(_.path) else usedJars.map(_.path.replaceAll("\\\\", "/")) val usedTargets = usedJarPaths .map(direct.get) From 0c25ac28932b764fa1b9a2c6f98fc1abe499fd0e Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Wed, 20 Mar 2019 17:06:32 +0100 Subject: [PATCH 03/11] Fix scala_test rule for Windows --- scala/private/rule_impls.bzl | 26 ++++++--- src/java/io/bazel/rulesscala/scala_test/BUILD | 1 + .../bazel/rulesscala/scala_test/Runner.java | 56 ++++++++++++++++++- 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index b6e245dad..4efb6294a 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -881,7 +881,8 @@ def _scala_binary_common( java_wrapper, unused_dependency_checker_mode, unused_dependency_checker_ignored_targets, - implicit_junit_deps_needed_for_java_compilation = []): + implicit_junit_deps_needed_for_java_compilation = [], + runfiles_ext = []): write_manifest(ctx) outputs = _compile_or_empty( ctx, @@ -902,7 +903,7 @@ def _scala_binary_common( runfiles = ctx.runfiles( transitive_files = depset( - [executable, java_wrapper] + ctx.files._java_runtime, + [executable, java_wrapper] + ctx.files._java_runtime + runfiles_ext, transitive = [rjars], ), collect_data = True, @@ -1117,16 +1118,21 @@ def scala_test_impl(ctx): jars.jars2labels, ) - args = " ".join([ - "-R \"{path}\"".format(path = ctx.outputs.jar.short_path), + args = "\n".join([ + "-R", ctx.outputs.jar.short_path, _scala_test_flags(ctx), - "-C io.bazel.rules.scala.JUnitXmlReporter ", + "-C", "io.bazel.rules.scala.JUnitXmlReporter", ]) + argsFile = ctx.actions.declare_file("%s.args" % ctx.label.name) + ctx.actions.write( + argsFile, + args + ) + executable = _declare_executable(ctx) - # main_class almost has to be "org.scalatest.tools.Runner" due to args.... - wrapper = _write_java_wrapper(ctx, args, "") + wrapper = _write_java_wrapper(ctx, "", "") out = _scala_binary_common( ctx, executable, @@ -1138,6 +1144,7 @@ def scala_test_impl(ctx): unused_dependency_checker_ignored_targets = unused_dependency_checker_ignored_targets, unused_dependency_checker_mode = unused_dependency_checker_mode, + runfiles_ext = [argsFile] ) rjars = out.transitive_rjars @@ -1158,7 +1165,10 @@ def scala_test_impl(ctx): coverage_runfiles.extend(_write_executable( ctx = ctx, executable = executable, - jvm_flags = ctx.attr.jvm_flags, + jvm_flags = [ + "-DRULES_SCALA_WS=%s" % ctx.workspace_name, + "-DRULES_SCALA_ARGS_FILE=%s" % argsFile.short_path + ] + ctx.attr.jvm_flags, main_class = ctx.attr.main_class, rjars = rjars, use_jacoco = ctx.configuration.coverage_enabled, diff --git a/src/java/io/bazel/rulesscala/scala_test/BUILD b/src/java/io/bazel/rulesscala/scala_test/BUILD index 34c233909..8bbf24723 100644 --- a/src/java/io/bazel/rulesscala/scala_test/BUILD +++ b/src/java/io/bazel/rulesscala/scala_test/BUILD @@ -4,5 +4,6 @@ java_library( visibility = ["//visibility:public"], deps = [ "//external:io_bazel_rules_scala/dependency/scalatest/scalatest", + "@bazel_tools//tools/java/runfiles:runfiles", ], ) diff --git a/src/java/io/bazel/rulesscala/scala_test/Runner.java b/src/java/io/bazel/rulesscala/scala_test/Runner.java index 14cfe549f..494f0e370 100644 --- a/src/java/io/bazel/rulesscala/scala_test/Runner.java +++ b/src/java/io/bazel/rulesscala/scala_test/Runner.java @@ -1,6 +1,12 @@ package io.bazel.rulesscala.scala_test; +import com.google.devtools.build.runfiles.Runfiles; +import java.io.IOException; +import java.util.List; import java.util.Map; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Paths; /** This exists only as a proxy for scala tests's runner to provide access to env variables */ public class Runner { @@ -9,12 +15,56 @@ public class Runner { */ private static final String TESTBRIDGE_TEST_ONLY = "TESTBRIDGE_TEST_ONLY"; - public static void main(String[] args) { + /** + * This is the name of the system property used to pass Bazel's workspace name + */ + private static final String RULES_SCALA_WS = "RULES_SCALA_WS"; + + /** + * This is the name of the system property used to pass a short path of the file, which includes + * org.scalatest.tools.Runner arguments + */ + private static final String RULES_SCALA_ARGS_FILE = "RULES_SCALA_ARGS_FILE"; + + public static void main(String[] args) throws IOException { org.scalatest.tools.Runner.main(extendArgs(args, System.getenv())); } - private static String[] extendArgs(String[] args, Map env) { - return extendFromEnvVar(args, env, TESTBRIDGE_TEST_ONLY, "-s"); + private static String[] extendArgs(String[] args, Map env) throws IOException { + args = extendFromSystemPropArgs(args); + args = extendFromEnvVar(args, env, TESTBRIDGE_TEST_ONLY, "-s"); + return args; + } + + private static String[] extendFromSystemPropArgs(String[] args) throws IOException { + String rulesWorkspace = System.getProperty(RULES_SCALA_WS); + if (rulesWorkspace == null || rulesWorkspace.trim().isEmpty()) + throw new IllegalArgumentException(RULES_SCALA_WS + " is null or empty."); + + String rulesArgsKey = System.getProperty(RULES_SCALA_ARGS_FILE); + if (rulesArgsKey == null || rulesArgsKey.trim().isEmpty()) + throw new IllegalArgumentException(RULES_SCALA_ARGS_FILE + " is null or empty."); + + String rulesArgsPath = Runfiles.create().rlocation(rulesWorkspace + "/" + rulesArgsKey); + if (rulesArgsPath == null) + throw new IllegalArgumentException("rlocation value is null for key: " + rulesArgsKey); + + List runnerArgs = Files.readAllLines(Paths.get(rulesArgsPath), Charset.forName("UTF-8")); + + int runpathFlag = runnerArgs.indexOf("-R"); + if (runpathFlag >= 0) { + String runpathKey = runnerArgs.get(runpathFlag + 1); + String runpath = Runfiles.create().rlocation(rulesWorkspace + "/" + runpathKey); + runnerArgs.set(runpathFlag + 1, runpath); + } + + String[] runnerArgsArray = runnerArgs.toArray(new String[runnerArgs.size()]); + + String[] result = new String[args.length + runnerArgsArray.length]; + System.arraycopy(args, 0, result, 0, args.length); + System.arraycopy(runnerArgsArray, 0, result, args.length, runnerArgsArray.length); + + return result; } private static String[] extendFromEnvVar( From eb34a2ee281c3f58f06acb22f7aafb5a0e468357 Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Tue, 26 Mar 2019 15:43:41 +0100 Subject: [PATCH 04/11] Fix scala_test rule for Windows - fixes too long paths issue --- scala/scala.bzl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 98761c594..310e886bc 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -572,10 +572,13 @@ def scala_test_suite( name, srcs = [], visibility = None, + use_short_names = False, **kwargs): ts = [] + i = 0 for test_file in srcs: - n = "%s_test_suite_%s" % (name, _sanitize_string_for_usage(test_file)) + i = i+1 + n = ("%s_%s" % (name, i)) if use_short_names else ("%s_test_suite_%s" % (name, _sanitize_string_for_usage(test_file))) scala_test( name = n, srcs = [test_file], From 27bc489b35e338ea549a275d4d835746f79b9db7 Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Tue, 26 Mar 2019 15:44:31 +0100 Subject: [PATCH 05/11] Fix scala_test rule for Windows - fixes jar.exe rlocation issue --- scala/private/rule_impls.bzl | 2 +- src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 4efb6294a..afb936280 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -596,7 +596,7 @@ def _write_executable(ctx, executable, rjars, main_class, jvm_flags, wrapper, us [("external/%s" % (j.short_path[3:]) if j.short_path.startswith("../") else j.short_path) for j in rjars.to_list()], ) jvm_flags_str = ";".join(jvm_flags) - java_for_exe = "%s/%s" % (ctx.workspace_name, str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path)) + java_for_exe = str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path) ctx.actions.run( outputs = [executable], diff --git a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java index 1e7c09b69..fc2367494 100644 --- a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java +++ b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java @@ -31,7 +31,7 @@ public static void main(String[] args) throws IOException { .addKeyValuePair("binary_type", "Java") .addKeyValuePair("workspace_name", workspaceName) .addKeyValuePair("symlink_runfiles_enabled", "0") - .addKeyValuePair("java_bin_path", javaBinPath) + .addKeyValuePair("java_bin_path", workspaceName + "/" + javaBinPath) .addKeyValuePair("jar_bin_path", jarBinPath) .addKeyValuePair("java_start_class", javaStartClass) .addKeyValuePair("classpath", classpath) From b0f2e5acdcce2e4895900c89b9156fc2bab2e76e Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Tue, 26 Mar 2019 15:45:38 +0100 Subject: [PATCH 06/11] Fix scala_test rule for Windows - fixes slash/backslash issue in scalac processor --- src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index fb09ccc75..3d794f350 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -287,11 +287,11 @@ private static void copyResources( } else { dstr = resource.destination; } - if (dstr.charAt(0) == '/') { + if (dstr.charAt(0) == '/' || dstr.charAt(0) == '\\') { // we don't want to copy to an absolute destination dstr = dstr.substring(1); } - if (dstr.startsWith("../")) { + if (dstr.startsWith("../") || dstr.startsWith("..\\")) { // paths to external repositories, for some reason, start with a leading ../ // we don't want to copy the resource out of our temporary directory, so // instead we replace ../ with external/ From b117954ec7e0d420d5a8d7d9e7a247695188c56b Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Wed, 3 Apr 2019 14:55:54 +0200 Subject: [PATCH 07/11] Fix ScalacProcessor tmp cleanup on Windows --- src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 3d794f350..0eb1e16c1 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -251,6 +251,7 @@ private static void removeTmp(Path tmp) throws IOException { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + file.toFile().setWritable(true); Files.delete(file); return FileVisitResult.CONTINUE; } From 5536aa85163bfb1be6bd7a360684a55b070ce7bb Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Fri, 5 Apr 2019 16:19:40 +0200 Subject: [PATCH 08/11] Fix too long classpath issue on Windows --- scala/private/rule_impls.bzl | 12 ++++++------ .../io/bazel/rulesscala/exe/LauncherFileWriter.java | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index afb936280..fa18e98b1 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -598,11 +598,14 @@ def _write_executable(ctx, executable, rjars, main_class, jvm_flags, wrapper, us jvm_flags_str = ";".join(jvm_flags) java_for_exe = str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path) + cpfile = ctx.actions.declare_file("%s.classpath" % ctx.label.name) + ctx.actions.write(cpfile, classpath) + ctx.actions.run( outputs = [executable], - inputs = [], + inputs = [cpfile], executable = ctx.attr._exe.files_to_run.executable, - arguments = [executable.path, ctx.workspace_name, java_for_exe, main_class, classpath, jvm_flags_str], + arguments = [executable.path, ctx.workspace_name, java_for_exe, main_class, cpfile.path, jvm_flags_str], mnemonic = "ExeLauncher", progress_message = "Creating exe launcher", ) @@ -1125,10 +1128,7 @@ def scala_test_impl(ctx): ]) argsFile = ctx.actions.declare_file("%s.args" % ctx.label.name) - ctx.actions.write( - argsFile, - args - ) + ctx.actions.write(argsFile, args) executable = _declare_executable(ctx) diff --git a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java index fc2367494..f54a57819 100644 --- a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java +++ b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java @@ -23,8 +23,9 @@ public static void main(String[] args) throws IOException { final String javaBinPath = args[2]; final String jarBinPath = javaBinPath.substring(0, javaBinPath.lastIndexOf('/')) + "/jar.exe"; final String javaStartClass = args[3]; - final String classpath = args[4]; + final String cpFile = args[4]; final List jvmFlags = Arrays.asList(args[5].split(";")); + final String classpath = Files.readAllLines(Paths.get(cpFile)).get(0); LaunchInfo launchInfo = LaunchInfo.builder() From 2546a90d981029f562a17a93944e02b0bee604c2 Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Fri, 5 Apr 2019 16:20:51 +0200 Subject: [PATCH 09/11] Fix scrooge_support problems on Windows --- .../bazel/rules_scala/scrooge_support/FocusedZipImporter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scala/io/bazel/rules_scala/scrooge_support/FocusedZipImporter.scala b/src/scala/io/bazel/rules_scala/scrooge_support/FocusedZipImporter.scala index f049bb8b0..e3c6ceb12 100644 --- a/src/scala/io/bazel/rules_scala/scrooge_support/FocusedZipImporter.scala +++ b/src/scala/io/bazel/rules_scala/scrooge_support/FocusedZipImporter.scala @@ -32,7 +32,7 @@ case class FocusedZipImporter(focus: Option[File], zips: List[File], zipFiles: L case child :: tail => loop(new File(leftPart, child), tail) } val parts = n.split("/", -1).toList - val newPath = loop(f, parts).toString + val newPath = loop(f, parts).getPath.replaceAllLiterally(File.separator, "/") if (parts(0) == File.pathSeparatorChar) newPath.substring(1) else newPath } From 73ba62a3347309f8df60b1f67a296eb9792aa8ad Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Mon, 8 Apr 2019 15:45:08 +0200 Subject: [PATCH 10/11] Windows CI - #683 --- .bazelrc.travis | 9 +++---- .travis.yml | 58 +++++++++++++++++++++++++--------------- test_reproducibility.ps1 | 3 +++ test_rules_scala.ps1 | 35 ++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 26 deletions(-) create mode 100644 test_reproducibility.ps1 create mode 100644 test_rules_scala.ps1 diff --git a/.bazelrc.travis b/.bazelrc.travis index b1d24691c..6ba330e33 100644 --- a/.bazelrc.travis +++ b/.bazelrc.travis @@ -1,11 +1,10 @@ # This is from Bazel's former travis setup, to avoid blowing up the RAM usage. -startup --host_jvm_args=-Xmx2500m -startup --host_jvm_args=-Xms2500m +startup --host_jvm_args=-Xmx3072m +startup --host_jvm_args=-Xms1024m # startup --batch # we actually start many bazels in the test script, we don't want batch -test --ram_utilization_factor=10 # Just making sure that we don't OOM with parallel builds -build --local_resources 2048,.5,1.0 +build --local_resources 3072,.5,1.0 # This is so we understand failures better build --verbose_failures @@ -17,7 +16,7 @@ build --verbose_failures # runs stuff in a container, and since Travis already runs its script # in a container (unless you require sudo in your .travis.yml) this # fails to run tests. -build --strategy=Scalac=worker --strategy=ScroogeRule=worker --worker_max_instances=3 +build --strategy=Scalac=worker --strategy=ScroogeRule=worker --worker_max_instances=2 #test --test_strategy=standalone # Below this line, .travis.yml will cat the default bazelrc. diff --git a/.travis.yml b/.travis.yml index 9e19f23ac..d23b486b8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ sudo: required # xcode8 has jdk8 osx_image: xcode8 # Not technically required but suppresses 'Ruby' in Job status message. -language: java +language: sh cache: directories: @@ -13,38 +13,54 @@ cache: os: - linux - osx + - windows env: # Linting is broken. Disable until fixed. # See https://github.com/bazelbuild/rules_scala/pull/622 # we want to test the last release #- V=0.16.1 TEST_SCRIPT=test_lint.sh - - V=0.23.1 TEST_SCRIPT=test_rules_scala.sh + - V=0.23.1 TEST_SCRIPT=test_rules_scala #- V=0.14.1 TEST_SCRIPT=test_intellij_aspect.sh - - V=0.23.1 TEST_SCRIPT=test_reproducibility.sh + - V=0.23.1 TEST_SCRIPT=test_reproducibility + before_install: - | - if [[ "${TRAVIS_OS_NAME}" == "osx" ]]; then - OS=darwin - else - sysctl kernel.unprivileged_userns_clone=1 - sudo apt-get update -q - sudo apt-get install libxml2-utils -y - OS=linux - fi - if [[ $V =~ .*rc[0-9]+.* ]]; then - PRE_RC=$(expr "$V" : '\([0-9.]*\)rc.*') - RC_PRC=$(expr "$V" : '[0-9.]*\(rc.*\)') - URL="https://storage.googleapis.com/bazel/${PRE_RC}/${RC_PRC}/bazel-${V}-installer-${OS}-x86_64.sh" + if [[ "${TRAVIS_OS_NAME}" == "windows" ]]; then + choco install jdk8 -params 'installdir=c:\\java8' + choco install bazel --version ${V} else - URL="https://github.com/bazelbuild/bazel/releases/download/${V}/bazel-${V}-installer-${OS}-x86_64.sh" + if [[ "${TRAVIS_OS_NAME}" == "osx" ]]; then + OS=darwin + else + sudo sysctl kernel.unprivileged_userns_clone=1 + sudo add-apt-repository -y ppa:openjdk-r/ppa + sudo apt-get update -q + sudo apt-get install openjdk-8-jdk -y + sudo apt-get install libxml2-utils -y + OS=linux + fi + + if [[ $V =~ .*rc[0-9]+.* ]]; then + PRE_RC=$(expr "$V" : '\([0-9.]*\)rc.*') + RC_PRC=$(expr "$V" : '[0-9.]*\(rc.*\)') + URL="https://storage.googleapis.com/bazel/${PRE_RC}/${RC_PRC}/bazel-${V}-installer-${OS}-x86_64.sh" + else + URL="https://github.com/bazelbuild/bazel/releases/download/${V}/bazel-${V}-installer-${OS}-x86_64.sh" + fi + wget -O install.sh "${URL}" + chmod +x install.sh + ./install.sh --user + rm -f install.sh fi - wget -O install.sh "${URL}" - chmod +x install.sh - ./install.sh --user - rm -f install.sh - cat .bazelrc.travis >> .bazelrc script: - - bash $TEST_SCRIPT ci + - | + if [[ "${TRAVIS_OS_NAME}" == "windows" ]]; then + powershell -Command 'Set-ExecutionPolicy RemoteSigned -scope CurrentUser' + powershell -File ./${TEST_SCRIPT}.ps1 + else + bash ./${TEST_SCRIPT}.sh ci + fi diff --git a/test_reproducibility.ps1 b/test_reproducibility.ps1 new file mode 100644 index 000000000..e70786fc9 --- /dev/null +++ b/test_reproducibility.ps1 @@ -0,0 +1,3 @@ +Set-StrictMode -Version latest +$ErrorActionPreference = 'Stop' + diff --git a/test_rules_scala.ps1 b/test_rules_scala.ps1 new file mode 100644 index 000000000..65324062e --- /dev/null +++ b/test_rules_scala.ps1 @@ -0,0 +1,35 @@ +#!/usr/bin/env pwsh + +Set-StrictMode -Version latest +$ErrorActionPreference = 'Stop' + +$env:JAVA_HOME='c:\\java8' + +function bazel() { + Write-Output ">> bazel $args" + $global:lastexitcode = 0 + $backupErrorActionPreference = $script:ErrorActionPreference + $script:ErrorActionPreference = "Continue" + & bazel.exe @args 2>&1 | %{ "$_" } + $script:ErrorActionPreference = $backupErrorActionPreference + if ($global:lastexitcode -ne 0 -And $args[0] -ne "shutdown") { + Write-Output "<< bazel $args (failed, exit code: $global:lastexitcode)" + throw ("Bazel returned non-zero exit code: $global:lastexitcode") + } + Write-Output "<< bazel $args (ok)" +} + +bazel build //test/... +bazel shutdown + +bazel test ` + //test:HelloLibTest ` + //test:HelloLibTestSuite_test_suite_HelloLibTest.scala ` + //test:HelloLibTestSuite_test_suite_HelloLibTest2.scala ` + //test:TestFilterTests ` + //test:no_sig ` + //test/aspect:aspect_test ` + //test/aspect:scala_test ` + //test/proto:test_blacklisted_proto ` + //test/src/main/scala/scalarules/test/resource_jars:resource_jars ` + //test/src/main/scala/scalarules/test/twitter_scrooge/... From beefdfca8f86897be7e046ea02f484c3b334e470 Mon Sep 17 00:00:00 2001 From: Michal Majcherski Date: Mon, 29 Apr 2019 15:30:48 +0200 Subject: [PATCH 11/11] Windows support - code review fixes --- scala/private/rule_impls.bzl | 176 +++++++++--------- src/java/io/bazel/rulesscala/exe/BUILD | 8 +- .../io/bazel/rulesscala/exe/LaunchInfo.java | 2 - src/java/io/bazel/rulesscala/scala_test/BUILD | 2 +- .../bazel/rulesscala/scala_test/Runner.java | 58 +++--- .../rulesscala/scalac/ScalacProcessor.java | 9 +- test/BUILD | 7 + .../LongNamesTest.scala | 9 + test_rules_scala.ps1 | 1 + 9 files changed, 156 insertions(+), 116 deletions(-) create mode 100644 test/longnames/looooooongnaaaaaaame/anooooootherlooooooooongname/anooooootherlooooooooongname2/anooooootherlooooooooongname3/LongNamesTest.scala diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index fa18e98b1..6ae6c4084 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -592,94 +592,102 @@ def _jar_path_based_on_java_bin(ctx): def _write_executable(ctx, executable, rjars, main_class, jvm_flags, wrapper, use_jacoco): if (_is_windows(ctx)): - classpath = ";".join( - [("external/%s" % (j.short_path[3:]) if j.short_path.startswith("../") else j.short_path) for j in rjars.to_list()], + return _write_executable_windows(ctx, executable, rjars, main_class, jvm_flags, wrapper, use_jacoco) + else: + return _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags, wrapper, use_jacoco) + +def _write_executable_windows(ctx, executable, rjars, main_class, jvm_flags, wrapper, use_jacoco): + # NOTE: `use_jacoco` is currently ignored on Windows. + # TODO: tests coverage support for Windows + classpath = ";".join( + [("external/%s" % (j.short_path[3:]) if j.short_path.startswith("../") else j.short_path) for j in rjars.to_list()], + ) + jvm_flags_str = ";".join(jvm_flags) + java_for_exe = str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path) + + cpfile = ctx.actions.declare_file("%s.classpath" % ctx.label.name) + ctx.actions.write(cpfile, classpath) + + ctx.actions.run( + outputs = [executable], + inputs = [cpfile], + executable = ctx.attr._exe.files_to_run.executable, + arguments = [executable.path, ctx.workspace_name, java_for_exe, main_class, cpfile.path, jvm_flags_str], + mnemonic = "ExeLauncher", + progress_message = "Creating exe launcher", + ) + return [] + +def _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags, wrapper, use_jacoco): + template = ctx.attr._java_stub_template.files.to_list()[0] + + jvm_flags = " ".join( + [ctx.expand_location(f, ctx.attr.data) for f in jvm_flags], + ) + + javabin = "export REAL_EXTERNAL_JAVA_BIN=${JAVABIN};JAVABIN=%s/%s" % ( + _runfiles_root(ctx), + wrapper.short_path, + ) + + if use_jacoco and _coverage_replacements_provider.is_enabled(ctx): + classpath = ":".join( + ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list() + ctx.files._jacocorunner + ctx.files._lcov_merger], ) - jvm_flags_str = ";".join(jvm_flags) - java_for_exe = str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path) - - cpfile = ctx.actions.declare_file("%s.classpath" % ctx.label.name) - ctx.actions.write(cpfile, classpath) - - ctx.actions.run( - outputs = [executable], - inputs = [cpfile], - executable = ctx.attr._exe.files_to_run.executable, - arguments = [executable.path, ctx.workspace_name, java_for_exe, main_class, cpfile.path, jvm_flags_str], - mnemonic = "ExeLauncher", - progress_message = "Creating exe launcher", + jacoco_metadata_file = ctx.actions.declare_file( + "%s.jacoco_metadata.txt" % ctx.attr.name, + sibling = executable, ) - return [] + ctx.actions.write(jacoco_metadata_file, "\n".join([ + jar.short_path.replace("../", "external/") + for jar in rjars + ])) + ctx.actions.expand_template( + template = template, + output = executable, + substitutions = { + "%classpath%": classpath, + "%javabin%": javabin, + "%jarbin%": _jar_path_based_on_java_bin(ctx), + "%jvm_flags%": jvm_flags, + "%needs_runfiles%": "", + "%runfiles_manifest_only%": "", + "%workspace_prefix%": ctx.workspace_name + "/", + "%java_start_class%": "com.google.testing.coverage.JacocoCoverageRunner", + "%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), + "%set_jacoco_java_runfiles_root%": """export JACOCO_JAVA_RUNFILES_ROOT=$JAVA_RUNFILES/{}/""".format(ctx.workspace_name), + "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=YES""", + }, + is_executable = True, + ) + return [jacoco_metadata_file] else: - template = ctx.attr._java_stub_template.files.to_list()[0] - - jvm_flags = " ".join( - [ctx.expand_location(f, ctx.attr.data) for f in jvm_flags], + # RUNPATH is defined here: + # https://github.com/bazelbuild/bazel/blob/0.4.5/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L227 + classpath = ":".join( + ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list()], ) - - javabin = "export REAL_EXTERNAL_JAVA_BIN=${JAVABIN};JAVABIN=%s/%s" % ( - _runfiles_root(ctx), - wrapper.short_path, + ctx.actions.expand_template( + template = template, + output = executable, + substitutions = { + "%classpath%": classpath, + "%java_start_class%": main_class, + "%javabin%": javabin, + "%jarbin%": _jar_path_based_on_java_bin(ctx), + "%jvm_flags%": jvm_flags, + "%needs_runfiles%": "", + "%runfiles_manifest_only%": "", + "%set_jacoco_metadata%": "", + "%set_jacoco_main_class%": "", + "%set_jacoco_java_runfiles_root%": "", + "%workspace_prefix%": ctx.workspace_name + "/", + "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO""", + }, + is_executable = True, ) - - if use_jacoco and _coverage_replacements_provider.is_enabled(ctx): - classpath = ":".join( - ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list() + ctx.files._jacocorunner + ctx.files._lcov_merger], - ) - jacoco_metadata_file = ctx.actions.declare_file( - "%s.jacoco_metadata.txt" % ctx.attr.name, - sibling = executable, - ) - ctx.actions.write(jacoco_metadata_file, "\n".join([ - jar.short_path.replace("../", "external/") - for jar in rjars - ])) - ctx.actions.expand_template( - template = template, - output = executable, - substitutions = { - "%classpath%": classpath, - "%javabin%": javabin, - "%jarbin%": _jar_path_based_on_java_bin(ctx), - "%jvm_flags%": jvm_flags, - "%needs_runfiles%": "", - "%runfiles_manifest_only%": "", - "%workspace_prefix%": ctx.workspace_name + "/", - "%java_start_class%": "com.google.testing.coverage.JacocoCoverageRunner", - "%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), - "%set_jacoco_java_runfiles_root%": """export JACOCO_JAVA_RUNFILES_ROOT=$JAVA_RUNFILES/{}/""".format(ctx.workspace_name), - "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=YES""", - }, - is_executable = True, - ) - return [jacoco_metadata_file] - else: - # RUNPATH is defined here: - # https://github.com/bazelbuild/bazel/blob/0.4.5/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L227 - classpath = ":".join( - ["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list()], - ) - ctx.actions.expand_template( - template = template, - output = executable, - substitutions = { - "%classpath%": classpath, - "%java_start_class%": main_class, - "%javabin%": javabin, - "%jarbin%": _jar_path_based_on_java_bin(ctx), - "%jvm_flags%": jvm_flags, - "%needs_runfiles%": "", - "%runfiles_manifest_only%": "", - "%set_jacoco_metadata%": "", - "%set_jacoco_main_class%": "", - "%set_jacoco_java_runfiles_root%": "", - "%workspace_prefix%": ctx.workspace_name + "/", - "%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO""", - }, - is_executable = True, - ) - return [] + return [] def _declare_executable(ctx): if (_is_windows(ctx)): @@ -1166,7 +1174,7 @@ def scala_test_impl(ctx): ctx = ctx, executable = executable, jvm_flags = [ - "-DRULES_SCALA_WS=%s" % ctx.workspace_name, + "-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name, "-DRULES_SCALA_ARGS_FILE=%s" % argsFile.short_path ] + ctx.attr.jvm_flags, main_class = ctx.attr.main_class, diff --git a/src/java/io/bazel/rulesscala/exe/BUILD b/src/java/io/bazel/rulesscala/exe/BUILD index 3cea7c9a9..da9e98fe7 100644 --- a/src/java/io/bazel/rulesscala/exe/BUILD +++ b/src/java/io/bazel/rulesscala/exe/BUILD @@ -5,20 +5,20 @@ java_library( "LaunchInfo.java", ], deps = [ - "@bazel_tools//tools/java/runfiles:runfiles", + "@bazel_tools//tools/java/runfiles", "//external:io_bazel_rules_scala/dependency/scala/guava", ], - visibility = ["//visibility:public"], + visibility = ["//visibility:private"], ) java_binary( name = "exe", main_class = "io.bazel.rulesscala.exe.LauncherFileWriter", - visibility = ["//visibility:public"], runtime_deps = [ ":exe-lib", ], data = [ - "@bazel_tools//tools/launcher:launcher", + "@bazel_tools//tools/launcher", ], + visibility = ["//visibility:public"], ) diff --git a/src/java/io/bazel/rulesscala/exe/LaunchInfo.java b/src/java/io/bazel/rulesscala/exe/LaunchInfo.java index 8931a3a41..1c4f76d40 100644 --- a/src/java/io/bazel/rulesscala/exe/LaunchInfo.java +++ b/src/java/io/bazel/rulesscala/exe/LaunchInfo.java @@ -14,7 +14,6 @@ package io.bazel.rulesscala.exe; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import java.io.IOException; @@ -41,7 +40,6 @@ public static Builder builder() { } /** Writes this object's entries to {@code out}, returns the total written amount in bytes. */ - @VisibleForTesting long write(OutputStream out) throws IOException { long len = 0; for (Entry e : entries) { diff --git a/src/java/io/bazel/rulesscala/scala_test/BUILD b/src/java/io/bazel/rulesscala/scala_test/BUILD index 8bbf24723..2b4f1d851 100644 --- a/src/java/io/bazel/rulesscala/scala_test/BUILD +++ b/src/java/io/bazel/rulesscala/scala_test/BUILD @@ -4,6 +4,6 @@ java_library( visibility = ["//visibility:public"], deps = [ "//external:io_bazel_rules_scala/dependency/scalatest/scalatest", - "@bazel_tools//tools/java/runfiles:runfiles", + "@bazel_tools//tools/java/runfiles", ], ) diff --git a/src/java/io/bazel/rulesscala/scala_test/Runner.java b/src/java/io/bazel/rulesscala/scala_test/Runner.java index 494f0e370..d786acd60 100644 --- a/src/java/io/bazel/rulesscala/scala_test/Runner.java +++ b/src/java/io/bazel/rulesscala/scala_test/Runner.java @@ -2,13 +2,17 @@ import com.google.devtools.build.runfiles.Runfiles; import java.io.IOException; +import java.io.File; import java.util.List; import java.util.Map; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Paths; -/** This exists only as a proxy for scala tests's runner to provide access to env variables */ +/** This exists only as a proxy for scala tests's runner to: + * - provide access to env variables + * - unwrap runner's arguments from a file (passed via file to overcome command-line string limitation on Windows) + **/ public class Runner { /** * This is the name of the env var set by bazel when a user provides a `--test_filter` test option @@ -16,9 +20,9 @@ public class Runner { private static final String TESTBRIDGE_TEST_ONLY = "TESTBRIDGE_TEST_ONLY"; /** - * This is the name of the system property used to pass Bazel's workspace name + * This is the name of the system property used to pass the main workspace name */ - private static final String RULES_SCALA_WS = "RULES_SCALA_WS"; + private static final String RULES_SCALA_MAIN_WS_NAME = "RULES_SCALA_MAIN_WS_NAME"; /** * This is the name of the system property used to pass a short path of the file, which includes @@ -31,32 +35,26 @@ public static void main(String[] args) throws IOException { } private static String[] extendArgs(String[] args, Map env) throws IOException { - args = extendFromSystemPropArgs(args); + args = extendFromFileArgs(args); args = extendFromEnvVar(args, env, TESTBRIDGE_TEST_ONLY, "-s"); return args; } - private static String[] extendFromSystemPropArgs(String[] args) throws IOException { - String rulesWorkspace = System.getProperty(RULES_SCALA_WS); - if (rulesWorkspace == null || rulesWorkspace.trim().isEmpty()) - throw new IllegalArgumentException(RULES_SCALA_WS + " is null or empty."); - - String rulesArgsKey = System.getProperty(RULES_SCALA_ARGS_FILE); - if (rulesArgsKey == null || rulesArgsKey.trim().isEmpty()) + private static String[] extendFromFileArgs(String[] args) throws IOException { + String runnerArgsFileKey = System.getProperty(RULES_SCALA_ARGS_FILE); + if (runnerArgsFileKey == null || runnerArgsFileKey.trim().isEmpty()) throw new IllegalArgumentException(RULES_SCALA_ARGS_FILE + " is null or empty."); - String rulesArgsPath = Runfiles.create().rlocation(rulesWorkspace + "/" + rulesArgsKey); - if (rulesArgsPath == null) - throw new IllegalArgumentException("rlocation value is null for key: " + rulesArgsKey); + String workspace = System.getProperty(RULES_SCALA_MAIN_WS_NAME); + if (workspace == null || workspace.trim().isEmpty()) + throw new IllegalArgumentException(RULES_SCALA_MAIN_WS_NAME + " is null or empty."); - List runnerArgs = Files.readAllLines(Paths.get(rulesArgsPath), Charset.forName("UTF-8")); + String runnerArgsFilePath = Runfiles.create().rlocation(workspace + "/" + runnerArgsFileKey); + if (runnerArgsFilePath == null) + throw new IllegalArgumentException("rlocation value is null for key: " + runnerArgsFileKey); - int runpathFlag = runnerArgs.indexOf("-R"); - if (runpathFlag >= 0) { - String runpathKey = runnerArgs.get(runpathFlag + 1); - String runpath = Runfiles.create().rlocation(rulesWorkspace + "/" + runpathKey); - runnerArgs.set(runpathFlag + 1, runpath); - } + List runnerArgs = Files.readAllLines(Paths.get(runnerArgsFilePath), Charset.forName("UTF-8")); + rlocateRunpathValue(workspace, runnerArgs); String[] runnerArgsArray = runnerArgs.toArray(new String[runnerArgs.size()]); @@ -73,7 +71,6 @@ private static String[] extendFromEnvVar( if (value == null) { return args; } - ; String[] flag = new String[] {flagName, value}; String[] result = new String[args.length + flag.length]; System.arraycopy(args, 0, result, 0, args.length); @@ -81,4 +78,21 @@ private static String[] extendFromEnvVar( return result; } + + /** + * Replaces ScalaTest Runner's runpath elements paths (see http://www.scalatest.org/user_guide/using_the_runner) + * with values from Bazel's runfiles + */ + private static void rlocateRunpathValue(String rulesWorkspace, List runnerArgs) throws IOException { + int runpathFlag = runnerArgs.indexOf("-R"); + if (runpathFlag >= 0) { + String[] runpathElements = runnerArgs.get(runpathFlag + 1).split(File.pathSeparator); + Runfiles runfiles = Runfiles.create(); + for (int i = 0; i < runpathElements.length; i++) { + runpathElements[i] = runfiles.rlocation(rulesWorkspace + "/" + runpathElements[i]); + } + String runpath = String.join(File.separator, runpathElements); + runnerArgs.set(runpathFlag + 1, runpath); + } + } } diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 0eb1e16c1..9e43ce639 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -22,6 +22,8 @@ import scala.tools.nsc.reporters.ConsoleReporter; class ScalacProcessor implements Processor { + private static boolean isWindows = System.getProperty("os.name").toLowerCase().contains("windows"); + /** This is the reporter field for scalac, which we want to access */ private static Field reporterField; @@ -251,7 +253,7 @@ private static void removeTmp(Path tmp) throws IOException { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - file.toFile().setWritable(true); + if (isWindows) file.toFile().setWritable(true); Files.delete(file); return FileVisitResult.CONTINUE; } @@ -288,11 +290,12 @@ private static void copyResources( } else { dstr = resource.destination; } - if (dstr.charAt(0) == '/' || dstr.charAt(0) == '\\') { + + if (dstr.charAt(0) == '/') { // we don't want to copy to an absolute destination dstr = dstr.substring(1); } - if (dstr.startsWith("../") || dstr.startsWith("..\\")) { + if (dstr.startsWith("../")) { // paths to external repositories, for some reason, start with a leading ../ // we don't want to copy the resource out of our temporary directory, so // instead we replace ../ with external/ diff --git a/test/BUILD b/test/BUILD index fe788dbff..319d20edc 100644 --- a/test/BUILD +++ b/test/BUILD @@ -101,6 +101,13 @@ scala_test_suite( ], ) +scala_test_suite( + name = "LongTestSuiteNamesSuite", + size = "small", + srcs = glob(["longnames/**/*.scala"]), + use_short_names = True, +) + scala_test_suite( name = "TestSuitePassesKwArgs", size = "small", # Not a macro, can pass test-specific attributes. diff --git a/test/longnames/looooooongnaaaaaaame/anooooootherlooooooooongname/anooooootherlooooooooongname2/anooooootherlooooooooongname3/LongNamesTest.scala b/test/longnames/looooooongnaaaaaaame/anooooootherlooooooooongname/anooooootherlooooooooongname2/anooooootherlooooooooongname3/LongNamesTest.scala new file mode 100644 index 000000000..caac1c037 --- /dev/null +++ b/test/longnames/looooooongnaaaaaaame/anooooootherlooooooooongname/anooooootherlooooooooongname2/anooooootherlooooooooongname3/LongNamesTest.scala @@ -0,0 +1,9 @@ +package test.longnames.looooooongnaaaaaaame.anooooootherlooooooooongname.anooooootherlooooooooongname2.anooooootherlooooooooongname3 + +import org.scalatest._ + +class SomeLoooooooongTestNaaaaaaaaaaaaaaaaaaaaaaaame extends FlatSpec with Matchers { + "This test" should "pass" in { + assert(true) + } +} diff --git a/test_rules_scala.ps1 b/test_rules_scala.ps1 index 65324062e..29bfc43bc 100644 --- a/test_rules_scala.ps1 +++ b/test_rules_scala.ps1 @@ -26,6 +26,7 @@ bazel test ` //test:HelloLibTest ` //test:HelloLibTestSuite_test_suite_HelloLibTest.scala ` //test:HelloLibTestSuite_test_suite_HelloLibTest2.scala ` + //test:LongTestSuiteNamesSuite ` //test:TestFilterTests ` //test:no_sig ` //test/aspect:aspect_test `