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

Support defining extra compiler arguments in the tool chain. #381

Conversation

jongerrish
Copy link
Contributor

Supports the following attributes in the toolchain:-

extra_copts - supports passing extra arguments to kotlinc
jvm_opts - supports passing extra arguments to the Java compiler + KAPT

@restingbull
Copy link
Collaborator

I'm really not in favor of arbitrary opts. I realize this is a carryover from java, but I've seen how much of a mess those can be. From a maintainer standpoint, it's a fair bit of increased overhead around triage (issues associated with combination of flags and internal setup) and as well as keeping a parsing structure in one piece. The current implementation would allow all sorts of difficult to diagnose issues including adding arbitrary compiler plugins without any kind of ordering (just add a jar and a library), random "friend" associations, and so forth.

Considering the -X options are subject to change, I'd prefer to see a more structured and validated approach. Additionally, a lot of the performance gains that Gradle realizes come from running the compiler via the embeddable api. Arbitrary fields increase the maintenance burden associated with changing the underlying implementation.

I'd much prefer to see a structured options around the actual for compiler options.

@jongerrish
Copy link
Contributor Author

I'm really not in favor of arbitrary opts. I realize this is a carryover from java, but I've seen how much of a mess those can be. From a maintainer standpoint, it's a fair bit of increased overhead around triage (issues associated with combination of flags and internal setup) and as well as keeping a parsing structure in one piece. The current implementation would allow all sorts of difficult to diagnose issues including adding arbitrary compiler plugins without any kind of ordering (just add a jar and a library), random "friend" associations, and so forth.

Considering the -X options are subject to change, I'd prefer to see a more structured and validated approach. Additionally, a lot of the performance gains that Gradle realizes come from running the compiler via the embeddable api. Arbitrary fields increase the maintenance burden associated with changing the underlying implementation.

I'd much prefer to see a structured options around the actual for compiler options.

Thats a fair point.

We're currently passing the following values:-

extra_copts = [
    "-nowarn"
],
jvm_opts = [
    "-Xlint:none",
    "-XDsuppressNotes",
],

How would you feel about adding a boolean warnings flag to control all of these?

@restingbull
Copy link
Collaborator

#383

@@ -264,10 +264,14 @@ def kt_jvm_compile_action(ctx, rule_kind, output_jar, compile_jar):
compile_jar = kt_compile_jar
else:
java_compile_jar = ctx.actions.declare_file(ctx.label.name + "-java.abi.jar")
toolchain = ctx.toolchains[_TOOLCHAIN_TYPE]
javac_opts = toolchain.jvm_opts + ["-nowarn", "-XepDisableAllChecks"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, um, what is the -XepDisableAllChecks? flag for the error prone processor?

@jongerrish
Copy link
Contributor Author

@restingbull has implemented a better solution, closing this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants