Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] fix windows issues #90

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ platforms:
macos:
test_targets:
- "//:all_tests"
windows:
test_targets:
- "//:all_tests"
1 change: 1 addition & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
build --strategy=KotlinCompile=worker
build --test_output=errors
build --verbose_failures
build --sandbox_debug

5 changes: 4 additions & 1 deletion kotlin/builder/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,8 @@ java_test(
size = "small",
srcs = glob(["unittests/**/*.java"]),
test_class = "io.bazel.kotlin.builder.mode.jvm.utils.JdepsParserTest",
deps = [":builder_for_tests"],
deps = [
":builder_for_tests",
"//third_party/jvm/com/google/truth"
],
)
35 changes: 20 additions & 15 deletions kotlin/builder/bootstrap.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,28 @@
# limitations under the License.
load("//kotlin:kotlin.bzl", _for_ide = "kt_jvm_library", "kt_jvm_import")

_HEADER = """
function join_by { local IFS="$$1"; shift; echo "$$*"; }

CP=$$(join_by : $(locations :%s))
"""

# We manually call the Kotlin compiler by constructing the correct Java classpath, because the usual
# "kotlinc" wrapper script fails to correctly detect KOTLIN_HOME unless we run with
# --spawn_strategy=standalone
def _gen_cmd(name, args):
return (_HEADER + """
def _gen_cmd(kt_cp_label, args):
return ("""
function join_by { local IFS="$$1"; shift; echo "$$*"; }
case "$$(uname -s)" in
CYGWIN*|MINGW32*|MSYS*)
SEP=";"
;;
*)
SEP=":"
;;
esac
CP=$$(join_by $$SEP $(locations :%s))
ARGS="%s"

KOTLIN_HOME=external/com_github_jetbrains_kotlin
java -Xmx256M -Xms32M -noverify \
-cp $${KOTLIN_HOME}/lib/kotlin-preloader.jar org.jetbrains.kotlin.preloading.Preloader \
-cp $${KOTLIN_HOME}/lib/kotlin-compiler.jar org.jetbrains.kotlin.cli.jvm.K2JVMCompiler \
-cp $${CP} -d $(OUTS) $${ARGS} $(SRCS)
""") % (name,args)
CMD="java -Xmx256M -Xms32M -noverify \
-cp $(location @com_github_jetbrains_kotlin//:preloader) org.jetbrains.kotlin.preloading.Preloader \
-cp $(location @com_github_jetbrains_kotlin//:compiler) org.jetbrains.kotlin.cli.jvm.K2JVMCompiler \
-cp $${CP} -d $(OUTS) $${ARGS} $(SRCS)"
$$CMD
""") % (kt_cp_label, args)

def kotlin_worker_lib(name, srcs, args = [], deps=[], runtime_deps=[], neverlink_deps=[]):
dep_label = name + "_deps"
Expand All @@ -50,6 +53,8 @@ def kotlin_worker_lib(name, srcs, args = [], deps=[], runtime_deps=[], neverlink
tools = [
"@com_github_jetbrains_kotlin//:home",
"@local_jdk//:jdk",
"@com_github_jetbrains_kotlin//:preloader",
"@com_github_jetbrains_kotlin//:compiler",
dep_label
],
srcs = srcs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.bazel.kotlin.builder.utils.ArgMap
import io.bazel.kotlin.builder.utils.DefaultKotlinCompilerPluginArgsEncoder
import io.bazel.kotlin.model.KotlinModel
import io.bazel.kotlin.model.KotlinModel.BuilderCommand
import java.io.File


@ImplementedBy(DefaultBuildCommandBuilder::class)
Expand Down Expand Up @@ -107,8 +108,7 @@ private class DefaultBuildCommandBuilder @Inject constructor(
addAllSourceJars(it)
}


joinedClasspath = classpathList.joinToString(":")
joinedClasspath = classpathList.joinToString(File.pathSeparator)
}

with(root.infoBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package io.bazel.kotlin.builder
import com.google.common.collect.ImmutableSet
import com.google.inject.*
import com.google.inject.util.Modules
import io.bazel.kotlin.builder.utils.BazelRunFiles
import io.bazel.kotlin.builder.utils.resolveVerified
import org.jetbrains.kotlin.preloading.ClassPreloadingUtils
import org.jetbrains.kotlin.preloading.Preloader
Expand All @@ -42,13 +43,12 @@ class KotlinToolchain private constructor(
internal val NO_ARGS = arrayOf<Any>()

private val isJdk9OrNewer = !System.getProperty("java.version").startsWith("1.")
private val javaRunfiles get() = Paths.get(System.getenv("JAVA_RUNFILES"))

private fun createClassLoader(javaHome: Path, kotlinHome: Path): ClassLoader {
val preloadJars = mutableListOf<File>().also {
it += kotlinHome.resolveVerified("lib", "kotlin-compiler.jar")
it += javaRunfiles.resolveVerified("io_bazel_rules_kotlin", "kotlin", "builder", "compiler_lib.jar")
if(!isJdk9OrNewer) {
it += BazelRunFiles.resolveVerified("io_bazel_rules_kotlin", "kotlin", "builder", "compiler_lib.jar")
if (!isJdk9OrNewer) {
it += javaHome.resolveVerified("lib", "tools.jar")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package io.bazel.kotlin.builder
import com.google.inject.AbstractModule
import com.google.inject.Provides
import io.bazel.kotlin.builder.KotlinToolchain.Companion.NO_ARGS
import io.bazel.kotlin.builder.utils.BazelRunFiles
import io.bazel.kotlin.builder.utils.executeAndAwait
import io.bazel.kotlin.builder.utils.resolveVerified
import io.bazel.kotlin.builder.utils.resolveVerifiedToAbsoluteString
Expand All @@ -30,7 +31,8 @@ internal object KotlinToolchainModule : AbstractModule() {
fun jarToolInvoker(toolchain: KotlinToolchain): KotlinToolchain.JarToolInvoker =
object : KotlinToolchain.JarToolInvoker {
override fun invoke(args: List<String>, directory: File?) {
val jarTool = toolchain.javaHome.resolveVerifiedToAbsoluteString("bin", "jar")
val jarTool = toolchain.javaHome.resolveVerifiedToAbsoluteString(
"bin", if (BazelRunFiles.isWindows) "jar.exe" else "jar")
val command = mutableListOf(jarTool).also { it.addAll(args) }
executeAndAwait(10, directory, command).takeIf { it != 0 }?.also {
throw CompilationStatusException("error running jar command ${command.joinToString(" ")}", it)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import io.bazel.kotlin.builder.CompilationStatusException
import io.bazel.kotlin.builder.KotlinToolchain
import io.bazel.kotlin.builder.utils.addAll
import io.bazel.kotlin.model.KotlinModel.BuilderCommand
import java.io.File

@ImplementedBy(DefaultJavaCompiler::class)
interface JavaCompiler {
Expand All @@ -35,7 +36,7 @@ private class DefaultJavaCompiler @Inject constructor(
val outputs = command.outputs
if (inputs.javaSourcesList.isNotEmpty() || inputs.generatedJavaSourcesList.isNotEmpty()) {
val args = mutableListOf(
"-cp", "${outputs.classDirectory}/:${outputs.tempDirectory}/:${inputs.joinedClasspath}",
"-cp", "${outputs.classDirectory}${File.separatorChar}${File.pathSeparator}${outputs.tempDirectory}${File.separatorChar}${File.pathSeparatorChar}${inputs.joinedClasspath}",
"-d", outputs.classDirectory
).let {
it.addAll(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.bazel.kotlin.builder.utils.addAll
import io.bazel.kotlin.model.KotlinModel
import java.io.ByteArrayInputStream
import java.io.ByteArrayOutputStream
import java.io.File
import java.io.PrintStream

@ImplementedBy(DefaultKotlinCompiler::class)
Expand Down Expand Up @@ -66,7 +67,7 @@ private class DefaultKotlinCompiler @Inject constructor(
"-language-version", command.info.toolchainInfo.common.languageVersion,
"-jvm-target", command.info.toolchainInfo.jvm.jvmTarget,
// https://github.com/bazelbuild/rules_kotlin/issues/69: remove once jetbrains adds a flag for it.
"--friend-paths", command.info.friendPathsList.joinToString(":")
"--friend-paths", command.info.friendPathsList.joinToString(File.pathSeparator)
)

args
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
package io.bazel.kotlin.builder.mode.jvm.utils

import com.google.devtools.build.lib.view.proto.Deps
import java.io.File
import java.nio.file.Path
import java.nio.file.Paths
import java.util.*
import java.util.function.Predicate
import java.util.stream.Stream

class JdepsParser private constructor(private val filename: String, private val isImplicit: Predicate<String>) {
class JdepsParser private constructor(
private val filename: String,
private val isImplicit: Predicate<String>
) {
private val packageSuffix: String = " ($filename)"

private val depMap = HashMap<String, Deps.Dependency.Builder>()
Expand Down Expand Up @@ -120,7 +123,7 @@ class JdepsParser private constructor(private val filename: String, private val
): Deps.Dependencies {
val filename = Paths.get(classJar).fileName.toString()
val jdepsParser = JdepsParser(filename, isImplicit)
Stream.of(*classPath.split(":".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray())
classPath.split(File.pathSeparator)
.forEach { x -> jdepsParser.consumeJarLine(x, Deps.Dependency.Kind.UNUSED) }
jdepLines.forEach { jdepsParser.processLine(it) }

Expand Down
74 changes: 74 additions & 0 deletions kotlin/builder/src/io/bazel/kotlin/builder/utils/BazelUtils.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright 2018 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.kotlin.builder.utils

import com.google.common.collect.ImmutableMap
import java.io.File
import java.io.FileInputStream
import java.nio.charset.Charset
import java.nio.file.Paths


/** Utility class for getting runfiles in tests on Windows. */
object BazelRunFiles {
val isWindows = System.getProperty("os.name").toLowerCase().indexOf("win") >= 0
/**
* Populated on windows. The RUNFILES_MANIFEST_FILE is set on platforms other then windows but it can be empty,]
*/
private val manifestFile: String? =
if (isWindows) {
checkNotNull(System.getenv("RUNFILES_MANIFEST_FILE"))
} else null

private val javaRunFiles = Paths.get(System.getenv("JAVA_RUNFILES"))

private val runfiles by lazy {
with(ImmutableMap.builder<String, String>()) {
FileInputStream(manifestFile)
.bufferedReader(Charset.forName("UTF-8"))
.lines()
.forEach { it ->
val line = it.trim { it <= ' ' }
if (!line.isEmpty()) {
// TODO(bazel-team): This is buggy when the path contains spaces, we should fix the manifest format.
line.split(" ").also {
check(it.size == 2) { "RunFiles manifest entry contains more than one space" }
put(it[0], it[1])
}
}
}
build()
}
}

/**
* Resolve a run file on windows or *nix.
*/
fun resolveVerified(vararg path: String): File {
return manifestFile?.let { mf ->
path.joinToString("/").let { rfPath ->
File(
checkNotNull(runfiles[rfPath]) {
"windows runfile manifest ${mf} did not contain path $rfPath"
}
)
}.also {
check(it.exists()) { "runfile file $it did not exist" }
}
} ?: javaRunFiles.resolveVerified(*path)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.jetbrains.kotlin.cli.common.messages.MessageRenderer
import org.jetbrains.kotlin.cli.common.messages.PrintingMessageCollector
import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler
import org.jetbrains.kotlin.config.Services
import java.io.File

@Suppress("unused")
class BazelK2JVMCompiler(private val delegate: K2JVMCompiler = K2JVMCompiler()) {
Expand All @@ -34,7 +35,7 @@ class BazelK2JVMCompiler(private val delegate: K2JVMCompiler = K2JVMCompiler())
// https://github.com/bazelbuild/rules_kotlin/issues/69: remove once jetbrains adds a flag for it.
args[i].startsWith("--friend-paths") -> {
i++
friendsPaths = args[i].split(":").toTypedArray()
friendsPaths = args[i].split(File.pathSeparator).toTypedArray()
}
else -> tally += args[i]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
*/
package io.bazel.kotlin.builder.mode.jvm.utils;

import com.google.common.collect.ImmutableSet;
import com.google.common.truth.Truth;
import com.google.devtools.build.lib.view.proto.Deps;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

import java.io.File;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.Arrays;
Expand Down Expand Up @@ -73,8 +76,8 @@ public class JdepsParserTest {
+ " com.axsy.testing.alt.sub -> java.lang java.base\n"
+ " com.axsy.testing.alt.sub -> kotlin kotlin-stdlib.jar\n";

private static final List<String> CLASSPATH =
Arrays.asList(
private static final ImmutableSet<String> CLASSPATH =
ImmutableSet.of(
"bazel-bin/cloud/qa/integrationtests/pkg/extensions/postgres/unused.jar",
"bazel-server-cloud/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar",
"bazel-server-cloud/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar",
Expand Down Expand Up @@ -108,12 +111,19 @@ private void testWithFixture(String fixture) throws IOException {
JdepsParser.Companion.parse(
LABEL,
CLASS_JAR,
CLASSPATH.stream().collect(Collectors.joining(":")),
CLASSPATH.stream().collect(Collectors.joining(File.pathSeparator)),
Arrays.asList(fixture.split("\n")),
IS_KOTLIN_IMPLICIT);
Assert.assertEquals(LABEL, result.getRuleLabel());

Assert.assertEquals(7, result.getDependencyCount());
Truth.assertThat(
result
.getDependencyList()
.stream()
.map(Deps.Dependency::getPath)
.collect(Collectors.toSet()))
.containsExactlyElementsIn(CLASSPATH);
Assert.assertEquals(1, depKinds(result, Deps.Dependency.Kind.UNUSED).size());
Assert.assertEquals(3, depKinds(result, Deps.Dependency.Kind.IMPLICIT).size());
Assert.assertEquals(3, depKinds(result, Deps.Dependency.Kind.EXPLICIT).size());
Expand Down
2 changes: 1 addition & 1 deletion kotlin/internal/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def _write_launcher_action(ctx, rjars, main_class, jvm_flags, args="", wrapper_p
jvm_flags: The flags that should be passed to the jvm.
args: Args that should be passed to the Binary.
"""
classpath = ":".join(["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list()])
classpath = ctx.configuration.host_path_separator.join(["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list()])
jvm_flags = " ".join([ctx.expand_location(f, ctx.attr.data) for f in jvm_flags])
template = ctx.attr._java_stub_template.files.to_list()[0]

Expand Down