Skip to content

Commit

Permalink
Expose kotlinc's -Xfriend-paths to all jvm/android kt rules under the…
Browse files Browse the repository at this point in the history
… attribute 'assocates=' (#465)

Associates lets a library associate it self to other libraries, making them part of the same module. This is constrained such that while multiple libraries may be associated, they must all shard the same module, and so cannot associate to anything that is part of a different module. These module relationships are in the bazel build graph, not the contents of the jars as such.

This module membership is transitive (within the above-mentioned constraint), though strict-deps would stop that. Also, only kotlin targets can be associated.

Per discussions across several media, the name "associates" was chosen over "friends" (despite the kotlinc flag being -Xfriend-paths) as that is the terminology used in the gradle kotlin plugin, which is kotlin's primary delivery vehicle, and to avoid confusion with the C++ friends concepts. The pre-existing "friends" attribute is preserved for backward compatibility with a warning. Future PR will add a flag to turn off that support, and then we'll delete it.

kt_jvm_import does not include this facility, but these can just set their module_name in common to participate.

Android should work, but because kt_android_* is a macro not a rule, the implicit target //my/android/library:mytarget_kt should be friended, since it has a KtJvmInfo. The //my/android/library will macro-resolve into an android_library. Until the android rules get the right kind of love, such that we can make a rule that has KtJvmInfo AND android-whatever providers, this simply is a known limitation we'll have to live with.

Also, the prior implementation shoved the full transitive closure (all jars, kotlin or no) of the friend= into the -Xfriends-paths flag, which is awful. This PR does break that, in case people were relying on that by some oddity. The fix is to just add the targets directly.

Fixes #211
  • Loading branch information
cgruber authored Jan 29, 2021
1 parent 71246c9 commit 15dd1de
Show file tree
Hide file tree
Showing 23 changed files with 406 additions and 58 deletions.
12 changes: 11 additions & 1 deletion .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ tasks:
- "//src/test/kotlin/io/bazel/kotlin/builder:builder_tests"
# KotlinJvmDaggerExampleTest and KotlinJvmKaptAssertionTest are not remote
# execution compatible, do not run them for now.
- "//src/test/kotlin/io/bazel/kotlin:KotlinJvmFriendsVisibilityTest"
- "//src/test/kotlin/io/bazel/kotlin:KotlinJvmAssociatesBasicVisibilityTest"
- "//src/test/kotlin/io/bazel/kotlin:KotlinJvmBasicAssertionTest"
test_flags:
# Override the default worker strategy for remote builds (worker strategy
Expand All @@ -37,6 +37,16 @@ tasks:
- "--override_repository=io_bazel_rules_kotlin=/tmp/rules_kotlin_release"
test_targets:
- //...
example-associates:
name: "Example - Associates"
platform: ubuntu1804
shell_commands:
- "cd ../.. && bazel build //:rules_kotlin_release && rm -rf /tmp/rules_kotlin_release && mkdir -p /tmp/rules_kotlin_release && tar -C /tmp/rules_kotlin_release -xvf bazel-bin/rules_kotlin_release.tgz"
working_directory: examples/associates
test_flags:
- "--override_repository=io_bazel_rules_kotlin=/tmp/rules_kotlin_release"
test_targets:
- //...
example-anvil:
name: "Example - Anvil"
platform: ubuntu1804
Expand Down
1 change: 1 addition & 0 deletions .bazelignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# we don't break trying to build separate workspaces using wildcards like //...
# examples/dagger doesn't have its own workspace, so don't do all of examples.
examples/android
examples/associates
examples/jetpack_compose
examples/node
examples/trivial
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ and `kt_js`, and `kt_android` typically applied to the rules (the exception bein
`kt_android_local_test`, which doesn't exist. Use an `android_local_test` that takes a
`kt_android_library` as a dependency).

Limited "friend" support is available, in the form of tests being friends of their library for the
system under test, allowing `internal` access to types and functions.
Support for kotlin's -Xfriend-paths via the `associates=` attribute in the jvm allow access to
`internal` members.

Also, `kt_jvm_*` rules support the following standard `java_*` rules attributes:
* `data`
Expand Down
1 change: 1 addition & 0 deletions examples/associates/.bazelversion
44 changes: 44 additions & 0 deletions examples/associates/WORKSPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

local_repository(
name = "io_bazel_rules_kotlin",
path = "../..",
)

load("@io_bazel_rules_kotlin//kotlin:dependencies.bzl", "kt_download_local_dev_dependencies")

kt_download_local_dev_dependencies()

load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kotlin_repositories", "kt_register_toolchains")

kotlin_repositories()

kt_register_toolchains()

RULES_JVM_EXTERNAL_TAG = "2.7"

RULES_JVM_EXTERNAL_SHA = "f04b1466a00a2845106801e0c5cec96841f49ea4e7d1df88dc8e4bf31523df74"

http_archive(
name = "rules_jvm_external",
sha256 = RULES_JVM_EXTERNAL_SHA,
strip_prefix = "rules_jvm_external-%s" % RULES_JVM_EXTERNAL_TAG,
url = "https://github.com/bazelbuild/rules_jvm_external/archive/%s.zip" % RULES_JVM_EXTERNAL_TAG,
)

load("@rules_jvm_external//:defs.bzl", "maven_install")

maven_install(
artifacts = [
"junit:junit:4.13",
],
repositories = [
"https://repo1.maven.org/maven2",
],
)

http_archive(
name = "rules_pkg",
sha256 = "4ba8f4ab0ff85f2484287ab06c0d871dcb31cc54d439457d28fd4ae14b18450a",
url = "https://github.com/bazelbuild/rules_pkg/releases/download/0.2.4/rules_pkg-0.2.4.tar.gz",
)
9 changes: 9 additions & 0 deletions examples/associates/projects/core/api/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")

package(default_visibility = ["//visibility:public"])

kt_jvm_library(
name = "api",
srcs = glob(["src/main/kotlin/**/*.kt"]),
deps = [],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package core.api

interface SomeInterface {
val name: String
val camelName: String
}

data class MyType(
override val name: String
) : SomeInterface {
override val camelName: String = name.camelCase()
}

internal fun String.camelCase() = this.split("_").joinToString("") {
"${it[0].toUpperCase()}${it.substring(1)}"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_test")

kt_jvm_test(
name = "CoreApiTest",
srcs = ["CoreApiTest.kt"],
friends = ["//projects/core/api"], # Deprecated - here to ensure it still works.
deps = [
"@maven//:junit_junit",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package core.api

import org.junit.Assert.assertEquals
import org.junit.Test

class CoreApiTest {
@Test fun testCamelCaseVar() {
val foo = MyType("foo_bar")
assertEquals("FooBar", foo.camelName)
}

@Test fun testCamelCaseFun() {
assertEquals("FooBar", "foo_bar".camelCase())
}
}
13 changes: 13 additions & 0 deletions examples/associates/projects/core/impl/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")

kt_jvm_library(
name = "impl",
srcs = glob(["src/main/kotlin/**/*.kt"]),
associates = ["//projects/core/api"],
visibility = [
"//projects/core:__subpackages__",
],
deps = [
# All my deps are associates.
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package core.impl

import core.api.SomeInterface
import core.api.camelCase

internal data class ImplType(
override val name: String
) : SomeInterface {
override val camelName: String = name.camelCase()
val customName = name.customStuff()
}

internal fun String.customStuff() = "${hashCode()}"
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_test")

kt_jvm_test(
name = "CoreImplTest",
srcs = ["CoreImplTest.kt"],
associates = [
"//projects/core/api",
"//projects/core/impl",
],
deps = [
"@maven//:junit_junit",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package core.impl

import core.api.camelCase
import org.junit.Assert.assertEquals
import org.junit.Test

class CoreImplTest {
@Test fun testCamelCaseVar() {
val foo = ImplType("foo_bar")
assertEquals("FooBar", foo.camelName)
}

@Test fun testCamelCaseFun() {
// Testing transitivity here. TODO: Once strict deps are in place, delete this case.
assertEquals("FooBar", "foo_bar".camelCase())
}
}
1 change: 1 addition & 0 deletions kotlin/internal/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ KT_COMPILER_REPO = "com_github_jetbrains_kotlin"
KtJvmInfo = provider(
fields = {
"module_name": "the module name",
"module_jars": "Jars comprising the module (logical compilation unit), a.k.a. associates",
"exported_compiler_plugins": "compiler plugins to be invoked by targets depending on this.",
"srcs": "the source files. [intelij-aspect]",
"outputs": "output jars produced by this rule. [intelij-aspect]",
Expand Down
13 changes: 12 additions & 1 deletion kotlin/internal/jvm/android.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,17 @@ load(

_ANDROID_SDK_JAR = "%s" % Label("//third_party:android_sdk")

def _kt_android_artifact(name, srcs = [], deps = [], plugins = [], friends = [], kotlinc_opts = None, javac_opts = None, enable_data_binding = False, **kwargs):
def _kt_android_artifact(
name,
srcs = [],
deps = [],
plugins = [],
friends = None,
associates = [],
kotlinc_opts = None,
javac_opts = None,
enable_data_binding = False,
**kwargs):
"""Delegates Android related build attributes to the native rules but uses the Kotlin builder to compile Java and
Kotlin srcs. Returns a sequence of labels that a wrapping macro should export.
"""
Expand All @@ -42,6 +52,7 @@ def _kt_android_artifact(name, srcs = [], deps = [], plugins = [], friends = [],
deps = base_deps + [base_name],
plugins = plugins,
friends = friends,
associates = associates,
testonly = kwargs.get("testonly", default = False),
visibility = ["//visibility:private"],
kotlinc_opts = kotlinc_opts,
Expand Down
83 changes: 83 additions & 0 deletions kotlin/internal/jvm/associates.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Copyright 2020 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
load(
"//kotlin/internal:defs.bzl",
_KtJvmInfo = "KtJvmInfo",
)
load(
"//kotlin/internal/utils:sets.bzl",
_sets = "sets",
)
load(
"//kotlin/internal/utils:utils.bzl",
_utils = "utils",
)

def _get_associates(ctx):
"""Creates a struct of associates meta data"""

friends_legacy = getattr(ctx.attr, "friends", [])
associates = getattr(ctx.attr, "associates", [])

if friends_legacy:
print("WARNING: friends=[...] is deprecated, please prefer associates=[...] instead.")
if associates:
fail("friends= may not be used together with associates=. Use one or the other.")
elif ctx.attr.testonly == False:
fail("Only testonly targets can use the friends attribute. ")
else:
associates = friends_legacy

if not bool(associates):
return struct(
targets = [],
module_name = _utils.derive_module_name(ctx),
jars = [],
)
elif ctx.attr.module_name:
fail("if associates have been set then module_name cannot be provided")
else:
jars = [depset([a], transitive = a[_KtJvmInfo].module_jars) for a in associates]
module_names = _sets.copy_of([x[_KtJvmInfo].module_name for x in associates])
if len(module_names) > 1:
fail("Dependencies from several different kotlin modules cannot be associated. " +
"Associates can see each other's \"internal\" members, and so must only be " +
"used with other targets in the same module: \n%s" % module_names)
if len(module_names) < 1:
# This should be impossible
fail("Error in rules - a KtJvmInfo was found which did not have a module_name")
return struct(
targets = associates,
jars = jars,
module_name = list(module_names)[0],
)

def _flatten_jars(nested_jars_depset):
"""Returns a list of strings containing the compile_jars for depset of targets.
This ends up unwinding the nesting of depsets, since compile_jars contains depsets inside
the nested_jars targets, which themselves are depsets. This function is intended to be called
lazily form within Args.add_all(map_each) as it collapses depsets.
"""
compile_jars_depsets = [
target[JavaInfo].compile_jars
for target in nested_jars_depset.to_list()
if target[JavaInfo].compile_jars
]
return [file.path for file in depset(transitive = compile_jars_depsets).to_list()]

associate_utils = struct(
get_associates = _get_associates,
flatten_jars = _flatten_jars,
)
Loading

0 comments on commit 15dd1de

Please sign in to comment.