-
Notifications
You must be signed in to change notification settings - Fork 367
Remove dependency on KotlinCompile.libraries #2748
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,9 +17,11 @@ | |
|
|
||
| package com.google.devtools.ksp.gradle | ||
|
|
||
| import com.android.build.api.variant.AndroidComponentsExtension | ||
| import com.google.devtools.ksp.gradle.utils.allKotlinSourceSetsObservable | ||
| import com.google.devtools.ksp.gradle.utils.canUseGeneratedKotlinApi | ||
| import com.google.devtools.ksp.gradle.utils.enableProjectIsolationCompatibleCodepath | ||
| import com.google.devtools.ksp.gradle.utils.isAgpBuiltInKotlinUsed | ||
| import com.google.devtools.ksp.impl.KotlinSymbolProcessing | ||
| import com.google.devtools.ksp.processing.ExitCode | ||
| import com.google.devtools.ksp.processing.KSPCommonConfig | ||
|
|
@@ -31,6 +33,7 @@ import com.google.devtools.ksp.processing.KspGradleLogger | |
| import org.gradle.api.DefaultTask | ||
| import org.gradle.api.JavaVersion | ||
| import org.gradle.api.artifacts.Configuration | ||
| import org.gradle.api.attributes.Attribute | ||
| import org.gradle.api.file.ConfigurableFileCollection | ||
| import org.gradle.api.file.DirectoryProperty | ||
| import org.gradle.api.logging.LogLevel | ||
|
|
@@ -223,21 +226,38 @@ abstract class KspAATask @Inject constructor( | |
| } | ||
|
|
||
| if (kotlinCompilation is KotlinJvmAndroidCompilation) { | ||
| // Workaround of a dependency resolution issue of AGP. | ||
| // FIXME: figure out how to filter or set variant attributes correctly. | ||
| val kaptGeneratedClassesDir = getKaptGeneratedClassesDir(project, sourceSetName) | ||
| val kspOutputDir = KspGradleSubplugin.getKspOutputDir(project, sourceSetName, target) | ||
| cfg.libraries.from( | ||
| project.files( | ||
| Callable { | ||
| kotlinCompileProvider.get().libraries.filter { | ||
| !kspOutputDir.get().asFile.isParentOf(it) && | ||
| !kaptGeneratedClassesDir.isParentOf(it) && | ||
| !(it.isDirectory && it.listFiles()?.isEmpty() == true) | ||
| } | ||
| if (project.isAgpBuiltInKotlinUsed()) { | ||
| // when legacy-kapt plugin is applied, we can't use KotlinCompile.libraries directly | ||
| // because it contains the kapt output classes dir leading to circular dependency | ||
| val androidComponents = project.extensions.getByType(AndroidComponentsExtension::class.java) | ||
| cfg.libraries.from(androidComponents.sdkComponents.bootClasspath) | ||
| cfg.libraries.from( | ||
| project.provider { | ||
| val configuration = project.configurations.getByName( | ||
| kotlinCompilation.compileDependencyConfigurationName | ||
| ) | ||
| configuration.incoming.artifactView { config -> | ||
| config.attributes.attribute( | ||
| Attribute.of("artifactType", String::class.java), "android-classes-jar" | ||
| ) | ||
| }.files | ||
| } | ||
| ) | ||
| ) | ||
| } else { | ||
| val kaptGeneratedClassesDir = getKaptGeneratedClassesDir(project, sourceSetName) | ||
| val kspOutputDir = KspGradleSubplugin.getKspOutputDir(project, sourceSetName, target) | ||
| cfg.libraries.from( | ||
| project.files( | ||
| Callable { | ||
| kotlinCompileProvider.get().libraries.filter { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I havent had a chance to think about it yet. There is 2 things that need to be answered:
If KGP implements the new API we can move to it as soon as it is released. Otherwise I believe we need some time (so users can test this) to determine if the alternative really covers all edge cases out there |
||
| !kspOutputDir.get().asFile.isParentOf(it) && | ||
| !kaptGeneratedClassesDir.isParentOf(it) && | ||
| !(it.isDirectory && it.listFiles()?.isEmpty() == true) | ||
| } | ||
| } | ||
| ) | ||
| ) | ||
| } | ||
| } else { | ||
| cfg.libraries.from( | ||
| project.provider { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package com.google.devtools.ksp.test | ||
|
|
||
| import com.google.devtools.ksp.test.fixtures.TemporaryTestProject | ||
| import org.gradle.testkit.runner.GradleRunner | ||
| import org.junit.Assert | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
|
|
||
| class AndroidDataBindingBuiltInKotlinIT { | ||
| @Rule | ||
| @JvmField | ||
| val project: TemporaryTestProject = TemporaryTestProject("android-data-binding-builtinkotlin") | ||
|
|
||
| @Test | ||
| fun testPlaygroundAndroid() { | ||
| val gradleRunner = GradleRunner.create().withProjectDir(project.root) | ||
|
|
||
| // Disabling configuration cache. See https://github.com/google/ksp/issues/299 for details | ||
| gradleRunner.withArguments( | ||
| "clean", | ||
| ":app:assemble", | ||
| "--configuration-cache-problems=warn", | ||
| "--info", | ||
| "--stacktrace" | ||
| ) | ||
| .build().let { result -> | ||
| val output = result.output.lines() | ||
| val kspTask = output.filter { | ||
| it.contains(":app:kspDebugKotlin") | ||
| } | ||
| Assert.assertTrue(kspTask.isNotEmpty()) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package com.google.devtools.ksp.test | ||
|
|
||
| import com.google.devtools.ksp.test.fixtures.TemporaryTestProject | ||
| import org.gradle.testkit.runner.GradleRunner | ||
| import org.junit.Assert | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
|
|
||
| class LegacyKaptKspIT { | ||
| @Rule | ||
| @JvmField | ||
| val project: TemporaryTestProject = TemporaryTestProject("legacy-kapt", "playground") | ||
|
|
||
| @Test | ||
| fun testPlaygroundAndroid() { | ||
| val gradleRunner = GradleRunner.create().withProjectDir(project.root) | ||
| gradleRunner.withArguments( | ||
| "clean", | ||
| ":app:testDebugUnitTest", | ||
| "--configuration-cache-problems=warn", | ||
| "--info", | ||
| "--stacktrace" | ||
| ).build().let { result -> | ||
| val output = result.output.lines() | ||
| val kspTask = output.filter { it.contains(":app:kspDebugKotlin") } | ||
| val kaptTask = output.filter { it.contains(":app:kaptDebugKotlin") } | ||
| Assert.assertTrue(kspTask.isNotEmpty()) | ||
| Assert.assertTrue(kaptTask.isNotEmpty()) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import org.jetbrains.kotlin.gradle.dsl.JvmTarget | ||
|
|
||
| plugins { | ||
| id("com.android.application") | ||
| id("com.google.dagger.hilt.android") version "2.57.2" | ||
| id("com.google.devtools.ksp") | ||
| } | ||
|
|
||
| dependencies { | ||
| implementation("androidx.activity:activity:1.11.0") | ||
| implementation("com.google.dagger:hilt-android:2.57.2") | ||
| ksp("com.google.dagger:hilt-compiler:2.57.2") | ||
| } | ||
|
|
||
| android { | ||
| namespace = "com.example.databinding" | ||
| compileSdk = 36 | ||
|
|
||
| defaultConfig { | ||
| applicationId = "com.example.databinding" | ||
| minSdk = 24 | ||
| targetSdk = 36 | ||
| versionCode = 1 | ||
| versionName = "1.0" | ||
| } | ||
|
|
||
| buildFeatures { | ||
| dataBinding = true | ||
| } | ||
|
|
||
| dataBinding { | ||
| enable = true | ||
| } | ||
|
|
||
| compileOptions { | ||
| sourceCompatibility = JavaVersion.VERSION_11 | ||
| targetCompatibility = JavaVersion.VERSION_11 | ||
| } | ||
| } | ||
|
|
||
| kotlin { | ||
| compilerOptions { | ||
| jvmTarget = JvmTarget.JVM_11 | ||
| } | ||
| } | ||
|
|
||
| hilt { | ||
| enableAggregatingTask = false | ||
| } | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | ||
| xmlns:tools="http://schemas.android.com/tools"> | ||
|
|
||
| <application | ||
| android:name=".App" | ||
| android:label="KSP #2597 Repro" | ||
| android:theme="@android:style/Theme.Material.Light.NoActionBar" | ||
| tools:ignore="MissingApplicationIcon"> | ||
|
|
||
| <activity | ||
| android:name=".MainActivity" | ||
| android:exported="true"> | ||
|
|
||
| <intent-filter> | ||
| <action android:name="android.intent.action.MAIN" /> | ||
| <category android:name="android.intent.category.LAUNCHER" /> | ||
| </intent-filter> | ||
| </activity> | ||
|
|
||
| </application> | ||
| </manifest> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package com.example.databinding | ||
|
|
||
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
|
|
||
| @Singleton class A @Inject constructor() { | ||
| val string = "a" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package com.example.databinding | ||
|
|
||
| import android.app.Application | ||
| import dagger.hilt.android.HiltAndroidApp | ||
|
|
||
| @HiltAndroidApp | ||
| class App : Application() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package com.example.databinding | ||
|
|
||
| import android.os.Bundle | ||
| import androidx.activity.ComponentActivity | ||
| import androidx.annotation.CallSuper | ||
| import androidx.annotation.LayoutRes | ||
| import androidx.databinding.DataBindingUtil | ||
| import androidx.databinding.ViewDataBinding | ||
|
|
||
| abstract class BindingActivity<T : ViewDataBinding>( | ||
| @get:LayoutRes private val layoutId: Int, | ||
| ) : ComponentActivity() { | ||
| protected lateinit var binding: T | ||
| private set | ||
|
|
||
| @CallSuper override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| binding = DataBindingUtil.setContentView<T>(this, layoutId) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package com.example.databinding | ||
|
|
||
| import android.os.Bundle | ||
| import com.example.databinding.databinding.ActivityMainBinding | ||
| import dagger.hilt.android.AndroidEntryPoint | ||
| import javax.inject.Inject | ||
|
|
||
| @AndroidEntryPoint | ||
| class MainActivity : BindingActivity<ActivityMainBinding>(R.layout.activity_main) { | ||
| @Inject lateinit var a: A | ||
|
|
||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| binding.a = a | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <layout xmlns:android="http://schemas.android.com/apk/res/android" | ||
| xmlns:tools="http://schemas.android.com/tools"> | ||
|
|
||
| <data> | ||
| <variable | ||
| name="a" | ||
| type="com.example.databinding.A" /> | ||
| </data> | ||
|
|
||
| <LinearLayout | ||
| android:id="@+id/main" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:layout_margin="30dp" | ||
| tools:context=".MainActivity"> | ||
|
|
||
| <TextView | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="@{a.string}" /> | ||
|
|
||
| </LinearLayout> | ||
| </layout> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| buildscript { | ||
| val testRepo: String by project | ||
|
|
||
| repositories { | ||
| maven(testRepo) | ||
| mavenCentral() | ||
| maven("https://redirector.kotlinlang.org/maven/bootstrap/") | ||
| google() | ||
| } | ||
| } | ||
|
|
||
| plugins { | ||
| id("com.android.application") apply false | ||
| id("com.google.devtools.ksp") apply false | ||
| } | ||
|
|
||
| allprojects { | ||
| val testRepo: String by project | ||
| repositories { | ||
| maven(testRepo) | ||
| mavenCentral() | ||
| maven("https://redirector.kotlinlang.org/maven/bootstrap/") | ||
| google() | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| android.newDsl=false |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| pluginManagement { | ||
| val kotlinVersion: String by settings | ||
| val kspVersion: String by settings | ||
| val testRepo: String by settings | ||
| val agpVersion: String by settings | ||
| plugins { | ||
| id("com.google.devtools.ksp") version kspVersion apply false | ||
| kotlin("jvm") version kotlinVersion apply false | ||
| id("com.android.application") version agpVersion apply false | ||
| id("com.android.library") version agpVersion apply false | ||
| } | ||
| repositories { | ||
| maven(testRepo) | ||
| gradlePluginPortal() | ||
| google() | ||
| mavenCentral() | ||
| maven("https://redirector.kotlinlang.org/maven/bootstrap/") | ||
| } | ||
| } | ||
|
|
||
| include(":app") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| plugins { | ||
| id("com.android.application") | ||
| id("com.android.legacy-kapt") | ||
| id("com.google.devtools.ksp") | ||
| } | ||
| dependencies { | ||
| implementation("androidx.constraintlayout:constraintlayout:2.1.4") | ||
| ksp("androidx.room:room-compiler:2.4.2") | ||
| implementation("androidx.room:room-runtime:2.4.2") | ||
| implementation("androidx.appcompat:appcompat:1.6.1") | ||
| } | ||
| android { | ||
| namespace = "com.example.kspandroidtestapp" | ||
| defaultConfig { | ||
| minSdk = 24 | ||
| } | ||
| compileSdk = 34 | ||
| buildFeatures { | ||
| viewBinding = true | ||
| } | ||
| viewBinding { | ||
| enable = true | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <manifest /> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import android.os.Bundle | ||
| import androidx.appcompat.app.AppCompatActivity | ||
| import com.example.kspandroidtestapp.databinding.ActivityMainBinding | ||
|
|
||
| class MainActivity : AppCompatActivity() { | ||
|
|
||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
|
|
||
| val binding = ActivityMainBinding.inflate(layoutInflater) | ||
|
|
||
| setContentView(binding.root) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <androidx.constraintlayout.widget.ConstraintLayout | ||
| xmlns:android="http://schemas.android.com/apk/res/android" | ||
| xmlns:tools="http://schemas.android.com/tools" | ||
| tools:context="MainActivity" | ||
| android:layout_height="wrap_content" | ||
| android:layout_width="wrap_content"> | ||
| </androidx.constraintlayout.widget.ConstraintLayout> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a bug in AGP or KGP to me. Why isn't
artifactTypeproperly set inconfiguration?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as i understand the consumer of a specific configuration needs to define what artifactType they want to consume. So here we have KSP saying please give me the incoming dependencies from this configuration, and i only want to see classes jar even if the dependency contains much more. So i dont think this is a bug on the producer side of this configuration (which is KGP)