search

hsyed / rules_kotlin_old

This repo is retired and will be recycled soon. Official rules found at ->
http://github.com/bazelbuild/rules_kotlin
Apache License 2.0
28 stars 3 forks source link

prefer kt_* rules over kotlin_* rules, as that is consistent with the general pattern in bazel #6

Open cgruber opened 6 years ago

cgruber commented 6 years ago

While java_library/java_binary happen to use the full language name, many others use some abbreviation or suffix, such as cc_library, or py_library. While it's not a strict style requirement for bazel rules, it might be a bit more in the spirit of bazel to prefer kt_library over kotlin_library.

Not all do, but the language rules ones with native support from google typically do.

Suffix in the rule name

java_library (language name is the suffix) groovy_library (language name is the suffix) d_library (language name is the suffix) go_library (language name is the suffix) py_library proto_library

Suffix not in the rule name:

objc_library (not a suffix, but still an abbreviation) csharp_library (instead of cs_library) cc_library (technically handles .c, .cpp, and .cc files) rust_library (not rs_library) closure_js_library (kind of a different example, since it's a js_library with added jazz)

Note: Internally to Google, we're trying to evaluate a bunch of naming questions, and this came up, so this issue is partly to generate some conversation out here. Let's not finalize any decisions about this in this bug thread, but use it to evaluate pros/cons.

hsyed commented 6 years ago

I'm for shortening the language name. I initially thought truly unified rules kinds might be possible -- at least the library kinds. kt_common_library and kt_jvm_library will always have to be distinct. Unless we put constraints in the library rule but the dependency propagation will be very confusing to reason about. If we can't have truly unified rules than a shorter variant makes more sense.

The rules are currently populating a provider attribute named kt. This is already being picked up by the intelij plugin -- eventually the intellij plugin will just use the provider.

Buck when with went with kotlin_. facebook/buck#810. Docs here. But this was in 2016, I don't know if multiplatform or native had been released then.

If we do shorten does it make sense to give the JVM rules the default language variant -- i.e., kt_library vs kt_jvm_library.

Shortening sooner rather than later makes sense otherwise we have to go through a deprecation phase -- the intellij plugin classifies the rules on the rule label kinds and aliasing is not an option. The plugin pr is currently being reviewed: bazelbuild/intellij#217.

JakeWharton commented 6 years ago

Is kt_library able to consume a common module for the JVM as well as be used for a pure JVM module? The Gradle plugin differentiates these cases as org.jetbrains.kotlin.jvm and org.jetbrains.kotlin.platform.jvm, for example, so I wouldn't think it unreasonable to differentiate here as kt_library and kt_platform_jvm_library.

On Thu, Jan 25, 2018, 8:37 PM Hassan Syed notifications@github.com wrote:

I'm for shortening the language name. I initially thought truly unified rules kinds might be possible -- at least the library kinds. kt_common_library and kt_jvm_library will always have to be distinct. Unless we put constraints in the library rule but the dependency propagation will be very confusing to reason about. If we can't have truly unified rules than a shorter variant makes more sense.

The rules are currently populating a provider attribute https://github.com/hsyed/rules_kotlin/blob/f0aeb20a8c7b291984657a620c8865cad5ff4879/kotlin/rules/compile.bzl#L153 named kt. This is already being picked up by the intelij plugin -- eventually the intellij plugin will just use the provider.

Buck when with went with kotlin_. facebook/buck#810 https://github.com/facebook/buck/pull/810. Docs here https://buckbuild.com/rule/kotlin_library.html. But this was in 2016, I don't know if multiplatform or native had been released then.

If we do shorten does it make sense to give the JVM rules the default language variant -- i.e., kt_library vs kt_jvm_library.

Shortening sooner rather than later makes sense otherwise we have to go through a deprecation phase -- the intellij plugin classifies the rules on the rule label kinds and aliasing is not an option. The plugin pr is currently being reviewed: bazelbuild/intellij#217 https://github.com/bazelbuild/intellij/pull/217.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hsyed/rules_kotlin/issues/6#issuecomment-360658284, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEXP28Q3R1NxE30aRHlaQb5UVv_l7ks5tOSxqgaJpZM4RtmWv .

hsyed commented 6 years ago

@JakeWharton No work has yet been started on this, but that is what I am thinking as well. Without looking closer at the details, the following rules might cover the semantics for marking a language library as a platform library.

  1. A common library serves as an API artifact so it should be possible to list it in a dep in a appropriate rules.
  2. A kt_common_library should not be allowed as runtime_dep.
  3. A kt_*_library should have a attribute type actual_for (or implements or ....) which sets it up as a platform implementation of the declared jar.
  4. An export attr should not allow a kt_common_library or if it does it would be an alternative to 3 --i.e., any library declaring a common library as exported is implementing it.
JakeWharton commented 6 years ago

Kotlin 1.2 changed the verbage from 'actual' to 'impl'. Gradle uses the configuration name "expectedBy" to denote a dependency on a common library that expects the definitions in the current module (example: https://github.com/JakeWharton/SdkSearch/blob/3ee9590e4250214c89cecf135d7bc76033a12622/api/dac/jdk/build.gradle#L10). I'm not a huge fan of its perspective and so I think I prefer your 'implements' suggestion.

On Thu, Jan 25, 2018 at 9:14 PM Hassan Syed notifications@github.com wrote:

@JakeWharton https://github.com/jakewharton No work has yet been started on this, but that is what I am thinking as well. Without looking closer at the details, the following rules might cover the semantics for marking a language library as a platform library.

  1. A common library serves as an API artifact so it should be possible to list it in a dep in a appropriate rules.
  2. A kt_common_library should not be allowed as runtime_dep.
  3. A kt_*_library should have a attribute type actual_for (or implements or ....) which sets it up as a platform implementation of the declared jar.
  4. An export attr should not allow a kt_common_library or if it does it would be an alternative to 3 --i.e., any library declaring a common library as exported is implementing it.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/hsyed/rules_kotlin/issues/6#issuecomment-360664129, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEf5wqtVYi5mK8wlSBRLD1LJrf1NBks5tOTUXgaJpZM4RtmWv .

cgruber commented 6 years ago

We're hashing out a few of these details ourselves, and I'll be inviting Hassan to our working document (I'll ping you also, Jake). But we have a few options on the table, certainly one being kt__library, and then using kt_library for pure-kotlin (xplat-friendly). The conversation is a bit spread out, among hangout chats, this thread, a google doc, but we're thinking some similar thoughts to this thread, so that's hopeful.

On Thu, 25 Jan 2018 at 19:16 Jake Wharton notifications@github.com wrote:

Kotlin 1.2 changed the verbage from 'actual' to 'impl'. Gradle uses the configuration name "expectedBy" to denote a dependency on a common library that expects the definitions in the current module (example:

https://github.com/JakeWharton/SdkSearch/blob/3ee9590e4250214c89cecf135d7bc76033a12622/api/dac/jdk/build.gradle#L10 ). I'm not a huge fan of its perspective and so I think I prefer your 'implements' suggestion.

On Thu, Jan 25, 2018 at 9:14 PM Hassan Syed notifications@github.com wrote:

@JakeWharton https://github.com/jakewharton No work has yet been started on this, but that is what I am thinking as well. Without looking closer at the details, the following rules might cover the semantics for marking a language library as a platform library.

  1. A common library serves as an API artifact so it should be possible to list it in a dep in a appropriate rules.
  2. A kt_common_library should not be allowed as runtime_dep.
  3. A kt_*_library should have a attribute type actual_for (or implements or ....) which sets it up as a platform implementation of the declared jar.
  4. An export attr should not allow a kt_common_library or if it does it would be an alternative to 3 --i.e., any library declaring a common library as exported is implementing it.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/hsyed/rules_kotlin/issues/6#issuecomment-360664129, or mute the thread < https://github.com/notifications/unsubscribe-auth/AAEEEf5wqtVYi5mK8wlSBRLD1LJrf1NBks5tOTUXgaJpZM4RtmWv

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hsyed/rules_kotlin/issues/6#issuecomment-360673120, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUN4gzl0I-Ya3lKeBQAy9esGepaQ4vbks5tOUOIgaJpZM4RtmWv .