bazelbuild / rules_kotlin

Bazel rules for Kotlin
Apache License 2.0
330 stars 207 forks source link

Android extensions support #145

Closed psa-anddev closed 2 years ago

psa-anddev commented 5 years ago

I have a Gradle project that I want to move to Bazel. I've managed to make everything works but these rules don't seem to provide a way to integrate Android extensions, which the project I mentioned makes heavy use of. I was wondering whether the support already exists or whether it is in the roadmap.

I was also wondering if this is not in the roadmap, would a PR implementing this be welcomed?

Thanks in advance for your response.

hsyed commented 5 years ago

Yes a PR would be very welcome.

Could you outline what the extensions do -- I am not an android developer (yet). I assume we need to enable Kotlin compiler plugins for this.

We need a working kitchen sink style Android example in this repository to work on this. @jin is looking into an idiomatic example.

We'd need a plugin abstraction in the builder. I have a huge pr with a custom compiler plugin so I will add a extension point something along the lines of:

abstract class PluginAdapter(private val jar: String, private val id: String) {
      // return null if not enabled.
      open fun contributeArguments(context: CompilationTaskContext, task: JvmCompilationTask): List<String>? = null

      open fun postProcess(context: CompilationTaskContext, task: JvmCompilationTask, t: Throwable?){}
}

Then we need a way of enabling and disabling plugins from the rules. Hopefully these plugins don't require any configuration in which case we could just pass a label to indicate the plugin should be active.

psa-anddev commented 5 years ago

The Android extensions add syntactic sugar to access views. In Android, the UI is defined in a layout file which is written in XML. When you want to use a view from the code, you assign it an id. Then, you would use findViewById with your id to access your view from the code. This adds a lot of boilerplate. The Android extensions add a series of properties that allows you to access them without having to write all the findViewById calls.

The way you use them in Gradle is adding an additional plugin to the build.gradle file. I've taken a quick look at the code of the extensions and it seems there's a bit of code generation however, it doesn't seem to be using kapt so I would assume that it is, in fact, a compiler plug in but I'm not really certain about it.

Regarding your abstraction layer, I like it, I would refine it a little bit. Something like this

abstract class PluginAdapter(private val jar: String, private val id: String) {
           abstract fun isEnabled(context: CompilationTaskContext, task: JvmCompilationTask): Boolean
           abstract fun contributeArguments(context: CompilationTaskContext, task: JvmCompilationTask): List<String>
          abstract fun postProcess(context: CompilationTaskContext, task: JvmCompilationTask, throwable: Throwable?)
}

Adding the isEnabled function, you can remove the nullability on the list in contributeArguments which, I think, makes the code nicer.

Since you say your PR is huge, perhaps, I can quickly write the abstract class and make a PR for it that you can merge independently. That way, the size of your PR would get reduced.

Kadanza commented 5 years ago

In addition, android extension is gradle plugin. This plugin auto generate classes. Can we use gradle plugin in bazel? How to resolve this problem?

apply plugin: 'kotlin-android-extensions' apply plugin: 'realm-android'

Realm is gradle plugin, such as android-extension. I need to use this or i can use google's room. This library use annotation process.

Gradle have annotation process, that generate code.

Such as annotationProcessor "android.arch.persistence.room:compiler:1.0.0"

I use annotationProcessor for my gradle project, but how use annotation process in bazel?

psa-anddev commented 5 years ago

@Kadanza, you shouldn't have any problems using room since these rules, as far as I know, already allow the use of kapt. In the case of Realm, I'm not even sure if it should be tackle in this project or in the general Android rules. However, if it is ok, I'd like to keep this issue focused on getting support for Android extensions.

JakeWharton commented 5 years ago

I would not recommend working on getting synthetic extensions working since they are experimental unless it comes for free with compiler plugin support. A better endevour would be Parcelize which is built on the same mechanisms but will be made available as a stable feature in the 1.3.x timeframe.

hsyed commented 5 years ago

Yes it is a compiler plugin. I have found it in the compiler distribution. The plugin comes with a runtime component. The runtime component would need to be dexed, I don't think it needs to go into the _toolchain attribute it could just be added to deps by the kt_android_library macro.

The plugin will probably need to be given the paths of the android resources to process I think you will need to add a new kotlin builder flag for it. Along with a builder flag the rule needs to get the resource path attribute, I don’t think this should be added to the attributes of kt_jvm_library. Here are the options:

  1. New internal kt_android_library rule (see js rules impl for how a macro sits in front of a rule). We should probably avoid this.
  2. Gather the resource files from the sandwich. This is probably the simplest. I'll put the heuristic below.
  3. Alternatively we could create a kt_compiler_plugin rule and provider and gather these in the aspect processing for ‘java_plugin’. This is also a viable approach -- there are other compiler plugins For example the plugins for spring -- i.e., all-open, no-args constructor. These plugins don't necessarily need to be configured though.

The heuristic for 2:

# this is psuedocode-ish.

gathered_android_resource_files = depset()

for dep in ctx.attr.deps:
  # Start by only gathering the resources from the base rule. Might need to gather from other deps and exported deps. 
  if _get_dep_name(dep.name) == _name_rule_name_ctx(ctx.label) + "_base":
    # add provider to the dep attribute
    if not <name of the android provider> in dep:
      fail("precondition failed")
   gathered_android_resource_files += dep[<name of the android provider>].android_resource_files

gathered_android_resource_files = gathered_android_resource_files.to_list()

@jin what do you think ?

Since the plugin generates classes (or source files and then classes) it should probably be run in the runAnnotationProcessors method.

Adding the isEnabled function, you can remove the nullability on the list in contributeArguments which, I think, makes the code nicer.

Returning null is quite idiomatic in jetbrains code. It allows for elegant chaining patterns. Alternatively you could just return an empty list as well which might be most appropriate here.

hsyed commented 5 years ago

@JakeWharton This is the plugin we are talking about. In the tutorial I see a import kotlinx.android.synthetic.main.activity_main.* could you elaborate a bit more.

JakeWharton commented 5 years ago

Elaborate on which part? In order to have those imports generated you must enable their experimental nature. If you scroll down you'll see the Parcelable section: https://kotlinlang.org/docs/tutorials/android-plugin.html#parcelable. This is also currently experimental but they are going to become stable in Kotlin 1.3.x.

psa-anddev commented 5 years ago

I would not recommend working on getting synthetic extensions working since they are experimental unless it comes for free with compiler plugin support. A better endevour would be Parcelize which is built on the same mechanisms but will be made available as a stable feature in the 1.3.x timeframe.

@JakeWharton, I have just looked at the official Kotlin documentation and it seems that the Android extensions that allows the synthetic imports for the views are production ready. If you look at the documentation page on Android extensions (linked here), you'll see clearly stated that there are some experimental features in the plug in such as the parcellable and the use of views present in flavours. However, the fact that certain features are told to be experimental whereas others are not seems to indicate that these other features are considered to be ready. In addition, these Android extensions have been around since before Google started to support Kotlin which is more than a year ago. If I am missing something, please, let me know.

However, even if these features were experimental, why should that prevent Bazel support for them? I don't find any compelling argument to state that because a certain feature is experimental, it shouldn't be widely supported by build systems other than Gradle. Anyway, if I misunderstood your point, please, let me know.

JakeWharton commented 5 years ago

The entire feature is experimental, not just parts of it.

However, even if these features were experimental, why should that prevent Bazel support for them?

Did you read what you quoted? I said if it comes for free then it's fine.

psa-anddev commented 5 years ago

The entire feature is experimental, not just parts of it.

However, even if these features were experimental, why should that prevent Bazel support for them?

Did you read what you quoted? I said if it comes for free then it's fine.

Don't take me wrong. I just want to know where you are coming from. I think the documentation should say explicitly that the whole feature is experimental. The fact that there is an experimental mode within the feature itself is a bit weird, don't you think? Perhaps, I don't understand the reasoning behind it.

But, what I really want to understand is why only experimental features that are easy to implement should be ported to Bazel. It would be great if you could clarify this for me.

JakeWharton commented 5 years ago

Both of the features of the Gradle plugin are entirely experimental which you must opt in to. We are in the process of making Parcelize enabled by default.

I said nothing about "easy". I said focus on the feature that is becoming stable and rather than the one that isn't.

Kadanza commented 5 years ago

It's too long wait then Kotlin android extension become stable. We can use only findViewById.

jin commented 5 years ago

I'm not familiar with Android Extensions at all, but after reading through the conversation so far, I've gathered the following:

1) It's a Kotlin compiler plugin. 1) It's an annotation processor. 1) It's not clear whether this should be scoped within rules_kotlin or rules_android. 1) It's highly experimental according to @JakeWharton, so APIs may still change, resulting in rippling downstream maintenance burden. 1) It should probably reuse the java_plugin architecture. @hsyed the 3rd plan for a kt_compiler_plugin sounds like the best solution to me.

JakeWharton commented 5 years ago

It is a compiler plugin, a build plugin, and part of the IDE plugin. It is not an annotation processor.

On Mon, Sep 17, 2018, 4:59 AM Jingwen notifications@github.com wrote:

I'm not familiar with Android Extensions at all, but after reading through the conversation so far, I've gathered the following:

  1. It's a Kotlin compiler plugin.
  2. It's an annotation processor.
  3. It's not clear whether this should be scoped within rules_kotlin or rules_android.
  4. It should probably reuse the java_plugin https://docs.bazel.build/versions/master/be/java.html#java_plugin architecture. @hsyed https://github.com/hsyed the 3rd plan for a kt_compiler_plugin sounds like the best solution to me.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_kotlin/issues/145#issuecomment-421934488, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEc1whJ24aHRxf08xnZiG1wpPsq4Gks5ub2RfgaJpZM4Wp9bw .

hsyed commented 5 years ago

@jin I am in nyc for an interview. I notice you are based in NY. I am staying in the meatpackers district. I’m free all day thursday if you would like to meet up and discuss the rules.

jin commented 5 years ago

@hsyed Unfortunately I'm not in NYC this week.

alexrs commented 5 years ago

Hey, I've seen that when a new Android project using Kotlin is created with Android Studio, these extensions are used by default. Is it still an experimental feature, or are there plans to support it in Bazel?

cgruber commented 4 years ago

We're actively working on this, at Square, just as an update. We may have a poor implementation initially, with it hard-coded (with some detection or signaling). In the long-run, I'd like to see a kt_plugin() but we may have a variant of the android extensions supported before we get so far as that.

dimsuz commented 4 years ago

I'm curious how hard will it be to also integrate newish ViewBinding (details here) which will be shipped with Android Gradle Plugin 3.6.x and is a better version (imo) of a view finding part of kotlin android extensions.

I haven't checked how tightly this feature is tied to AGP, I guess if it relies on some command-line tool, it can be reused.

cgruber commented 4 years ago

Don't know, it would require some investigation. It's really frustrating how much the kotlin features end up leaning on gradle, but mostly they are extricable.

inez commented 4 years ago

Early and poor implementation is now available here: https://github.com/inez/rules_kotlin/commit/8309825270d1083c76364420ba27247f7571e007

cgruber commented 4 years ago

We'll re-structure @inez' PR against the cgruber repo and replay it here.

jigt-gft commented 4 years ago

hi @inez ,

I have seen that you have been able to solve the error in the rules_kotlin of the Kotlin Android support.

We are working on a project in which we have that problem when making the compilation.

Please, could you tell me how you have solved it?

Have you made the changes you made locally?

If so, could you help me solve my problem.

Best regards

sumit-tokopedia commented 4 years ago

have this issue resolved because its still comming in legacy_1.4?

roscrazy commented 3 years ago

Hi guys, would we have any update here? would this blocker be fixed with the kt_compiler_plugin support? Or do we have any other blocker

fisheye-123 commented 3 years ago

I'm curious how hard will it be to also integrate newish ViewBinding (details here) which will be shipped with Android Gradle Plugin 3.6.x and is a better version (imo) of a view finding part of kotlin android extensions.

I haven't checked how tightly this feature is tied to AGP, I guess if it relies on some command-line tool, it can be reused.

hey, did you happen to get viewbinding work in bazel?

arunkumar9t2 commented 3 years ago

Related #522

cgruber commented 2 years ago

Duplicate of #231 and the conversation has evolved somewhat off-line. Closing this, but the issue will continue in another bug, as we figure out what's working and isn't.