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

[RFC] Add kotlinc option support in toolchain #303

Closed
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
9 changes: 8 additions & 1 deletion kotlin/internal/toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def _kotlin_toolchain_impl(ctx):
toolchain = dict(
language_version = ctx.attr.language_version,
api_version = ctx.attr.api_version,
extra_copts = ctx.attr.extra_copts,
debug = ctx.attr.debug,
jvm_target = ctx.attr.jvm_target,
kotlinbuilder = ctx.attr.kotlinbuilder,
Expand Down Expand Up @@ -107,6 +108,10 @@ _kt_toolchain = rule(
"1.3",
],
),
"extra_copts": attr.string_list(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably follow naming from java_toolchain and be kotlincopts https://docs.bazel.build/versions/master/be/java.html#java_toolchain.javacopts

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context I plan to add javacopts to kt_jvm_library later, having kotlinc and javac prefixes is prob best to avoid confusion while also being on same page with rules_java

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna lean towards a hard no on adding javacopts to the kt_jvm_lib. When IR becomes available, it would get dicey.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just quick question: what is IR?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna lean towards a hard no on adding javacopts to the kt_jvm_lib. When IR becomes available, it would get dicey.

Hold on, javacoptions are for javac call for .java sources in mixed Kotlin/Java kt_jvm_library, they have nothing to do with kotlinc options :)

just quick question: what is IR?

Intermediate Representation, form of abstract (byte)code produced by Kotlin toolchain before it is transformed to actual instructions of the target platform

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally, I'd agree, but the kotlin java compilation is currently interleaved. I'm taking a wait and see approach.

doc = "The extra options for kotlinc compiler.",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
doc = "The extra options for kotlinc compiler.",
doc = "The list of extra arguments for the Kotlin compiler. Please refer to the Kotlin compiler documentation for the extensive list of possible Kotlin compiler flags.",

allow_empty = True
),
"debug": attr.string_list(
doc = """Debugging tags passed to the builder. Two tags are supported. `timings` will cause the builder to
print timing information. `trace` will cause the builder to print tracing messages. These tags can be
Expand Down Expand Up @@ -170,14 +175,16 @@ def define_kt_toolchain(
name,
language_version = None,
api_version = None,
jvm_target = None):
jvm_target = None,
extra_copts = None):
"""Define the Kotlin toolchain."""
impl_name = name + "_impl"
_kt_toolchain(
name = impl_name,
language_version = language_version,
api_version = api_version,
jvm_target = jvm_target,
extra_copts = extra_copts,
debug =
select({
"@io_bazel_rules_kotlin//kotlin/internal:builder_debug_trace": ["trace"],
Expand Down
2 changes: 2 additions & 0 deletions kotlin/internal/utils/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ def _init_builder_args(ctx, rule_kind, module_name):
args.add("--kotlin_language_version", toolchain.language_version)
args.add("--kotlin_passthrough_flags", "-Xuse-experimental=kotlin.Experimental")

args.add_all("--extra_copts", toolchain.extra_copts)

debug = depset(toolchain.debug)
for tag in ctx.attr.tags:
if tag == "trace":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ class KotlinBuilder @Inject internal constructor(
JS_PASSTHROUGH_FLAGS("--kotlin_js_passthrough_flags"),
JS_LIBRARIES("--kotlin_js_libraries"),
DEBUG("--kotlin_debug_tags"),
EXTRA_COPTS("--extra_copts"),
TASK_ID("--kotlin_task_id");
}

Expand All @@ -165,6 +166,9 @@ class KotlinBuilder @Inject internal constructor(
argMap.mandatorySingle(KotlinBuilderFlags.API_VERSION)
toolchainInfoBuilder.commonBuilder.languageVersion =
argMap.mandatorySingle(KotlinBuilderFlags.LANGUAGE_VERSION)
toolchainInfoBuilder.commonBuilder.addAllExtraCopts(
argMap.optional(KotlinBuilderFlags.EXTRA_COPTS) ?: emptyList<String>()
)
this
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ internal fun JvmCompilationTask.commonArgs(): CompilationArgs = baseArgs()
"-Xfriend-paths=${it.joinToString(X_FRIENDS_PATH_SEPARATOR)}"
}
.flag("-d", directories.classes)
.values(info.toolchainInfo.common.extraCoptsList)
.givenNotEmpty(info.passthroughFlags) { it.split(" ") }

internal fun JvmCompilationTask.baseArgs(): CompilationArgs = CompilationArgs()
Expand Down
Binary file modified src/main/protobuf/jars/libkotlin_model_proto-speed.jar
Binary file not shown.
3 changes: 3 additions & 0 deletions src/main/protobuf/kotlin_model.proto
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ message KotlinToolchainInfo {
// mandatory
// oneof "enable", "warn" or "error"
string coroutines = 3;
// optional
// extra kotlinc options to be passed into compiler
repeated string extra_copts = 4;
}

// Properties specific to the JVM.
Expand Down