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

Lots of warnings with KSP related to incremental compilation #4182

Closed
rakshitsoni02 opened this issue Dec 7, 2023 · 14 comments
Closed

Lots of warnings with KSP related to incremental compilation #4182

rakshitsoni02 opened this issue Dec 7, 2023 · 14 comments

Comments

@rakshitsoni02
Copy link

**w: [ksp] No dependencies reported for generated source xxx_Factory.java which willprevent incremental compilation.**

There are almost above warnings for many classes, it's after migration from kapt to KSP

Using the latest KSP, AGP & Dagger[2.49]

@rakshitsoni02 rakshitsoni02 changed the title Lot of wanrings with KSP Lots of warnings with KSP related to incremental compilation Dec 7, 2023
@bcorso
Copy link

bcorso commented Dec 7, 2023

I wasn't able to reproduce this. If you can share a minimal reproducible example project I can take a deeper look.

@rakshitsoni02
Copy link
Author

sure, I'll post the URL here soon, meanwhile we also mentioned the issue here as well

@michaelaubertcmc
Copy link

Hi @bcorso

@rakshitsoni02 and I work together

We got some additional information from the KSP team:

Does Dagger use the Room compiler-processing module to support KSP
https://android.googlesource.com/platform//frameworks/support/+/27daece9174f19f73e32b13d4f431bec940b03fd/room/compiler-processing/README.md

Each of the warnings we see tells us to raise a bug on Room (component 413107):

w: [ksp] No dependencies reported for generated source *******_Factory.java which will prevent incremental compilation.
Please file a bug at https://issuetracker.google.com/issues/new?component=413107

The Android application we're trying to build does not depend on Room. There is no ".room" anywhere in the source code.

Dagger 2.50 doesn't fix the issue.

Considering the Room compiler-processor module doesn't appear to be a supported public library, is it possible we've hit an unexpected path in the code?

Beyond the pollution of build logs, we're concerned about the warning telling us our builds will be slower, since migrating from Kapt to KSP is supposed to make builds faster

@bcorso
Copy link

bcorso commented Jan 4, 2024

Yes, Dagger uses the XProcessing library, which is currently in Room.

Considering the Room compiler-processor module doesn't appear to be a supported public library, is it possible we've hit an unexpected path in the code?

Our team contributes to the XProcessing library and works closely with the Room team. We'll continue to fix bugs related to Dagger usage.

The main issue is that it's still unclear how you are getting in this error since we can't reproduce it ourselves. Were you able to create a minimal reproducible example project that you can share?

@michaelaubertcmc
Copy link

Thanks for the update @bcorso

The minimal reproducible example is probably not going to happen because the application we have the problem with has been in the Play Store for more than 10 years and contains ways too much technical debt. We wouldn't know where to start. It doesn't use Room or Realm but it uses both RxJava and Coroutine Flow. It contains at least 4 different screen navigation patterns in modules that were developed over the years. We use Dagger without Hilt.

It looks like migrating away from kapt will not be a low hanging fruit for us. I will post more when I have some time to better understand how Dagger/Xprocessing works and what the issue could possibly be.

@bcorso
Copy link

bcorso commented Jan 9, 2024

The error happens when you generate a source but don't have an originating element. However, in Dagger we always use the element that was annotated as the originating element, which is why it's unclear to me how you are hitting this.

However, looking deeper in XProcessing, there is one place where it could be dropping the originating element. In particular, it looks like fun XElement.originatingElementForPoet(): Element? tries to create a synthetic Element when using KSP, and unfortunately the wrapAsOriginatingElement() method will just return null if it can't properly create the synthetic Element.

I'm not 100% sure that's where the originating element is being dropped, but it seems like a good place to start.

On our side, I can also look into changing XProcessing so that wrapAsOriginatingElement() fails with a more specific error message when it can't create the synthetic Element rather than silently dropping it, which should help verify that's where the issue is and give you more information about what went wrong.

On your side, can you share a bit more about the xxx sources corresponding to the xxx_Factory.java in the warning?

  • Is it a source with an @Inject-annotated constructor?
  • Are all sources with @Inject-annotated constructors giving this warning or just a few?
  • If just a few, is there anything special/different with the ones giving the warning?

@bcorso
Copy link

bcorso commented Jan 9, 2024

FWIW, I've filed https://issuetracker.google.com/319297571 to XProcessing to give better error messages in this case.

@michaelaubertcmc
Copy link

Thanks for the leads @bcorso

I did a clean build of our Android application. That showed 362 KSP warnings.
I looked at about 15 of the source files, randomly in the list and they all had @Inject-annotated constructors.
I looked for @Injected-annotated constructors in our source tree and found 559.
I looked for source files with @Injected-annotated constructors and without a KSP warning, to compare with the files with both.

The number of parameters in the constructor (0 or more) doesn't seem to matter.
Neither do:

  • whether the Kotlin class is annotated with a javax.inject.Scope annotation
  • whether the Kotlin class contains a companion object
  • whether the Kotlin class extends an interface

I'll see what else I can think of

@HGyllensvard
Copy link

Hey everyone,

I just ran into this problem as we upgraded to ksp. We are on dagger 2.50.

In our case, we have a multi-module setup and the issue was related to how we had set up our dependencies.

Let's say we have
:app
:moduleA

In :app we applied ksp and dagger as expected.

Our toml file looks something like:

dagger = "2.50"

dagger = { module = "com.google.dagger:dagger", version.ref = "dagger" }
dagger-compiler = { module = "com.google.dagger:dagger-compiler", version.ref = "dagger" }

In our build.gradle:

dependencies {
    implementation libs.dagger
    ksp libs.dagger.compiler
}

What I noticed was that we only received the warning for files in certain modules. The issue was with our setup in :moduleA. While we injected classes from this module, we only used @Inject here, so in an attempt to speed up our builds, we did not have kapt (or now ksp) and dagger as dependencies in all modules but had only added the Javax annotation dependency inside these modules, in this case :moduleA.

Once I applied the dagger dependencies inside :moduleA the warning was resolved for us.

Hope that helps!

@rakshitsoni02
Copy link
Author

rakshitsoni02 commented Feb 8, 2024

Good shout @HGyllensvard
This has solved the warning issue, but I am curious in your case was there any difference in build time after adding the plugin in each module, it was the same after adding dagger dependencies in our feature modules, we were not adding in every module for the same build speed reason, I also believe the warning message can be improved in this case 😉

@bcorso
Copy link

bcorso commented Feb 8, 2024

While we injected classes from this module, we only used @Inject here, so in an attempt to speed up our builds, we did not have kapt (or now ksp) and dagger as dependencies in all modules but had only added the Javax annotation dependency inside these modules

Ironically, it likely worsened your build speed, at least in theory. In particular, while it may improve the build speed of :moduleA it typically worsens the build speed of your app overall (especially incremental builds).

The idea is that the more Dagger can do at the individual library level the more it can skip during incremental processing if that library isn't touched. In particular, if you remove Dagger's compiler from :moduleA Dagger will still process it's classes and generate the things it needs, but it will do it while compiling :app instead of :moduleA. Since incremental processing likely triggers :app a lot more than :moduleA you will end up paying that processing cost more often.

@rakshitsoni02 rakshitsoni02 reopened this Feb 8, 2024
@bcorso
Copy link

bcorso commented Feb 8, 2024

@rakshitsoni02 what's the reason you reopened this?

@rakshitsoni02
Copy link
Author

in case there is any need to update the warning message related to this issue?

so closing now, feel free to update the issue if needed. Thanks

@tiper
Copy link

tiper commented Feb 14, 2024

Hey @rakshitsoni02 and @bcorso.

A way to reproduce this warning is with the following scenario. Consider a feature module containing its own modules for data, domain, and dagger integration. In the domain module, you define contracts, while the data module provides concrete implementations of these contracts. However, imagine that one of these implementations requires a dependency from either another module or directly from the app.

In your dagger configuration, you specify the dependency modules, along with the binding of the required service. Both modules are added to your app component.

During compilation, within the feature's Dagger module, Dagger successfully generates only certain dependencies. However, dependencies that Dagger cannot generate, due to their reliance on dependencies from the app, will result in the triggering of this warning.

@bcorso I've attached here a simple project reproducing this.
Issue4182.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants