Skip to content

Commit 1ac66b7

Browse files
abstrattabstratt
committed
Deduplicate configuration cache report problems
Issue: #36046
1 parent 9c0c594 commit 1ac66b7

File tree

12 files changed

+381
-82
lines changed

12 files changed

+381
-82
lines changed

‎platforms/core-configuration/configuration-cache/src/integTest/groovy/org/gradle/internal/cc/impl/ConfigurationCacheGracefulDegradationIntegrationTest.groovy‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ class ConfigurationCacheGracefulDegradationIntegrationTest extends AbstractConfi
200200
configurationCache.assertNoConfigurationCache()
201201

202202
and:
203+
// feature degradation is reported as a report problem, but it is silently suppressed from the console
203204
problems.assertResultConsoleSummaryHasNoProblems(result)
204205
problems.htmlReport(result).assertContents {
205206
totalProblemsCount = 1
@@ -239,7 +240,8 @@ class ConfigurationCacheGracefulDegradationIntegrationTest extends AbstractConfi
239240
withProblem("Build file 'build.gradle': line 17: invocation of 'Task.project' at execution time is unsupported with the configuration cache.")
240241
}
241242
problems.htmlReport(result).assertContents {
242-
totalProblemsCount = 2
243+
// gracefully degraded task appears in the report
244+
withProblem(INVOCATION_OF_TASK_PROJECT_AT_EXECUTION_TIME)
243245
withProblem(INVOCATION_OF_TASK_PROJECT_AT_EXECUTION_TIME)
244246
withIncompatibleTask(":foo", "Project access.")
245247
}

‎platforms/core-configuration/configuration-cache/src/integTest/groovy/org/gradle/internal/cc/impl/ConfigurationCacheProblemReportingIntegrationTest.groovy‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ class ConfigurationCacheProblemReportingIntegrationTest extends AbstractConfigur
636636
withProblem("Task `:problems` of type `BrokenTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.")
637637
withProblem("Task `:problems` of type `BrokenTask`: cannot serialize object of type 'org.gradle.api.internal.project.DefaultProject', a subtype of 'org.gradle.api.Project', as these are not supported with the configuration cache.")
638638
totalProblemsCount = 6
639-
problemsWithStackTraceCount = 2
639+
problemsWithStackTraceCount = 1
640640
}
641641

642642
when:
@@ -653,7 +653,7 @@ class ConfigurationCacheProblemReportingIntegrationTest extends AbstractConfigur
653653
withProblem("Task `:problems` of type `BrokenTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.")
654654
withProblem("Task `:problems` of type `BrokenTask`: cannot serialize object of type 'org.gradle.api.internal.project.DefaultProject', a subtype of 'org.gradle.api.Project', as these are not supported with the configuration cache.")
655655
totalProblemsCount = 6
656-
problemsWithStackTraceCount = 2
656+
problemsWithStackTraceCount = 1
657657
}
658658
failure.assertHasFailures(1)
659659

@@ -671,7 +671,7 @@ class ConfigurationCacheProblemReportingIntegrationTest extends AbstractConfigur
671671
withProblem("Task `:problems` of type `BrokenTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.")
672672
withProblem("Task `:problems` of type `BrokenTask`: cannot serialize object of type 'org.gradle.api.internal.project.DefaultProject', a subtype of 'org.gradle.api.Project', as these are not supported with the configuration cache.")
673673
totalProblemsCount = 6
674-
problemsWithStackTraceCount = 2
674+
problemsWithStackTraceCount = 1
675675
}
676676

677677
when:
@@ -686,7 +686,7 @@ class ConfigurationCacheProblemReportingIntegrationTest extends AbstractConfigur
686686
withProblem("Task `:moreProblems` of type `BrokenTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.")
687687
withProblem("Task `:problems` of type `BrokenTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.")
688688
totalProblemsCount = 4
689-
problemsWithStackTraceCount = 2
689+
problemsWithStackTraceCount = 1
690690
}
691691
}
692692

‎platforms/core-configuration/configuration-cache/src/main/kotlin/org/gradle/internal/cc/impl/problems/ConfigurationCacheProblems.kt‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,14 +239,15 @@ class ConfigurationCacheProblems(
239239

240240
private
241241
fun reportDegradingFeature(feature: String) {
242+
// we report degrading features as problems
242243
val problem = problemFactory
243244
.problem {
244245
// for now, we don't expect interesting information from degrading features, so only the feature name is displayed
245246
text("Feature '$feature' is incompatible with the configuration cache.")
246247
}
247248
.build()
249+
summarizer.onIncompatibleFeature(problem)
248250
report.onProblem(problem)
249-
summarizer.onIncompatibleFeature()
250251
}
251252

252253
override fun onProblem(problem: PropertyProblem) {
@@ -344,7 +345,7 @@ class ConfigurationCacheProblems(
344345
addNotReportedDegradingTasks()
345346
addDegradingFeatures()
346347
val summary = summarizer.get()
347-
val hasNoProblemsForConsole = summary.reportableProblemCount == 0
348+
val hasNoProblemsForConsole = summary.consoleProblemCount == 0
348349
val outputDirectory = outputDirectoryFor(reportDir)
349350
val details = detailsFor(summary)
350351
val htmlReportFile = report.writeReportFileTo(outputDirectory, ProblemReportDetailsJsonSource(details))
@@ -421,12 +422,12 @@ class ConfigurationCacheProblems(
421422
@Suppress("CyclomaticComplexMethod")
422423
override fun beforeComplete(failure: Throwable?) {
423424
val summary = summarizer.get()
424-
val reportableProblemCount = summary.reportableProblemCount
425+
val consoleProblemCount = summary.consoleProblemCount
425426
val deferredProblemCount = summary.deferredProblemCount
426-
val hasProblems = reportableProblemCount > 0
427+
val hasProblems = consoleProblemCount > 0
427428
val discardStateDueToProblems = discardStateDueToProblems(summary)
428429
val hasTooManyProblems = hasTooManyProblems(summary)
429-
val problemCountString = reportableProblemCount.counter("problem")
430+
val problemCountString = consoleProblemCount.counter("problem")
430431
val reusedProjectsString = reusedProjects.counter("project")
431432
val updatedProjectsString = updatedProjects.counter("project")
432433
when {
@@ -487,7 +488,7 @@ class ConfigurationCacheProblems(
487488
private
488489
fun discardStateDueToProblems(summary: Summary) =
489490
incompatibleTasks.isNotEmpty() || shouldDegradeGracefully() ||
490-
summary.reportableProblemCount > 0 && !isWarningMode
491+
summary.consoleProblemCount > 0 && !isWarningMode
491492

492493
private
493494
fun hasTooManyProblems(summary: Summary) =

‎platforms/core-configuration/configuration-cache/src/main/kotlin/org/gradle/internal/cc/impl/problems/ConfigurationCacheProblemsSummary.kt‎

Lines changed: 88 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.gradle.internal.cc.impl.problems
1818

19+
import com.google.common.annotations.VisibleForTesting
1920
import com.google.common.collect.Comparators
2021
import com.google.common.collect.ImmutableList
2122
import com.google.common.collect.ImmutableMap
@@ -38,11 +39,16 @@ private
3839
const val MAX_PROBLEM_EXCEPTIONS = 5
3940

4041
/**
42+
* Collects problems and produces a summary upon request.
43+
*
4144
* This class is thread-safe.
4245
*/
4346
internal
4447
class ConfigurationCacheProblemsSummary(
4548

49+
/**
50+
* The maximum number of unique problems that should be collected.
51+
*/
4652
private
4753
val maxCollectedProblems: Int = 4096
4854

@@ -94,23 +100,26 @@ class ConfigurationCacheProblemsSummary(
94100

95101
fun get(): Summary = lock.withLock {
96102
Summary(
97-
totalProblemCount,
98-
deferredProblemCount,
99-
suppressedSilentlyProblemCount,
100-
ImmutableMap.copyOf(problemCauses),
101-
ImmutableList.copyOf(originalProblemExceptions),
102-
overflowed,
103-
maxCollectedProblems,
104-
incompatibleTasksCount,
105-
incompatibleFeatureCount
103+
totalProblemCount = totalProblemCount,
104+
uniqueProblemCount = problemCauses.size,
105+
deferredProblemCount = deferredProblemCount,
106+
consoleProblemCount = totalProblemCount - suppressedSilentlyProblemCount,
107+
consoleProblemCauses = ImmutableMap.copyOf(problemCauses.filterValues { it != ProblemSeverity.SuppressedSilently }),
108+
originalProblemExceptions = ImmutableList.copyOf(originalProblemExceptions),
109+
overflowed = overflowed,
110+
maxCollectedProblems = maxCollectedProblems,
111+
incompatibleTasksCount = incompatibleTasksCount,
112+
incompatibleFeatureCount = incompatibleFeatureCount
106113
)
107114
}
108115

109116
/**
110-
* Returns`true` if the problem was accepted, `false` if it was rejected because the maximum number of problems was reached.
117+
* Returns`true` if the problem was accepted, `false` if it was rejected because the maximum number of unique problems was reached,
118+
* or it was not unique.
111119
*/
112120
fun onProblem(problem: PropertyProblem, severity: ProblemSeverity): Boolean {
113121
lock.withLock {
122+
// count problems regardless of uniqueness / overflowing
114123
totalProblemCount += 1
115124
when (severity) {
116125
ProblemSeverity.Deferred -> deferredProblemCount += 1
@@ -119,17 +128,21 @@ class ConfigurationCacheProblemsSummary(
119128
ProblemSeverity.Interrupting -> {}
120129
}
121130
if (overflowed) {
131+
// we already overflowed during a previous problem
122132
return false
123133
}
124-
if (totalProblemCount > maxCollectedProblems) {
134+
if (problemCauses.size == maxCollectedProblems) {
135+
// we are overflowing now
125136
overflowed = true
126137
return false
127138
}
128-
if (severity != ProblemSeverity.SuppressedSilently) {
129-
val isNewCause = recordProblemCause(problem, severity)
130-
if (isNewCause && severity != ProblemSeverity.Interrupting) {
131-
collectOriginalException(problem)
132-
}
139+
val isNewCause = recordProblemCause(problem, severity)
140+
if (!isNewCause) {
141+
// reject non-unique problems
142+
return false
143+
}
144+
if (severity != ProblemSeverity.Interrupting) {
145+
collectOriginalException(problem)
133146
}
134147
return true
135148
}
@@ -141,8 +154,9 @@ class ConfigurationCacheProblemsSummary(
141154
}
142155
}
143156

144-
fun onIncompatibleFeature() {
157+
fun onIncompatibleFeature(problem: PropertyProblem) {
145158
lock.withLock {
159+
onProblem(problem, ProblemSeverity.SuppressedSilently)
146160
incompatibleFeatureCount += 1
147161
}
148162
}
@@ -157,16 +171,24 @@ class ConfigurationCacheProblemsSummary(
157171
}
158172

159173
/**
160-
* Returns true if problems with the same cause have not been seen before.
174+
* Collects the cause for the problem, if not seen before.
175+
*
176+
* Returns whether problem was accepted.
161177
*/
162178
private
163179
fun recordProblemCause(problem: PropertyProblem, severity: ProblemSeverity): Boolean {
164-
val cause = ProblemCause.of(problem)
165-
val isNew = !problemCauses.containsKey(cause)
166-
problemCauses.merge(cause, severity) { old, new ->
167-
if (severityComparator.compare(old, new) < 0) old else new
180+
val newCause = ProblemCause.of(problem)
181+
var problemAccepted = true
182+
problemCauses.merge(newCause, severity) { existingSeverity, newSeverity ->
183+
val newSeverityIsHigher = severityComparator.compare(existingSeverity, newSeverity) > 0
184+
if (newSeverityIsHigher) {
185+
newSeverity
186+
} else {
187+
problemAccepted = false
188+
existingSeverity
189+
}
168190
}
169-
return isNew
191+
return problemAccepted
170192
}
171193
}
172194

@@ -178,23 +200,31 @@ class Summary(
178200
*/
179201
val totalProblemCount: Int,
180202

203+
val consoleProblemCount: Int,
204+
181205
/**
182206
* Number of [deferred][ProblemSeverity.Deferred] failures.
183207
*/
184208
val deferredProblemCount: Int,
185209

186210
/**
187-
* Number of problems which shouldn't be reported in the console.
211+
* Problems that should appear in the console summary.
212+
*
213+
* This should exclude problems with severity [ProblemSeverity.SuppressedSilently], which are still
214+
* accounted for in [totalProblemCount] and shown in the HTML report, but intentionally omitted
215+
* from the console to keep output noise low during graceful degradation.
188216
*/
189217
private
190-
val suppressedSilentlyProblemCount: Int,
218+
val consoleProblemCauses: Map<ProblemCause, ProblemSeverity>,
191219

192-
private
193-
val reportableProblemCauses: Map<ProblemCause, ProblemSeverity>,
220+
/**
221+
* Count of problems that should appear in the HTML report.
222+
*/
223+
val uniqueProblemCount: Int,
194224

195225
val originalProblemExceptions: List<Throwable>,
196226

197-
private
227+
internal
198228
val overflowed: Boolean,
199229

200230
private
@@ -211,22 +241,27 @@ class Summary(
211241
private
212242
val incompatibleFeatureCount: Int
213243
) {
214-
val reportableProblemCount: Int
215-
get() = totalProblemCount - suppressedSilentlyProblemCount
216-
217-
val reportableProblemCauseCount: Int
218-
get() = reportableProblemCauses.size
219244

245+
/**
246+
* Builds the console feedback string for configuration cache problems.
247+
*
248+
* Rules:
249+
* - If there are reportable problems, print a summary header and a curated list of up to [MAX_CONSOLE_PROBLEMS].
250+
* - If there are no reportable problems but there are incompatible tasks or features (graceful degradation),
251+
* do not list problems; emit a short notice and always print the report link.
252+
* - In all cases where an HTML report exists, append a link to it.
253+
*/
220254
fun textForConsole(cacheActionText: String, htmlReportFile: File? = null): String {
221255
val documentationRegistry = DocumentationRegistry()
222256
return StringBuilder().apply {
223257
// When build degrades gracefully, we keep the console output minimal but still want to see the report link
224-
val hasReportableProblems = reportableProblemCount > 0
225-
if (hasReportableProblems) {
258+
val topConsoleProblems = topProblemsForConsole().iterator()
259+
val hasConsoleProblems = topConsoleProblems.hasNext()
260+
if (hasConsoleProblems) {
226261
appendLine()
227-
appendSummaryHeader(cacheActionText, reportableProblemCount)
262+
appendSummaryHeader(cacheActionText)
228263
appendLine()
229-
topProblemsForConsole().forEach { problem ->
264+
topConsoleProblems.forEach { problem ->
230265
append("- ")
231266
append(problem.userCodeLocation.capitalized())
232267
append(": ")
@@ -235,15 +270,15 @@ class Summary(
235270
appendLine(" See ${documentationRegistry.getDocumentationFor(it.page, it.anchor)}")
236271
}
237272
}
238-
if (reportableProblemCauseCount > MAX_CONSOLE_PROBLEMS) {
239-
appendLine("plus ${reportableProblemCauseCount - MAX_CONSOLE_PROBLEMS} more problems. Please see the report for details.")
273+
if (uniqueProblemCount > MAX_CONSOLE_PROBLEMS) {
274+
appendLine("plus ${uniqueProblemCount - MAX_CONSOLE_PROBLEMS} more problems. Please see the report for details.")
240275
}
241276
}
242277
val hasIncompatibleTasks = incompatibleTasksCount > 0
243278
val hasIncompatibleFeatures = incompatibleFeatureCount > 0
244279
htmlReportFile?.let {
245280
appendLine()
246-
if ((hasIncompatibleTasks || hasIncompatibleFeatures) && !hasReportableProblems) {
281+
if ((hasIncompatibleTasks || hasIncompatibleFeatures) && !hasConsoleProblems) {
247282
// Some tests parse this line, you may need to change them if you change the message.
248283
append("Some tasks or features in this build are not compatible with the configuration cache.")
249284
appendLine()
@@ -255,31 +290,31 @@ class Summary(
255290

256291
private
257292
fun topProblemsForConsole(): Sequence<ProblemCause> =
258-
reportableProblemCauses.entries.stream()
293+
consoleProblemCauses.entries.stream()
259294
.collect(Comparators.least(MAX_CONSOLE_PROBLEMS, consoleComparatorForProblemCauseWithSeverity()))
260295
.asSequence()
261296
.map { it.key }
262297

263298
private
264299
fun StringBuilder.appendSummaryHeader(
265-
cacheAction: String,
266-
reportableProblemCount: Int
300+
cacheAction: String
267301
) {
268302
// Some tests parse this header.
269-
append(reportableProblemCount)
270-
append(if (reportableProblemCount == 1) " problem was found " else " problems were found ")
303+
append(consoleProblemCount)
304+
append(if (consoleProblemCount == 1) " problem was found " else " problems were found ")
271305
append(cacheAction)
272306
append(" the configuration cache")
273307
if (overflowed) {
274308
append(", only the first ")
275309
append(maxCollectedProblems)
276310
append(" were considered")
277311
}
278-
if (reportableProblemCount != reportableProblemCauseCount) {
312+
val consoleUniqueProblemCount = consoleProblemCauses.size
313+
if (consoleUniqueProblemCount != consoleProblemCount) {
279314
append(", ")
280-
append(reportableProblemCauseCount)
315+
append(consoleUniqueProblemCount)
281316
append(" of which ")
282-
append(if (reportableProblemCauseCount == 1) "seems unique" else "seem unique")
317+
append(if (consoleUniqueProblemCount == 1) "seems unique" else "seem unique")
283318
}
284319
append(".")
285320
}
@@ -291,6 +326,11 @@ class Summary(
291326
private
292327
fun clickableUrlFor(file: File) =
293328
ConsoleRenderer().asClickableFileUrl(file)
329+
330+
@VisibleForTesting
331+
internal
332+
fun severityFor(of: ProblemCause): ProblemSeverity? =
333+
this.consoleProblemCauses[of]
294334
}
295335

296336

0 commit comments

Comments
 (0)