rules_kotlin icon indicating copy to clipboard operation
rules_kotlin copied to clipboard

Add support for javac KAPT arguments

Open benjaminRomano opened this issue 3 years ago • 3 comments

Background Internally, we needed to support javac arguments in kotlin rules to pass through dagger options to KAPT such as fastInit and disable format generated sources.

This is our own implementation to satisfy our requirements, but I suspect this is not the desirable approach for this based on this issue: https://github.com/bazelbuild/rules_kotlin/issues/403. However, it may be helpful for getting us closer to the right APIs here.

Change

  • Add a kapt_javac_arguments attribute to the kotlin toolchain rule
  • Pass through kapt_javac_arguments to the KotlinWorker through a javac_arguments field in the kotlin model proto.

To encode the key/value pairs through to the kotlin worker we use a : delimiter. This can cause issues if the key used for an annotation processor's option includes a : which I'm not sure is possible or not.

benjaminRomano avatar Jan 12 '21 22:01 benjaminRomano

Apologies for the delay.

Quick question: how often is the toolchain args used vs. rule args?

restingbull avatar Jan 26 '21 23:01 restingbull

In this PR, I only added support to toolchain, we didn't have a use case where we'd want this to be applied to a specific target.

I believe its quite rare for us to use rule args to configure kt targets.

benjaminRomano avatar Jan 26 '21 23:01 benjaminRomano

I think we might need to consider new kt_plugin or kt_annotation_processor rule to allow us to pass-through args to KAPT.

Currently with Ben's change we apply Dagger flags to the entire KT toolchain which causes warnings like this:

warning: warning: The following options were not recognized by any processor: '[dagger.fastInit, dagger.nullableValidation, dagger.experimentalDaggerErrorMessages, dagger.formatGeneratedSource]'

because we end up applying Dagger flags to all actions even if Dagger AP is not used.

@jongerrish

nkoroste avatar Mar 19 '21 21:03 nkoroste