Skip to content

Commit

Permalink
Sort dependencies in ComponentTreeDeps manually to give consistent ou…
Browse files Browse the repository at this point in the history 
…tput.

Incremental processing can change the order we receive elements in our metadata package. This CL sorts our metadata deps manually by file name before generating the @ComponentTreeDeps.

Fixes #3006

RELNOTES=Fix #3006:Sort dependencies in ComponentTreeDeps manually to give consistent output.
PiperOrigin-RevId: 411619490
  • Loading branch information
@bcorso
bcorso authored and Dagger Team committed Nov 22, 2021
1 parent 0c52e85 commit 6769a1c
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 18 deletions.
58 changes: 58 additions & 0 deletions java/dagger/hilt/android/plugin/src/test/kotlin/IncrementalProcessorTest.kt
View file
Expand Up @@ -425,6 +425,7 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun changeActivitySource_addPublicMethod() {
runFullBuild()
val componentTreeDepsFullBuild = genComponentTreeDeps.readText(Charsets.UTF_8)

// Change Activity 1 source
searchAndReplace(
Expand Down Expand Up @@ -470,6 +471,11 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val componentTreeDepsIncrementalBuild = genComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(componentTreeDepsFullBuild)
.isEqualTo(componentTreeDepsIncrementalBuild)

// Check compilation outputs
// * Gen sources from activity 1 are re-compiled
val recompiledClassFiles = if (incapMode == ISOLATING_MODE) {
Expand Down Expand Up @@ -504,6 +510,7 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun changeActivitySource_addPrivateMethod() {
runFullBuild()
val componentTreeDepsFullBuild = genComponentTreeDeps.readText(Charsets.UTF_8)

// Change Activity 1 source
searchAndReplace(
Expand Down Expand Up @@ -548,6 +555,11 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val componentTreeDepsIncrementalBuild = genComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(componentTreeDepsFullBuild)
.isEqualTo(componentTreeDepsIncrementalBuild)

// Check compilation outputs
// * Gen sources from activity 1 are re-compiled
val recompiledClassFiles = if (incapMode == ISOLATING_MODE) {
Expand Down Expand Up @@ -578,6 +590,7 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun changeModuleSource() {
runFullBuild()
val componentTreeDepsFullBuild = genComponentTreeDeps.readText(Charsets.UTF_8)

// Change Module 1 source
searchAndReplace(
Expand Down Expand Up @@ -619,6 +632,11 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val componentTreeDepsIncrementalBuild = genComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(componentTreeDepsFullBuild)
.isEqualTo(componentTreeDepsIncrementalBuild)

// Check compilation outputs
// * Gen sources from module 1 are re-compiled
val recompiledClassFiles = if (incapMode == ISOLATING_MODE) {
Expand Down Expand Up @@ -649,6 +667,7 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun changeAppSource() {
runFullBuild()
val componentTreeDepsFullBuild = genComponentTreeDeps.readText(Charsets.UTF_8)

// Change Application source
searchAndReplace(
Expand Down Expand Up @@ -690,6 +709,11 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val componentTreeDepsIncrementalBuild = genComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(componentTreeDepsFullBuild)
.isEqualTo(componentTreeDepsIncrementalBuild)

// Check compilation outputs
val recompiledClassFiles = if (incapMode == ISOLATING_MODE) {
listOf(
Expand Down Expand Up @@ -719,6 +743,7 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun deleteActivitySource() {
runFullBuild()
val componentTreeDepsFullBuild = genComponentTreeDeps.readText(Charsets.UTF_8)

srcActivity2.delete()

Expand Down Expand Up @@ -758,6 +783,11 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val componentTreeDepsIncrementalBuild = genComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(componentTreeDepsFullBuild)
.isEqualTo(componentTreeDepsIncrementalBuild)

// Check compilation outputs
// * All compiled classes from activity 2 should be deleted
// * Unrelated activities and modules are in isolation and should be unchanged
Expand Down Expand Up @@ -792,6 +822,7 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun deleteModuleSource() {
runFullBuild()
val componentTreeDepsFullBuild = genComponentTreeDeps.readText(Charsets.UTF_8)

srcModule2.delete()

Expand Down Expand Up @@ -828,6 +859,11 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val componentTreeDepsIncrementalBuild = genComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(componentTreeDepsFullBuild)
.isEqualTo(componentTreeDepsIncrementalBuild)

// Check compilation outputs
// * All compiled classes from module 2 should be deleted
// * Unrelated activities and modules are in isolation and should be unchanged
Expand Down Expand Up @@ -939,6 +975,8 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun changeTestSource_addPublicMethod() {
runFullTestBuild()
val test1ComponentTreeDepsFullBuild = genTest1ComponentTreeDeps.readText(Charsets.UTF_8)
val test2ComponentTreeDepsFullBuild = genTest2ComponentTreeDeps.readText(Charsets.UTF_8)

// Change Test 1 source
searchAndReplace(
Expand Down Expand Up @@ -972,6 +1010,15 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val test1ComponentTreeDepsIncrementalBuild = genTest1ComponentTreeDeps.readText(Charsets.UTF_8)
val test2ComponentTreeDepsIncrementalBuild = genTest2ComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(test1ComponentTreeDepsFullBuild)
.isEqualTo(test1ComponentTreeDepsIncrementalBuild)
expect.withMessage("Full build")
.that(test2ComponentTreeDepsFullBuild)
.isEqualTo(test2ComponentTreeDepsIncrementalBuild)

val recompiledClassFiles = if (incapMode == ISOLATING_MODE) {
listOf(
classSrcTest1,
Expand All @@ -996,6 +1043,8 @@ class IncrementalProcessorTest(private val incapMode: String) {
@Test
fun changeTestSource_addPrivateMethod() {
runFullTestBuild()
val test1ComponentTreeDepsFullBuild = genTest1ComponentTreeDeps.readText(Charsets.UTF_8)
val test2ComponentTreeDepsFullBuild = genTest2ComponentTreeDeps.readText(Charsets.UTF_8)

// Change Test 1 source
searchAndReplace(
Expand Down Expand Up @@ -1031,6 +1080,15 @@ class IncrementalProcessorTest(private val incapMode: String) {
}
assertChangedFiles(FileType.JAVA, regeneratedSourceFiles)

val test1ComponentTreeDepsIncrementalBuild = genTest1ComponentTreeDeps.readText(Charsets.UTF_8)
val test2ComponentTreeDepsIncrementalBuild = genTest2ComponentTreeDeps.readText(Charsets.UTF_8)
expect.withMessage("Full build")
.that(test1ComponentTreeDepsFullBuild)
.isEqualTo(test1ComponentTreeDepsIncrementalBuild)
expect.withMessage("Full build")
.that(test2ComponentTreeDepsFullBuild)
.isEqualTo(test2ComponentTreeDepsIncrementalBuild)

val recompiledClassFiles = if (incapMode == ISOLATING_MODE) {
listOf(classSrcTest1)
} else {
Expand Down
28 changes: 14 additions & 14 deletions java/dagger/hilt/processor/internal/root/ir/ComponentTreeDepsIrCreator.kt
View file
Expand Up @@ -21,12 +21,12 @@ import com.squareup.javapoet.ClassName
// Produces ComponentTreeDepsIr for a set of aggregated deps and roots to process.
class ComponentTreeDepsIrCreator private constructor(
private val isSharedTestComponentsEnabled: Boolean,
private val aggregatedRoots: Set<AggregatedRootIr>,
private val defineComponentDeps: Set<DefineComponentClassesIr>,
private val aliasOfDeps: Set<AliasOfPropagatedDataIr>,
private val aggregatedDeps: Set<AggregatedDepsIr>,
private val aggregatedUninstallModulesDeps: Set<AggregatedUninstallModulesIr>,
private val aggregatedEarlyEntryPointDeps: Set<AggregatedEarlyEntryPointIr>,
private val aggregatedRoots: List<AggregatedRootIr>,
private val defineComponentDeps: List<DefineComponentClassesIr>,
private val aliasOfDeps: List<AliasOfPropagatedDataIr>,
private val aggregatedDeps: List<AggregatedDepsIr>,
private val aggregatedUninstallModulesDeps: List<AggregatedUninstallModulesIr>,
private val aggregatedEarlyEntryPointDeps: List<AggregatedEarlyEntryPointIr>,
) {
private fun prodComponents(): Set<ComponentTreeDepsIr> {
// There should only be one prod root in a given build.
Expand Down Expand Up @@ -104,7 +104,7 @@ class ComponentTreeDepsIrCreator private constructor(
}
}

private fun rootsUsingSharedComponent(roots: Set<AggregatedRootIr>): Set<ClassName> {
private fun rootsUsingSharedComponent(roots: List<AggregatedRootIr>): Set<ClassName> {
if (!isSharedTestComponentsEnabled) {
return emptySet()
}
Expand All @@ -120,7 +120,7 @@ class ComponentTreeDepsIrCreator private constructor(
}

private fun aggregatedDepsByRoot(
aggregatedRoots: Set<AggregatedRootIr>,
aggregatedRoots: List<AggregatedRootIr>,
rootsUsingSharedComponent: Set<ClassName>,
hasEarlyEntryPoints: Boolean
): Map<ClassName, Set<ClassName>> {
Expand Down Expand Up @@ -236,12 +236,12 @@ class ComponentTreeDepsIrCreator private constructor(
aggregatedEarlyEntryPointDeps: Set<AggregatedEarlyEntryPointIr>,
) = ComponentTreeDepsIrCreator(
isSharedTestComponentsEnabled,
aggregatedRoots,
defineComponentDeps,
aliasOfDeps,
aggregatedDeps,
aggregatedUninstallModulesDeps,
aggregatedEarlyEntryPointDeps
aggregatedRoots.toList().sortedBy { it.fqName.canonicalName() },
defineComponentDeps.toList().sortedBy { it.fqName.canonicalName() },
aliasOfDeps.toList().sortedBy { it.fqName.canonicalName() },
aggregatedDeps.toList().sortedBy { it.fqName.canonicalName() },
aggregatedUninstallModulesDeps.toList().sortedBy { it.fqName.canonicalName() },
aggregatedEarlyEntryPointDeps.toList().sortedBy { it.fqName.canonicalName() }
).let { producer ->
if (isTest) {
producer.testComponents()
Expand Down
8 changes: 4 additions & 4 deletions util/run-local-gradle-tests.sh
View file
Expand Up @@ -4,11 +4,11 @@ set -ex

readonly GRADLE_PROJECTS=(
"java/dagger/hilt/android/plugin"
"javatests/artifacts/dagger/simple"
"javatests/artifacts/dagger/simpleKotlin"
# "javatests/artifacts/dagger/simple"
# "javatests/artifacts/dagger/simpleKotlin"
)
for project in "${GRADLE_PROJECTS[@]}"; do
echo "Running gradle tests for $project"
./$project/gradlew -p $project build --no-daemon --stacktrace
./$project/gradlew -p $project test --continue --no-daemon --stacktrace
./$project/gradlew -p $project build --no-daemon --stacktrace --debug
./$project/gradlew -p $project test --continue --no-daemon --stacktrace --debug
done

0 comments on commit 6769a1c

Please sign in to comment.