[Hilt] `enableExperimentalClasspathAggregation` breaks lint

[sergeys-opera]

Enabling enableExperimentalClasspathAggregation in a multi-module project breaks app:lint task.

$ ./gradlew :app:lintRelease

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':app:lintRelease'.
> Could not resolve all files for configuration ':app:debugRuntimeClasspath'.
   > Failed to transform full.jar (project :ui) to match attributes {artifactType=jar-for-dagger, com.android.build.api.attributes.BuildTypeAttr=debug, com.android.build.api.attributes.VariantAttr=debug, org.gradle.libraryelements=jar, org.gradle.usage=java-runtime}.
      > Execution failed for JetifyTransform: /x/y/z/dep/build/intermediates/full_jar/debug/full.jar.
         > Transform's input file does not exist: /x/y/z/dep/build/intermediates/full_jar/debug/full.jar. (See https://issuetracker.google.com/issues/158753935)

This issue has probably the same cause as https://github.com/google/dagger/issues/2288 but IMO is more important because lint usually runs automatically on CI server and cannot be simply disabled/removed.

It might be that it is an issue in lint (see https://issuetracker.google.com/issues/158060799) but it occurs only after integrating Hilt with enableExperimentalClasspathAggregation = true. And this makes me feel that it should be fixed in Hilt.

Dagger Hilt version: 2.32-alpha AGP version: 4.1.1

[danysantiago]

This is indeed an issue in Lint and should be fixed if you use AGP 7.0, we've been trying to backport a fix to AGP 4.2 but I don't think we'll have a fix for AGP 4.1, sorry.

[sergeys-opera]

That is sad because:

1. There is no timeline for when AGP 7.0 is released.
2. AGP 7.0 requires Java 11, Gradle 7, new Android Studio version and maybe a couple of other things. The transition to AGP 7.0 might not be that simple especially for bigger (and older) projects.

Is there a workaround for this? Or do we have to disable lint at all if we want to use enableExperimentalClasspathAggregation = true?

[jschmid]

For what it's worth, linting fails with AGP 4.2.0-beta05 and Gradle 6.7.1.

[trevjonez]

I personally think that we should consider enableExperimentalClasspathAggregation to be analogous to enableIllegalAndDontDoItEverBecauseItIsALieClasspathAggregation.

The difference between api and implementation has been clearly defined and the flag exists as a workaround for those not using either for the purposes for which they were defined.

The funny part being. that documentation and phrasing ends up leading people to overlook the "when possible" part, sadly when they shouldn't, the most.

In my experience with dagger, most abstractions are very leaky, to a fatal level. Which to me has displayed, proved, and meant, most things are API not implementation.

From there I can only hope they would remove the flag and educate people so that such a flag would never have a purpose. Because if you need this flag the library has a bug, that should not be accepted. Push fixes upstream, please don't work around them.

[jschmid]

Same issue with AGP 4.2.0-beta06

[danysantiago]

Ideally we don't want to have this flag at all but we are trying to address the high cognitive load and decision making process of using api vs implementation with Hilt. Specifically, even if the rules documented by Gradle are well-defined, it is still not easy to reason or make a decision based solely on the user code, since you would need to take into account Dagger/Hilt generated code which makes it very difficult for inexperienced users.

Let me explain the issue as we understand it, what the flag does and why we believe using api all over might not be the best approach.

The issue relates to Gradle module boundaries so we’ll need a chain of dependencies: app -> repo -> httpLibrary.

• app is the ‘root’ module, it defines the @HiltAndroidApp and depends on repo:

  
  package app;

@HiltAndroidApp class MyApp extends Application { @Inject Repo repo; }

* `repo` has public APIs used by the app and internally uses a http library:
```java
package data;

public class Repo {
  @Inject public Repo() { }

  private HttpClient getCient() { }
}

• httpLibrary is a 3rd party dependency fetched from a maven repository. Note that HttpClient is a type that is never exposed as part of repo’s API.

If repo declares its dependency as implementation and since Dagger takes a look at Repo, things will fail with a ‘not found type’ because none of the types and classes from httpLibrary are in the compile classpath of app. This is not so bad, since you get a compile error and even though it can be hard to understand the issue based on the error message, it can be fixed, but there are other more dangerous cases where the errors are silent, specifically with Hilt.

Consider a multibinding case, where you have a module that contributes to a multibinding.

In our repo module we now have:

package data;

public class Repo {
  @Inject public Repo(Set<Cache> caches) {
    registerCaches(caches);
    // ...
  }
}

repo now depends on a sqlCache Gradle module which contains:

@Module
@InstallIn(SingletonComponent.class)
public final SQLCacheModule {
  @Provides
  @IntoSet
  public static Cache provideCache() { return new SQLCache(); }
}

A user might end up reasoning that implementation is the right choice because SQLCache is an implementation class that is exposed as the Cache interface, a common interface used by repo and sqlCache. In this case however, because the SQLCacheModule is not visible to the root (app), the module will not be installed and the cache will not show up in the multibinding set. This is very dangerous because it becomes a runtime issue instead of a compilation error. The set will have a missing entry.

The flag is an attempt to address the friction and the incompatibility of the recommendation vs what Dagger processes. This is done by gathering your dependencies and adding them as compileOnly to the root. In other words and using the examples above, app, besides having an implementation dependency on repo, it will also have a compileOnly dependency to httpLibrary / sqlCache.

Using api all around is not the best solution either, because types are leaked in between modules, it increases the compile classpath at each node until the root, and more modules have to be compiled along the dependency line when a change is done in a leaf. Whereas the flag only increases the compile classpath at the root.

Since the errors are silent in Hilt we need to do something. We could try detecting the issue and informing the user, making using implementation an error, but using api has the disadvantages mentioned above. Given these tradeoffs, we decided it'd be best to offer a flag to opt-in into this behaviour that we wish we could just do automatically. Instead of expecting each user to know the internals of Dagger generated code so that they can make their api vs implementation decision. We hope to make changes to AGP to make this more automatic and to remove the current issues (such as Lint) and hackiness around the flag implementation.

[danysantiago]

This is fixed by https://github.com/google/dagger/commit/823570363429e7a5e0ae3e3078d9c14b4a8f77ad, we've introduced a new plugin flag called enableAggregatingTask that will replace enableExperimentalClasspathAggregation in a future version and possibly become the default behavior. We still have a bit more work to do, so will re-open this for now (since the bot closed it) and will actually mark it as resolve once we do a release with these changes and further documentation.

[aelobosz]

Hi, I have another issue switching to hilt {enableAggregatingTask = true}. This change solves the Lint error but I have a new error with product flavors when run dependencies task. When I put a flavor with name axxx and another bxxx it doesnt fail, but if I put a flavor that begins with "h" ("a" to "g" works) it throws me the following error.

Execution failed for task ':app:dependencies'.

Cannot change dependencies of dependency configuration ':app:hmsDebugAndroidTestRuntimeClasspath' after it has been included in dependency resolution. Use 'defaultDependencies' instead of 'beforeResolve' to specify default dependencies for a configuration.

this configuration fails !!

flavorDimensions "service"
    productFlavors{
        gms{
            dimension "service"
        }
        hms{
            dimension "service"
        }
    }

This works!!

 flavorDimensions "service"
    productFlavors{
        gms{
            dimension "service"
        }
        ahms{
            dimension "service"
        }
    }

"Hms" works too (capital)

[eric-labelle]

FYI, @aelobosz I documented the same issue here https://github.com/google/dagger/issues/2744