jamesfredley commented on code in PR #16025:
URL: https://github.com/apache/grails-core/pull/16025#discussion_r3677414855
##########
build-logic/plugins/build.gradle:
##########
@@ -40,6 +40,7 @@ dependencies {
implementation
"org.cyclonedx.bom:org.cyclonedx.bom.gradle.plugin:${gradleProperties.gradleCycloneDxPluginVersion}"
implementation
"com.github.spotbugs.snom:spotbugs-gradle-plugin:${gradleProperties.spotbugsPluginVersion}"
implementation
"org.sonatype.gradle.plugins:scan-gradle-plugin:${gradleProperties.sonatypeScanPluginVersion}"
+ implementation 'org.yaml:snakeyaml:2.4'
Review Comment:
Fixed. `snakeyamlVersion=2.6` now lives in the build-tooling block of the
root `gradle.properties` alongside `pmdVersion`, `checkstyleVersion`, and
friends, and `build-logic/plugins/build.gradle` consumes it as
`"org.yaml:snakeyaml:${gradleProperties.snakeyamlVersion}"`, matching how every
other build-tooling dependency in that file is declared. That also brings it up
to the 2.6 you identified instead of 2.4.
##########
gradle.properties:
##########
@@ -72,6 +72,9 @@ pmdVersion=7.25.0
spotbugsPluginVersion=6.4.8
sonatypeScanPluginVersion=3.1.6
+# PMD is enforced only for projects with a clean baseline. Add project paths
after clearing debt.
+grails.code-analysis.enabled.pmd.projects=:grails-data-graphql-core,:grails-data-mongodb-spring-data,:grails-datasource,:grails-testing-support-core
Review Comment:
Agreed, and removed. The hardcoded list is gone from `gradle.properties`
entirely.
PMD and SpotBugs are now opted into per project, in the module that owns the
decision:
```groovy
grailsCodeAnalysis {
// PMD baseline is clean for this module; keep it blocking.
pmdEnabled = true
}
```
`GrailsCodeAnalysisExtension` gained lazy `pmdEnabled` / `spotbugsEnabled`
properties, and `GrailsCodeAnalysisPlugin` defers `configurePmd` /
`configureSpotbugs` to `afterEvaluate` so a module's own `build.gradle` can set
them. The four clean modules carry the flag themselves.
The root aggregator no longer needs a list either: it derives the enabled
set from `pluginManager.withPlugin('pmd')` /
`withPlugin('com.github.spotbugs')` across `allprojects`, so adding a module is
a one-line change in that module and nothing central has to be edited. The
`-Pgrails.code-analysis.enabled.pmd` and `-P...pmd.projects=:a,:b` properties
are kept purely as overrides for baseline and CI runs, and the unknown-path
validation now applies only to paths passed through that override.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -84,82 +86,217 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
def violationsDir =
project.layout.buildDirectory.dir('reports/violations')
- def styleXmlDir =
project.layout.buildDirectory.dir('reports/code-style')
- def analysisXmlDir =
project.layout.buildDirectory.dir('reports/code-analysis')
-
- def styleTask = registerStyleAggregation(project, styleXmlDir,
violationsDir)
- def analysisTask = registerAnalysisAggregation(project,
analysisXmlDir, violationsDir)
+ TaskProvider<RepositoryConventionsTask> repositoryConventionsTask =
project.file(CANONICAL_ROOT_MARKER).isDirectory() ?
+ registerRepositoryConventions(project, violationsDir) : null
+ def styleTask = registerStyleAggregation(project, violationsDir)
+ def analysisTask = registerAnalysisAggregation(project, violationsDir)
registerJacocoAggregation(project, violationsDir)
project.tasks.register('aggregateViolations') { Task task ->
task.group = 'verification'
task.description = 'Aggregates all violation reports (style +
analysis) into build/reports/violations/'
task.dependsOn(styleTask, analysisTask)
+ if (repositoryConventionsTask) {
+ task.dependsOn(repositoryConventionsTask)
+ }
+ }
+ }
+
+ private static TaskProvider<RepositoryConventionsTask>
registerRepositoryConventions(Project root, Provider<Directory> violationsDir) {
+ root.tasks.register('validateRepositoryConventions',
RepositoryConventionsTask) { RepositoryConventionsTask task ->
+ task.group = 'verification'
+ task.description = 'Validates repository conventions and writes
build/reports/violations/REPOSITORY_CONVENTIONS.md'
+ task.repositoryDirectory.set(root.layout.projectDirectory)
+ task.conventionSources.from(
+ root.file('AGENTS.md'),
+ root.fileTree('.agents/skills') { include '*/SKILL.md' },
+ root.fileTree('.github/workflows') { include '**/*.yml',
'**/*.yaml' },
+ root.fileTree('.') {
+ include '**/action.yml', '**/action.yaml'
+ exclude '**/build/**', '**/generated/**',
'**/.gradle/**', '**/.git/**', '**/.hg/**', '**/.svn/**'
+ },
+ root.fileTree('.') {
+ include '**/messages*.properties'
+ exclude '**/build/**', '**/generated/**'
+ }
+ )
+ task.reportFile.set(violationsDir.map {
it.file('REPOSITORY_CONVENTIONS.md') })
+ task.outputs.upToDateWhen { false }
+ task.dependsOn(root.tasks.matching { Task candidate ->
candidate.name == 'rat' })
}
}
- private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> styleXmlDir, Provider<Directory> violationsDir) {
- // Wire property flags as Providers — values are resolved at task
execution time, not at apply() time,
- // and Providers are configuration-cache safe to capture in task
actions
+ private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> violationsDir) {
+ Directory rootDirectory = root.layout.projectDirectory
def checkStyleTests = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.TEST_STYLING_PROPERTY)
+ def ignoreFailures = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.IGNORE_FAILURES_PROPERTY)
def codenarcEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CODENARC_ENABLED_PROPERTY, true)
def checkstyleEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CHECKSTYLE_ENABLED_PROPERTY, true)
+ def codenarcMarkers = root.files()
+ def checkstyleMarkers = root.files()
+ def codenarcReports = root.files()
+ def checkstyleReports = root.files()
+ def codenarcMarkdown =
root.layout.buildDirectory.file('reports/violations/CODENARC_VIOLATIONS.md')
+ def checkstyleMarkdown =
root.layout.buildDirectory.file('reports/violations/CHECKSTYLE_VIOLATIONS.md')
+ def cleanupTask = root.tasks.register('cleanAggregateStyleReports') {
+ it.doLast {
+ deleteReports(codenarcMarkers.files, codenarcReports.files)
+ deleteReports(checkstyleMarkers.files, checkstyleReports.files)
+ }
+ }
- def aggregateTask = root.tasks.register('aggregateStyleViolations') {
+ def writerTask = root.tasks.register('writeStyleViolations') {
it.group = 'verification'
- it.description = 'Aggregates CodeNarc and Checkstyle violation
reports into build/reports/violations/'
-
it.outputs.file(root.file('build/reports/violations/CODENARC_VIOLATIONS.md'))
-
it.outputs.file(root.file('build/reports/violations/CHECKSTYLE_VIOLATIONS.md'))
+ it.description = 'Writes CodeNarc and Checkstyle violation reports
into build/reports/violations/'
+ it.inputs.files(codenarcMarkers).optional()
+ it.inputs.files(checkstyleMarkers).optional()
+ it.inputs.property('ignoreFailures', ignoreFailures)
+ it.outputs.file(codenarcMarkdown)
+ it.outputs.file(checkstyleMarkdown)
+ it.outputs.upToDateWhen { false }
+ it.doFirst {
+ codenarcMarkdown.get().asFile.delete()
+ checkstyleMarkdown.get().asFile.delete()
+ }
it.doLast {
- parseStyleViolations(styleXmlDir.get(), violationsDir.get(),
- checkStyleTests.get(), codenarcEnabled.get(),
checkstyleEnabled.get())
+ parseStyleViolations(codenarcMarkers.files,
checkstyleMarkers.files, rootDirectory, violationsDir.get(),
+ checkStyleTests.get(), codenarcEnabled.get(),
+ checkstyleEnabled.get(), ignoreFailures.get())
}
}
- root.subprojects { Project sub ->
- sub.pluginManager.withPlugin('codenarc') {
- aggregateTask.configure {
- it.dependsOn(sub.tasks.withType(CodeNarc))
- }
+ def aggregateTask = root.tasks.register('aggregateStyleViolations') {
+ it.group = 'verification'
+ it.description = 'Aggregates CodeNarc and Checkstyle violations
into build/reports/violations/'
+ it.dependsOn(writerTask)
+ }
+ if (root.tasks.names.contains('validateRepositoryConventions')) {
+ aggregateTask.configure {
it.dependsOn(root.tasks.named('validateRepositoryConventions')) }
+ }
+ def finalizeTask = root.tasks.register('finalizeStyleViolations') {
+ it.group = 'verification'
+ it.dependsOn(writerTask)
+ }
+ root.allprojects { Project sub ->
+ sub.tasks.withType(CodeNarc).all { CodeNarc codeNarcTask ->
Review Comment:
Fixed. All four blocks use `configureEach`, and the aggregate, writer, and
cleanup relationships are now collection-based:
```groovy
def pmdTasks = sub.tasks.withType(Pmd)
aggregateTask.configure { it.dependsOn(pmdTasks); it.dependsOn(cleanupTask) }
writerTask.configure { it.mustRunAfter(pmdTasks) }
pmdTasks.configureEach { Pmd pmdTask -> ... }
```
Nothing is realized at configuration time any more. Thanks for the note
about never-realized markers, that is what made the collection form safe to
adopt without extra bookkeeping.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -84,82 +86,217 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
def violationsDir =
project.layout.buildDirectory.dir('reports/violations')
- def styleXmlDir =
project.layout.buildDirectory.dir('reports/code-style')
- def analysisXmlDir =
project.layout.buildDirectory.dir('reports/code-analysis')
-
- def styleTask = registerStyleAggregation(project, styleXmlDir,
violationsDir)
- def analysisTask = registerAnalysisAggregation(project,
analysisXmlDir, violationsDir)
+ TaskProvider<RepositoryConventionsTask> repositoryConventionsTask =
project.file(CANONICAL_ROOT_MARKER).isDirectory() ?
+ registerRepositoryConventions(project, violationsDir) : null
+ def styleTask = registerStyleAggregation(project, violationsDir)
+ def analysisTask = registerAnalysisAggregation(project, violationsDir)
registerJacocoAggregation(project, violationsDir)
project.tasks.register('aggregateViolations') { Task task ->
task.group = 'verification'
task.description = 'Aggregates all violation reports (style +
analysis) into build/reports/violations/'
task.dependsOn(styleTask, analysisTask)
+ if (repositoryConventionsTask) {
+ task.dependsOn(repositoryConventionsTask)
+ }
+ }
+ }
+
+ private static TaskProvider<RepositoryConventionsTask>
registerRepositoryConventions(Project root, Provider<Directory> violationsDir) {
+ root.tasks.register('validateRepositoryConventions',
RepositoryConventionsTask) { RepositoryConventionsTask task ->
+ task.group = 'verification'
+ task.description = 'Validates repository conventions and writes
build/reports/violations/REPOSITORY_CONVENTIONS.md'
+ task.repositoryDirectory.set(root.layout.projectDirectory)
+ task.conventionSources.from(
+ root.file('AGENTS.md'),
+ root.fileTree('.agents/skills') { include '*/SKILL.md' },
+ root.fileTree('.github/workflows') { include '**/*.yml',
'**/*.yaml' },
+ root.fileTree('.') {
+ include '**/action.yml', '**/action.yaml'
+ exclude '**/build/**', '**/generated/**',
'**/.gradle/**', '**/.git/**', '**/.hg/**', '**/.svn/**'
+ },
+ root.fileTree('.') {
+ include '**/messages*.properties'
+ exclude '**/build/**', '**/generated/**'
+ }
+ )
+ task.reportFile.set(violationsDir.map {
it.file('REPOSITORY_CONVENTIONS.md') })
+ task.outputs.upToDateWhen { false }
+ task.dependsOn(root.tasks.matching { Task candidate ->
candidate.name == 'rat' })
}
}
- private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> styleXmlDir, Provider<Directory> violationsDir) {
- // Wire property flags as Providers — values are resolved at task
execution time, not at apply() time,
- // and Providers are configuration-cache safe to capture in task
actions
+ private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> violationsDir) {
+ Directory rootDirectory = root.layout.projectDirectory
def checkStyleTests = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.TEST_STYLING_PROPERTY)
+ def ignoreFailures = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.IGNORE_FAILURES_PROPERTY)
def codenarcEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CODENARC_ENABLED_PROPERTY, true)
def checkstyleEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CHECKSTYLE_ENABLED_PROPERTY, true)
+ def codenarcMarkers = root.files()
+ def checkstyleMarkers = root.files()
+ def codenarcReports = root.files()
+ def checkstyleReports = root.files()
+ def codenarcMarkdown =
root.layout.buildDirectory.file('reports/violations/CODENARC_VIOLATIONS.md')
+ def checkstyleMarkdown =
root.layout.buildDirectory.file('reports/violations/CHECKSTYLE_VIOLATIONS.md')
+ def cleanupTask = root.tasks.register('cleanAggregateStyleReports') {
Review Comment:
Fixed. Quality tasks no longer `dependsOn` the cleanup task, so a
single-module `build` no longer wipes the other 60+ modules and no longer
defeats each task's own up-to-date check. The cleanup task is now attached to
the aggregate lane only: the aggregate task depends on it and quality tasks
merely `mustRunAfter` it, which keeps stale markers from renamed or removed
tasks purged when the aggregate lane actually runs.
You were also right that the report deletion was redundant.
`GradleUtils.configureReportMarker` already deletes each task's own report in
`doFirst`, so the cleanup task now only removes markers, and the
`codenarcReports` / `checkstyleReports` / `pmdReports` / `spotbugsReports`
collections that existed solely to feed that deletion are gone.
One follow-on worth flagging, since it is the other half of this thread:
with cleanup no longer a dependency of every analyzer task, a direct
single-analyzer run would have left earlier runs' markers in place and folded
them into the regenerated report. That is fixed in the thread on the finalizer
above, by removing the analyzer finalizers entirely so only the aggregate lane
writes the Markdown. Cleanup stays scoped to that lane, which is where it
belongs.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -84,82 +86,217 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
def violationsDir =
project.layout.buildDirectory.dir('reports/violations')
- def styleXmlDir =
project.layout.buildDirectory.dir('reports/code-style')
- def analysisXmlDir =
project.layout.buildDirectory.dir('reports/code-analysis')
-
- def styleTask = registerStyleAggregation(project, styleXmlDir,
violationsDir)
- def analysisTask = registerAnalysisAggregation(project,
analysisXmlDir, violationsDir)
+ TaskProvider<RepositoryConventionsTask> repositoryConventionsTask =
project.file(CANONICAL_ROOT_MARKER).isDirectory() ?
+ registerRepositoryConventions(project, violationsDir) : null
+ def styleTask = registerStyleAggregation(project, violationsDir)
+ def analysisTask = registerAnalysisAggregation(project, violationsDir)
registerJacocoAggregation(project, violationsDir)
project.tasks.register('aggregateViolations') { Task task ->
task.group = 'verification'
task.description = 'Aggregates all violation reports (style +
analysis) into build/reports/violations/'
task.dependsOn(styleTask, analysisTask)
+ if (repositoryConventionsTask) {
+ task.dependsOn(repositoryConventionsTask)
+ }
+ }
+ }
+
+ private static TaskProvider<RepositoryConventionsTask>
registerRepositoryConventions(Project root, Provider<Directory> violationsDir) {
+ root.tasks.register('validateRepositoryConventions',
RepositoryConventionsTask) { RepositoryConventionsTask task ->
+ task.group = 'verification'
+ task.description = 'Validates repository conventions and writes
build/reports/violations/REPOSITORY_CONVENTIONS.md'
+ task.repositoryDirectory.set(root.layout.projectDirectory)
+ task.conventionSources.from(
+ root.file('AGENTS.md'),
+ root.fileTree('.agents/skills') { include '*/SKILL.md' },
+ root.fileTree('.github/workflows') { include '**/*.yml',
'**/*.yaml' },
+ root.fileTree('.') {
+ include '**/action.yml', '**/action.yaml'
+ exclude '**/build/**', '**/generated/**',
'**/.gradle/**', '**/.git/**', '**/.hg/**', '**/.svn/**'
+ },
+ root.fileTree('.') {
+ include '**/messages*.properties'
+ exclude '**/build/**', '**/generated/**'
+ }
+ )
+ task.reportFile.set(violationsDir.map {
it.file('REPOSITORY_CONVENTIONS.md') })
+ task.outputs.upToDateWhen { false }
+ task.dependsOn(root.tasks.matching { Task candidate ->
candidate.name == 'rat' })
}
}
- private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> styleXmlDir, Provider<Directory> violationsDir) {
- // Wire property flags as Providers — values are resolved at task
execution time, not at apply() time,
- // and Providers are configuration-cache safe to capture in task
actions
+ private static TaskProvider<Task> registerStyleAggregation(Project root,
Provider<Directory> violationsDir) {
+ Directory rootDirectory = root.layout.projectDirectory
def checkStyleTests = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.TEST_STYLING_PROPERTY)
+ def ignoreFailures = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.IGNORE_FAILURES_PROPERTY)
def codenarcEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CODENARC_ENABLED_PROPERTY, true)
def checkstyleEnabled = GradleUtils.booleanProvider(root,
GrailsCodeStylePlugin.CHECKSTYLE_ENABLED_PROPERTY, true)
+ def codenarcMarkers = root.files()
+ def checkstyleMarkers = root.files()
+ def codenarcReports = root.files()
+ def checkstyleReports = root.files()
+ def codenarcMarkdown =
root.layout.buildDirectory.file('reports/violations/CODENARC_VIOLATIONS.md')
+ def checkstyleMarkdown =
root.layout.buildDirectory.file('reports/violations/CHECKSTYLE_VIOLATIONS.md')
+ def cleanupTask = root.tasks.register('cleanAggregateStyleReports') {
+ it.doLast {
+ deleteReports(codenarcMarkers.files, codenarcReports.files)
+ deleteReports(checkstyleMarkers.files, checkstyleReports.files)
+ }
+ }
- def aggregateTask = root.tasks.register('aggregateStyleViolations') {
+ def writerTask = root.tasks.register('writeStyleViolations') {
it.group = 'verification'
- it.description = 'Aggregates CodeNarc and Checkstyle violation
reports into build/reports/violations/'
-
it.outputs.file(root.file('build/reports/violations/CODENARC_VIOLATIONS.md'))
-
it.outputs.file(root.file('build/reports/violations/CHECKSTYLE_VIOLATIONS.md'))
+ it.description = 'Writes CodeNarc and Checkstyle violation reports
into build/reports/violations/'
+ it.inputs.files(codenarcMarkers).optional()
+ it.inputs.files(checkstyleMarkers).optional()
+ it.inputs.property('ignoreFailures', ignoreFailures)
+ it.outputs.file(codenarcMarkdown)
+ it.outputs.file(checkstyleMarkdown)
+ it.outputs.upToDateWhen { false }
+ it.doFirst {
+ codenarcMarkdown.get().asFile.delete()
+ checkstyleMarkdown.get().asFile.delete()
+ }
it.doLast {
- parseStyleViolations(styleXmlDir.get(), violationsDir.get(),
- checkStyleTests.get(), codenarcEnabled.get(),
checkstyleEnabled.get())
+ parseStyleViolations(codenarcMarkers.files,
checkstyleMarkers.files, rootDirectory, violationsDir.get(),
+ checkStyleTests.get(), codenarcEnabled.get(),
+ checkstyleEnabled.get(), ignoreFailures.get())
}
}
- root.subprojects { Project sub ->
- sub.pluginManager.withPlugin('codenarc') {
- aggregateTask.configure {
- it.dependsOn(sub.tasks.withType(CodeNarc))
- }
+ def aggregateTask = root.tasks.register('aggregateStyleViolations') {
+ it.group = 'verification'
+ it.description = 'Aggregates CodeNarc and Checkstyle violations
into build/reports/violations/'
+ it.dependsOn(writerTask)
+ }
+ if (root.tasks.names.contains('validateRepositoryConventions')) {
+ aggregateTask.configure {
it.dependsOn(root.tasks.named('validateRepositoryConventions')) }
+ }
+ def finalizeTask = root.tasks.register('finalizeStyleViolations') {
+ it.group = 'verification'
+ it.dependsOn(writerTask)
+ }
+ root.allprojects { Project sub ->
+ sub.tasks.withType(CodeNarc).all { CodeNarc codeNarcTask ->
+ codenarcMarkers.from(GradleUtils.reportMarker(sub, 'codenarc',
codeNarcTask.name))
+ def reportLocation = codeNarcTask.reports.xml.outputLocation
+ codenarcReports.from { reportLocation.get().asFile }
+ codeNarcTask.dependsOn(cleanupTask)
+ codeNarcTask.finalizedBy(finalizeTask)
Review Comment:
Fixed, and I ended up taking both of the options you offered, because a
review pass over the final diff showed the first one alone was not enough.
First, the reports now record their scope. Every aggregate report carries a
header line naming the modules that actually contributed data:
```
# CodeNarc Violations Summary
Generated on: ...
Modules analyzed: :grails-core
```
`none` is emitted when nothing contributed.
Second, and this is the part I had missed: the header alone did not fix it.
Because the reports are assembled from marker files left on disk, a module
analyzed in an EARLIER run still contributed its stale markers to the
regenerated report, so `./gradlew :grails-core:check` could produce a report
listing modules it never ran, built from previous-run data. So I also took your
"only rewrite in the aggregate lane" option: `finalizeStyleViolations` and
`finalizeAnalysisViolations` are gone along with all four analyzer
`finalizedBy` registrations. A direct analyzer task now produces only its own
XML report and marker, and the aggregate Markdown is written solely by
`aggregateStyleViolations`, `aggregateAnalysisViolations`, and
`aggregateViolations`, which purge markers first.
There is a regression test that builds an authoritative aggregate report,
runs a single direct analyzer, and asserts the aggregate Markdown is
byte-for-byte unchanged.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -84,82 +86,217 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
def violationsDir =
project.layout.buildDirectory.dir('reports/violations')
- def styleXmlDir =
project.layout.buildDirectory.dir('reports/code-style')
- def analysisXmlDir =
project.layout.buildDirectory.dir('reports/code-analysis')
-
- def styleTask = registerStyleAggregation(project, styleXmlDir,
violationsDir)
- def analysisTask = registerAnalysisAggregation(project,
analysisXmlDir, violationsDir)
+ TaskProvider<RepositoryConventionsTask> repositoryConventionsTask =
project.file(CANONICAL_ROOT_MARKER).isDirectory() ?
+ registerRepositoryConventions(project, violationsDir) : null
+ def styleTask = registerStyleAggregation(project, violationsDir)
+ def analysisTask = registerAnalysisAggregation(project, violationsDir)
registerJacocoAggregation(project, violationsDir)
project.tasks.register('aggregateViolations') { Task task ->
task.group = 'verification'
task.description = 'Aggregates all violation reports (style +
analysis) into build/reports/violations/'
task.dependsOn(styleTask, analysisTask)
+ if (repositoryConventionsTask) {
+ task.dependsOn(repositoryConventionsTask)
+ }
+ }
+ }
+
+ private static TaskProvider<RepositoryConventionsTask>
registerRepositoryConventions(Project root, Provider<Directory> violationsDir) {
+ root.tasks.register('validateRepositoryConventions',
RepositoryConventionsTask) { RepositoryConventionsTask task ->
+ task.group = 'verification'
+ task.description = 'Validates repository conventions and writes
build/reports/violations/REPOSITORY_CONVENTIONS.md'
+ task.repositoryDirectory.set(root.layout.projectDirectory)
+ task.conventionSources.from(
+ root.file('AGENTS.md'),
+ root.fileTree('.agents/skills') { include '*/SKILL.md' },
+ root.fileTree('.github/workflows') { include '**/*.yml',
'**/*.yaml' },
+ root.fileTree('.') {
+ include '**/action.yml', '**/action.yaml'
+ exclude '**/build/**', '**/generated/**',
'**/.gradle/**', '**/.git/**', '**/.hg/**', '**/.svn/**'
+ },
+ root.fileTree('.') {
+ include '**/messages*.properties'
+ exclude '**/build/**', '**/generated/**'
+ }
+ )
+ task.reportFile.set(violationsDir.map {
it.file('REPOSITORY_CONVENTIONS.md') })
+ task.outputs.upToDateWhen { false }
+ task.dependsOn(root.tasks.matching { Task candidate ->
candidate.name == 'rat' })
Review Comment:
Both fixed. The lookup is now `root.tasks.named { String name -> name ==
'rat' }`, which filters by name without realizing anything, and the
relationship is `mustRunAfter` rather than `dependsOn` since the task reads
nothing from rat's output.
You are right that the hard dependency was silently no-oping when rat is not
registered, which is exactly the independent-build case. RAT is still part of
the developer-facing gate: `aggregateViolations` pulls in both the conventions
task and `rat` directly, so the ordering holds where it matters and standalone
runs are no longer taxed.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -84,82 +86,217 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
def violationsDir =
project.layout.buildDirectory.dir('reports/violations')
- def styleXmlDir =
project.layout.buildDirectory.dir('reports/code-style')
- def analysisXmlDir =
project.layout.buildDirectory.dir('reports/code-analysis')
-
- def styleTask = registerStyleAggregation(project, styleXmlDir,
violationsDir)
- def analysisTask = registerAnalysisAggregation(project,
analysisXmlDir, violationsDir)
+ TaskProvider<RepositoryConventionsTask> repositoryConventionsTask =
project.file(CANONICAL_ROOT_MARKER).isDirectory() ?
+ registerRepositoryConventions(project, violationsDir) : null
+ def styleTask = registerStyleAggregation(project, violationsDir)
+ def analysisTask = registerAnalysisAggregation(project, violationsDir)
registerJacocoAggregation(project, violationsDir)
project.tasks.register('aggregateViolations') { Task task ->
task.group = 'verification'
task.description = 'Aggregates all violation reports (style +
analysis) into build/reports/violations/'
task.dependsOn(styleTask, analysisTask)
+ if (repositoryConventionsTask) {
+ task.dependsOn(repositoryConventionsTask)
+ }
+ }
+ }
+
+ private static TaskProvider<RepositoryConventionsTask>
registerRepositoryConventions(Project root, Provider<Directory> violationsDir) {
+ root.tasks.register('validateRepositoryConventions',
RepositoryConventionsTask) { RepositoryConventionsTask task ->
+ task.group = 'verification'
+ task.description = 'Validates repository conventions and writes
build/reports/violations/REPOSITORY_CONVENTIONS.md'
+ task.repositoryDirectory.set(root.layout.projectDirectory)
+ task.conventionSources.from(
+ root.file('AGENTS.md'),
+ root.fileTree('.agents/skills') { include '*/SKILL.md' },
+ root.fileTree('.github/workflows') { include '**/*.yml',
'**/*.yaml' },
+ root.fileTree('.') {
+ include '**/action.yml', '**/action.yaml'
+ exclude '**/build/**', '**/generated/**',
'**/.gradle/**', '**/.git/**', '**/.hg/**', '**/.svn/**'
+ },
+ root.fileTree('.') {
Review Comment:
Fixed. Both trees now carry the same exclusion set, including
`**/.gradle/**`, so daemon-mutated files can no longer perturb the input
fingerprint of an `@InputFiles` property.
While in there I also took your other suggestion and widened the include
from `**/messages*.properties` to `**/grails-app/i18n/**/*.properties`, so the
`spring-security-core.properties` family is covered too.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy:
##########
@@ -243,9 +382,11 @@ class GrailsViolationAggregationPlugin implements
Plugin<Project> {
}
@CompileDynamic
- private static void parseStyleViolations(Directory styleXmlDir, Directory
violationsDir,
- boolean checkStyleTests, boolean codenarcEnabled, boolean
checkstyleEnabled) {
+ private static void parseStyleViolations(Set<File> codenarcMarkers,
Set<File> checkstyleMarkers, Directory rootDirectory,
+ Directory violationsDir, boolean checkStyleTests,
+ boolean codenarcEnabled, boolean checkstyleEnabled, boolean
ignoreFailures) {
Review Comment:
Removed, option (b). CodeNarc and Checkstyle tasks already honor their own
`ignoreFailures` task property and fail on their own findings, so mirroring the
gate in the aggregator would have duplicated it rather than added anything. The
parameter, the `inputs.property('ignoreFailures', ...)` declaration on
`writeStyleViolations`, and the unused argument are all gone, so there is no
longer an input that cannot influence the output. The unconditional
missing-report failure is unchanged, and `parseAnalysisViolations` keeps its
own gate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
