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

Cleanup Android dependencies #77

Merged
merged 9 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -157,22 +157,7 @@ load("//jvm/dependencies:deps.bzl", artifacts = "maven")
load("@rules_jvm_external//:defs.bzl", "maven_install")

maven_install(
artifacts = artifacts + [
"androidx.databinding:databinding-adapters:7.1.0",
"androidx.databinding:databinding-common:7.1.0",
"androidx.databinding:databinding-compiler:7.1.0",
"androidx.databinding:databinding-runtime:7.1.0",
"androidx.annotation:annotation:1.3.0",
"com.google.dagger:dagger:2.35.1",
"com.google.dagger:dagger-compiler:2.35.1",
"com.google.dagger:dagger-producers:2.35.1",
"javax.inject:javax.inject:1",
"org.robolectric:robolectric:4.8",
"androidx.test:core:1.4.0",
"androidx.test.ext:junit-ktx:1.1.3",
"junit:junit:4.12",
"org.junit.vintage:junit-vintage-engine:5.6.0",
],
artifacts = artifacts,
fetch_sources = True,
override_targets = overridden_targets,
repositories = [
Expand Down
7 changes: 7 additions & 0 deletions android/demo/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_import")
load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")
load(":deps.bzl", "main_deps", "test_deps")
load("//android:build.bzl", "applitools_config")
load("@rules_player//kotlin:lint.bzl", "lint")

kt_android_library(
name = "demo_lib",
Expand Down Expand Up @@ -80,3 +81,9 @@ sh_test(
":demo_test_app",
],
)

lint(
name = "demo",
srcs = glob(["src/**/*.kt"]),
lint_config = "//jvm:lint_config",
)
30 changes: 11 additions & 19 deletions android/demo/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,26 @@ load("//jvm/dependencies:versions.bzl", "versions")
load("@rules_player//maven:parse_coordinates.bzl", "parse_coordinates")

maven_main = [
"androidx.appcompat:appcompat:1.2.0",
"androidx.core:core-ktx:1.3.2",
"androidx.constraintlayout:constraintlayout:2.0.4",
"androidx.navigation:navigation-fragment-ktx:2.3.3",
"androidx.navigation:navigation-ui-ktx:2.3.3",
"androidx.navigation:navigation-fragment:2.3.3",
"androidx.navigation:navigation-ui:2.3.3",
"androidx.navigation:navigation-runtime:2.3.3",
"com.afollestad.material-dialogs:core:3.3.0",
"androidx.navigation:navigation-runtime:%s" % versions.androidx.navigation,
"androidx.navigation:navigation-ui-ktx:%s" % versions.androidx.navigation,
"androidx.navigation:navigation-fragment-ktx:%s" % versions.androidx.navigation,

"com.afollestad.material-dialogs:core:%s" % versions.material_dialogs,
"com.google.android.material:material:%s" % versions.material,
#"com.squareup.leakcanary:leakcanary-android:2.2",
]

maven_test = [
"com.applitools:eyes-android-espresso:4.7.6",
"androidx.test:runner:1.3.0",
"androidx.test:rules:1.3.0",
"androidx.test.espresso:espresso-core:3.3.0",
"androidx.test.espresso:espresso-contrib:3.3.0",
"androidx.test.espresso:espresso-intents:3.3.0",
"androidx.test.ext:junit-ktx:1.1.2",
"androidx.test.espresso:espresso-intents:%s" % versions.androidx.test.espresso,
"androidx.test.ext:junit-ktx:%s" % versions.androidx.test.junit,
"com.applitools:eyes-android-espresso:%s" % versions.testing.applitools,
]

maven = maven_main + maven_test

main_deps = parse_coordinates(maven) + [
"//android/player",
"//plugins/reference-assets/android:assets",
main_deps = parse_coordinates(maven_main) + [
"//jvm/utils",
"//plugins/reference-assets/android:assets",
"//plugins/common-types/jvm:common-types",
"//plugins/pending-transaction/jvm:pending-transaction",
"//plugins/reference-assets/mocks:jar",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ abstract class ApplitoolsTest {
fun Eyes.checkPlayer(name: String) = check(name, Target.region(ViewMatchers.withId(R.id.player_canvas)))

companion object {
val batchInfo = BatchInfo("reference-assets@${PR_NUMBER}").apply {
val batchInfo = BatchInfo("reference-assets@$PR_NUMBER").apply {
// Only manually set the batch ID if it's not a hardcoded fallback
if (BATCH_ID != "local") id = BATCH_ID
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import androidx.navigation.fragment.NavHostFragment
import androidx.navigation.ui.*
import com.google.android.material.navigation.NavigationView
import com.intuit.player.android.reference.demo.R
import com.intuit.player.android.reference.demo.lifecycle.DemoPlayerViewModel
import com.intuit.player.android.reference.demo.model.AssetMock
import com.intuit.player.android.reference.demo.model.StringMock
import com.intuit.player.android.ui.PlayerFragment
Expand Down
21 changes: 21 additions & 0 deletions android/deps.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
load("//jvm/dependencies:versions.bzl", "versions")
load("//android/player:deps.bzl", player = "maven")
load("//android/demo:deps.bzl", demo = "maven")

android = [
# Grab Databinding
"androidx.databinding:databinding-adapters:%s" % versions.androidx.databinding,
"androidx.databinding:databinding-common:%s" % versions.androidx.databinding,
"androidx.databinding:databinding-runtime:%s" % versions.androidx.databinding,

# Grab Dagger
"com.google.dagger:dagger:%s" % versions.dagger,
"com.google.dagger:dagger-compiler:%s" % versions.dagger,
"javax.inject:javax.inject:%s" % versions.javax.inject,

# AndroidX Resolutions
"androidx.activity:activity-ktx:%s" % versions.androidx.activity,
"androidx.fragment:fragment-ktx:%s" % versions.androidx.fragment,
]

maven = android + player + demo
16 changes: 9 additions & 7 deletions android/player/BUILD
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
load(":deps.bzl", "main_deps", "main_resources")
load(":deps.bzl", "main_deps", "main_resources", "test_deps")
load("//jvm:build.bzl", "distribution")
load("@grab_bazel_common//tools/databinding:databinding.bzl", "kt_db_android_library")
load("@junit//junit5-jupiter-starter-bazel:junit5.bzl", "kt_jvm_junit5_test")
load("@rules_player//kotlin:lint.bzl", "lint")

kt_db_android_library(
name = "player",
Expand All @@ -25,10 +26,11 @@ kt_jvm_junit5_test(
associates = [":player-kotlin"],
kotlinc_opts = "//jvm:test_options",
test_package = "com.intuit.player.android",
deps = [
":player",
"//jvm/testutils",
"@grab_bazel_common//tools/test:mockable-android-jar",
"@maven//:io_mockk_mockk",
],
deps = [":player"] + test_deps,
)

lint(
name = "player",
srcs = glob(["src/**/*.kt"]),
lint_config = "//jvm:lint_config",
)
24 changes: 16 additions & 8 deletions android/player/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ load("//jvm/dependencies:versions.bzl", "versions")
load("@rules_player//maven:parse_coordinates.bzl", "parse_coordinates")

maven = [
"org.jetbrains.kotlinx:kotlinx-coroutines-android:%s" % versions.kotlin.coroutines,
# UI helpers
"androidx.core:core-ktx:%s" % versions.androidx.core,
"androidx.appcompat:appcompat:%s" % versions.androidx.appcompat,
"androidx.transition:transition:%s" % versions.androidx.transition,

# TODO: Potentially externalize versions
"androidx.appcompat:appcompat:1.2.0",
"androidx.core:core-ktx:1.3.2",
"androidx.constraintlayout:constraintlayout:2.0.4",
"androidx.navigation:navigation-fragment-ktx:2.3.3",
"androidx.navigation:navigation-ui-ktx:2.3.3",
"androidx.transition:transition:1.4.1",
# Lifecycle
"androidx.lifecycle:lifecycle-runtime-ktx:%s" % versions.androidx.lifecycle,
"androidx.lifecycle:lifecycle-viewmodel-ktx:%s" % versions.androidx.lifecycle,

# Default fallback
Copy link
Contributor

Choose a reason for hiding this comment

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

what's constraintlayout gotta do with default fallback? you mean the fallbackView? is that the only place we use it..?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would love to not have a dependency on constraintlayout in the base android player dep, but we are using it to build the default fallback view:

https://github.com/player-ui/player/blob/main/android/player/src/main/res/layout/default_fallback.xml#L2

"androidx.constraintlayout:constraintlayout:%s" % versions.androidx.constraintlayout,
]

main_exports = [
Expand All @@ -29,3 +31,9 @@ main_resources = [
"//plugins/partial-match-fingerprint/core:PartialMatchFingerprintPlugin_Bundles",
"//core/partial-match-registry:Registry_Bundles",
]

test_deps = [
"@grab_bazel_common//tools/test:mockable-android-jar",
"@maven//:io_mockk_mockk",
"//jvm/testutils",
]
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import com.intuit.player.android.extensions.Styles
import com.intuit.player.android.extensions.removeSelf
import com.intuit.player.jvm.core.asset.Asset
import com.intuit.player.jvm.core.asset.AssetWrapper
import com.intuit.player.jvm.core.bridge.Invokable
import com.intuit.player.jvm.core.bridge.Node
import com.intuit.player.jvm.core.bridge.NodeWrapper
import com.intuit.player.jvm.core.bridge.serialization.encoding.requireNodeDecoder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public open class PlayerViewModel(flows: AsyncFlowIterator) : ViewModel(), Andro
private val factory: (AsyncFlowIterator) -> T = { i -> PlayerViewModel(i) as T }
) : ViewModelProvider.Factory {

override fun <T : ViewModel?> create(modelClass: Class<T>): T {
override fun <T : ViewModel> create(modelClass: Class<T>): T {
return factory(iterator).apply(PlayerViewModel::start) as T
}
}
Expand Down
6 changes: 3 additions & 3 deletions jvm/dependencies/deps.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@rules_player//javascript:utils.bzl", "remove_duplicates")
load(":common.bzl", common = "maven")
load("//android/player:deps.bzl", android = "maven")
load("//android/demo:deps.bzl", demo = "maven")
load("//android:deps.bzl", android = "maven")
load("//jvm/core:deps.bzl", core = "maven")
load("//jvm/graaljs:deps.bzl", graaljs = "maven")
load("//jvm/j2v8:deps.bzl", j2v8 = "maven")
Expand All @@ -13,4 +13,4 @@ load("@grab_bazel_common//:workspace_defs.bzl", grab = "GRAB_BAZEL_COMMON_ARTIFA

tooling = distribution + grab

maven = common + core + graaljs + j2v8 + utils + testutils + perf + plugins + tooling + android + demo
maven = remove_duplicates(common + core + graaljs + j2v8 + utils + testutils + perf + plugins + tooling + android)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥

25 changes: 25 additions & 0 deletions jvm/dependencies/versions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,34 @@ versions = struct(
),
hooks = "0.11.1",
testing = struct(
applitools = "4.7.6",
junit = "4.12",
jupiter = "5.6.0",
kluent = "1.68",
mockk = "1.9.3",
robolectric = "4.8",
),
jmh = "1.21",
androidx = struct(
activity = "1.2.3",
appcompat = "1.3.0",
constraintlayout = "2.1.4",
core = "1.6.0",
databinding = "7.2.2",
fragment = "1.3.4",
lifecycle = "2.4.0",
navigation = "2.3.3",
test = struct(
core = "1.4.0",
espresso = "3.3.0",
junit = "1.1.3",
),
transition = "1.4.1",
),
dagger = "2.35.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

i recall you saying you needed to add this for some odd reason, do we still need this

Copy link
Member Author

Choose a reason for hiding this comment

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

dagger is needed to load the grab common repository -- at least it was when i initially integrated. it'd be cool to get off that repo eventually

javax = struct(
inject = "1",
),
material_dialogs = "3.3.0",
material = "1.4.0",
)
11 changes: 10 additions & 1 deletion plugins/reference-assets/android/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")
load(":deps.bzl", "main_deps")
load("@rules_player//kotlin:lint.bzl", "lint")
load(":deps.bzl", "main_deps", "main_exports")

kt_android_library(
name = "assets",
Expand All @@ -8,5 +9,13 @@ kt_android_library(
manifest = ":src/main/AndroidManifest.xml",
resource_files = glob(["src/main/res/**"]),
deps = main_deps,
exports = main_exports,
visibility = ["//visibility:public"],
)

lint(
name = "assets",
srcs = glob(["src/main/**/*.kt"]),
lint_config = "//jvm:lint_config",
)

9 changes: 8 additions & 1 deletion plugins/reference-assets/android/build.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_local_test")
load("@rules_player//kotlin:lint.bzl", "lint")

def kt_asset_test(
name,
Expand All @@ -12,7 +13,7 @@ def kt_asset_test(
custom_package = "com.intuit.player.android.reference.assets",
test_class = test_class,
deps = deps + [
"//plugins/reference-assets/android/src/test/java/com/intuit/player/android/reference/assets/test",
"//plugins/reference-assets/android/src/androidTest/java/com/intuit/player/android/reference/assets/test",
"//jvm/j2v8:j2v8-all",
],
resources = [
Expand All @@ -22,3 +23,9 @@ def kt_asset_test(
"minSdkVersion": "14",
},
)

lint(
name = name,
srcs = native.glob(["**/*.kt"]),
lint_config = "//jvm:lint_config",
)
13 changes: 5 additions & 8 deletions plugins/reference-assets/android/deps.bzl
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
load("//jvm/dependencies:versions.bzl", "versions")
load("@rules_player//maven:parse_coordinates.bzl", "parse_coordinates")
load("//plugins/reference-assets/android/src/androidTest/java/com/intuit/player/android/reference/assets/test:deps.bzl", maven_test = "maven")

maven = [
"org.jetbrains.kotlinx:kotlinx-coroutines-android:%s" % versions.kotlin.coroutines,

"androidx.appcompat:appcompat:1.2.0",
"androidx.core:core-ktx:1.3.2",
"androidx.constraintlayout:constraintlayout:2.0.4",
]
maven_main = []

main_exports = [
"//android/player",
]

main_deps = main_exports + parse_coordinates(maven) + [
main_deps = main_exports + parse_coordinates(maven_main) + [
Copy link
Contributor

Choose a reason for hiding this comment

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

since maven_main doesn't have anything in it right now, maybe remove the parse_coordinates, i'm not against keeping the maven_main though

Copy link
Member Author

@sugarmanz sugarmanz Aug 31, 2022

Choose a reason for hiding this comment

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

This is more for the patternization of deps.bzl. If someone wants to add another dependency, all they need to do right now is add "androidx.core:core-ktx:%s" % versions.androidx.core" and that will automatically be pulled into the maven_install_and_ converted to the corresponding@maven//:androidx_core_core_ktx` Bazel reference.

If we remove the parse_coordinates, then they'd need to do:

converted to the corresponding @maven//:androidx_core_core_ktx Bazel reference.

manually, or reintroduce the parse_coordinates method.

"//jvm:kotlin_serialization",
"//plugins/reference-assets/jvm:reference-assets",
"//plugins/pending-transaction/jvm:pending-transaction",
]

maven = maven_main + maven_test
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.intuit.player.android.reference.assets.input

import android.widget.FrameLayout
import android.widget.LinearLayout
import android.widget.TextView
import androidx.constraintlayout.widget.ConstraintLayout
import androidx.core.view.get
Expand All @@ -12,7 +11,6 @@ import com.intuit.player.android.reference.assets.test.shouldBeView
import com.intuit.player.jvm.core.player.state.InProgressState
import com.intuit.player.jvm.core.player.state.dataModel
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Test

class InputTest : AssetTest("input") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import android.content.Context
import android.os.Build.VERSION_CODES.P
import android.view.View
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.runner.AndroidJUnit4
import com.intuit.player.android.AndroidPlayer
import com.intuit.player.android.asset.RenderableAsset
import com.intuit.player.android.reference.assets.ReferenceAssetsPlugin
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")
load("@rules_player//kotlin:lint.bzl", "lint")
load(":deps.bzl", "main_deps")

kt_android_library(
name = "test",
srcs = ["assertions.kt", "AssetTest.kt"],
deps = main_deps,
visibility = ["//plugins/reference-assets/android:__subpackages__"],
)

lint(
name = "test",
srcs = glob(["*.kt"]),
lint_config = "//jvm:lint_config",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("//jvm/dependencies:versions.bzl", "versions")
load("@rules_player//maven:parse_coordinates.bzl", "parse_coordinates")

maven = [
"androidx.test:core:%s" % versions.androidx.test.core,
"androidx.test:runner:%s" % versions.androidx.test.core,
"junit:junit:%s" % versions.testing.junit,
"org.robolectric:robolectric:%s" % versions.testing.robolectric,
]

main_deps = parse_coordinates(maven) + [
"@robolectric//bazel:android-all",
"//jvm/utils",
"//plugins/common-types/jvm:common-types",
"//plugins/pending-transaction/jvm:pending-transaction",
"//plugins/reference-assets/android:assets",
"//plugins/reference-assets/mocks:jar",
]
Loading