matrei commented on code in PR #16078:
URL: https://github.com/apache/grails-core/pull/16078#discussion_r3699458219
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsDependencyValidatorPlugin.groovy:
##########
@@ -78,6 +87,53 @@ class GrailsDependencyValidatorPlugin implements
Plugin<Project> {
task.onlyIf { Task t -> !shouldSkip(t.project) }
task.doLast { Task t -> validateDependencies(project) }
}
+ project.tasks.register(VALIDATE_RUNTIME_TASK_NAME) { Task task ->
+ task.group = 'verification'
+ task.description = 'Fails if developer-only tooling reaches
the production runtime classpath.'
+ task.onlyIf { Task t ->
t.project.findProperty(FORBIDDEN_RUNTIME_EXT) != null }
+ task.doLast { Task t -> validateProductionClasspath(project) }
+ }
+ project.tasks.matching { Task candidate -> candidate.name ==
'check' }.configureEach { Task check ->
Review Comment:
I think `tasks.matching` is a problem for lazy configuration:
https://docs.gradle.org/current/userguide/task_configuration_avoidance.html#sec:eager_apis_to_avoid
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsDependencyValidatorPlugin.groovy:
##########
@@ -78,6 +87,53 @@ class GrailsDependencyValidatorPlugin implements
Plugin<Project> {
task.onlyIf { Task t -> !shouldSkip(t.project) }
task.doLast { Task t -> validateDependencies(project) }
}
+ project.tasks.register(VALIDATE_RUNTIME_TASK_NAME) { Task task ->
+ task.group = 'verification'
+ task.description = 'Fails if developer-only tooling reaches
the production runtime classpath.'
+ task.onlyIf { Task t ->
t.project.findProperty(FORBIDDEN_RUNTIME_EXT) != null }
+ task.doLast { Task t -> validateProductionClasspath(project) }
Review Comment:
Accessing `project` at execution time.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1940,3 +1940,193 @@ refreshes. Any context not started through
`SpringApplication` — the CLI, plai
tests — keeps the previous fallback behaviour unchanged: a plain
`DefaultStackTraceFilterer` with no
configuration applied. The filterer is JVM-global rather than per-context, so
in a JVM hosting more than one
application context the last one booted supplies the filterer for all of them.
+
+==== 39. Jansi Removed and Developer Tooling Off the Application Classpath
+
+Grails no longer depends on a separate ANSI library. The console renders its
escape sequences directly,
+and the FuseSource Jansi artifact - unmaintained, and the subject of an
unfixed CVE - is gone from the
+dependency graph along with the `org.jline:jansi` fork that briefly replaced
it.
+
+At the same time, developer-only tooling no longer ships with an application.
`grails-console` (the Swing
+console, the Groovy shell, and the script and command runners) was previously
exposed by the web starter,
+which put it - and everything it pulls in - on every application's production
runtime classpath:
Review Comment:
This is not true for the `grails-console`, `groovy-console`,
`groovy-groovysh` and `groovy-swing`.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1940,3 +1940,193 @@ refreshes. Any context not started through
`SpringApplication` — the CLI, plai
tests — keeps the previous fallback behaviour unchanged: a plain
`DefaultStackTraceFilterer` with no
configuration applied. The filterer is JVM-global rather than per-context, so
in a JVM hosting more than one
application context the last one booted supplies the filterer for all of them.
+
+==== 39. Jansi Removed and Developer Tooling Off the Application Classpath
+
+Grails no longer depends on a separate ANSI library. The console renders its
escape sequences directly,
+and the FuseSource Jansi artifact - unmaintained, and the subject of an
unfixed CVE - is gone from the
+dependency graph along with the `org.jline:jansi` fork that briefly replaced
it.
+
+At the same time, developer-only tooling no longer ships with an application.
`grails-console` (the Swing
+console, the Groovy shell, and the script and command runners) was previously
exposed by the web starter,
+which put it - and everything it pulls in - on every application's production
runtime classpath:
+
+* `org.apache.grails:grails-console`
+* `org.apache.groovy:groovy-console`, `groovy-groovysh`, `groovy-swing`
+* the JLine family, including `org.jline:jansi`
+
+None of these are on an application's runtime classpath now. They are
provisioned onto the `grailsCli`
+configuration instead, which the Grails Gradle plugin puts on the compile and
test classpaths but never on
+`runtimeClasspath` - so `bootRun`, `bootJar` and `bootWar` no longer carry
them. The `console` and `shell`
+tasks are unaffected and continue to work exactly as before.
+
+===== What you need to change
+
+Applications generated by earlier versions of Grails declare Jansi explicitly:
+
+[source,groovy]
+.build.gradle
+----
+runtimeOnly "org.fusesource.jansi:jansi"
+----
+
+Delete that line. New projects are generated without it.
+
+If your own code imports Jansi directly, it must move off it - there is no
Grails-managed replacement
+coordinate. Application code that needs coloured output should use Spring
Boot's
+`org.springframework.boot.ansi.AnsiOutput`, or JLine's
`org.jline.utils.AttributedStringBuilder` if the
+project already depends on JLine.
+
+If your code, or a `runtimeOnly` dependency of it, genuinely needs the Groovy
shell, Swing console or JLine
+at application runtime, declare that dependency explicitly rather than relying
on the starter:
+
+[source,groovy]
+.build.gradle
+----
+runtimeOnly "org.apache.groovy:groovy-groovysh"
+----
+
+===== Controlling ANSI output
+
+Jansi's global switch went away with the library. Console colour is now driven
by Spring Boot's own
+property, so it is configured in one place alongside the rest of the
application's output settings:
+
+[source,yaml]
+.application.yml
+----
+spring:
+ output:
+ ansi:
+ enabled: never # always | detect | never
+----
+
+`detect` is the default and keeps the previous behaviour - colour only when
the attached terminal
+supports it. `never` suppresses it entirely, and `always` forces it on even
when the terminal is not
+detected as capable, which is useful for CI logs that render ANSI.
+
+The same property works as a system property, which is how the CLI reads it
since it has no Spring
+`Environment` to bind from:
+
+[source,bash]
+----
+./gradlew bootRun -Dspring.output.ansi.enabled=never
+----
+
+If you previously disabled colour through Jansi's own switch, replace it - the
property no longer has any
+effect now that the library is gone:
+
+[cols="1,1"]
+|===
+|Before |Now
+
+|`-Dorg.fusesource.jansi.Ansi.disable=true`
+|`-Dspring.output.ansi.enabled=never`
+|===
+
+===== Remove `withJansi` from your Logback configuration
+
+Logback's `ConsoleAppender` has a `withJansi` option that loads
`org.fusesource.jansi.AnsiConsole` by
+name to wrap the output stream. With Jansi gone from the dependency graph, an
application that still
+enables it logs a warning on every startup and falls back to the plain stream:
+
+[source]
+----
+-WARN in ch.qos.logback.core.ConsoleAppender[STDOUT] - Failed to create
AnsiPrintStream. Falling back
+on the default stream. java.lang.ClassNotFoundException:
org.fusesource.jansi.AnsiConsole
+----
+
+Delete the element from `grails-app/conf/logback.xml` or `logback-spring.xml`:
+
+[source,xml]
+----
+<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <withJansi>true</withJansi> <!--1-->
+ <encoder>
+ ...
+ </encoder>
+</appender>
+----
+<1> remove this line
+
+Nothing is lost. Coloured log output comes from Spring Boot's `ColorConverter`
- the `%clr(...)` tokens
+in the encoder pattern - which is independent of Jansi and honours
`spring.output.ansi.enabled`. The
+`withJansi` option only wrapped the stream for legacy Windows consoles.
+
+Applications generated by recent versions of Grails already omit it: Jansi's
global `System.out`
+replacement breaks console logging after a Spring Boot DevTools restart
+(https://github.com/apache/grails-core/issues/15663[issue #15663]), so it was
removed from the
+generated configuration before Jansi itself was dropped. Only applications
carrying older
+configuration forward need this change.
+
+===== `GrailsConsole` moved to a companion artifact
+
+The console classes are developer tooling, so they now ship as
`grails-bootstrap`'s CLI companion
+rather than in the artifact applications depend on. These types moved out of
+`org.apache.grails.bootstrap:grails-bootstrap` and into
+`org.apache.grails.bootstrap:grails-bootstrap-cli`:
+
+* `grails.build.logging.GrailsConsole`
+* `grails.build.logging.ConsoleLogger`
+* `grails.build.logging.GrailsEclipseConsole`
+* `org.grails.build.logging.GrailsConsolePrintStream`
+* `org.grails.build.logging.GrailsConsoleErrorPrintStream`
+* `org.grails.build.logging.GrailsConsoleAntBuilder`
+* `org.grails.build.logging.GrailsConsoleBuildListener`
+* `org.grails.build.interactive.CandidateListCompletionHandler`
+
+Their package names are unchanged - only the artifact that carries them is
different.
+
+Command code needs no change: the CLI tier is auto-provisioned onto
`grailsCli`, so commands in
+`grails-app/commands` and in a plugin's `src/cli` continue to see
`GrailsConsole` as before.
+
+If application code (not command code) references any of these types, it was
relying on developer
+tooling being present at runtime. Prefer a logger, or Spring Boot's
+`org.springframework.boot.ansi.AnsiOutput` for coloured output. If the
dependency is genuinely
+required, declare the companion explicitly:
+
+[source,groovy]
+.build.gradle
+----
+implementation "org.apache.grails.bootstrap:grails-bootstrap-cli"
+----
+
+Test code that extended `GrailsConsolePrintStream` to capture output does not
need it: that class
+routes output through the CLI console, which a test almost never wants. Extend
`java.io.PrintStream`
+directly instead.
+
+==== 40. Plugin Authors: Separate Source Sets for the CLI Tier
+
+This affects plugins that apply `org.apache.grails.gradle.grails-plugin-cli`
and therefore publish a
+companion `-cli` artifact.
+
+Previously the `cli` source set's output and its dependencies were added to
the plugin's `test`
+classpath, so unit tests saw CLI classes and CLI-only libraries that an
application never receives. A test
+could pass against something that was not actually there in production.
+
+The `cli` tier now has its own test source sets:
+
+[cols="1,2"]
+|===
+|Source set |Purpose
+
+|`src/testCli`
+|Tests for CLI classes. Runs as the `testCli` task, wired into `check`.
+
+|`src/integration-test-cli`
Review Comment:
Why are we diverging in the naming of these source sets? One use camelCase
and one kebab-case. I think we should align on kebab-case as `integration-test`
already uses it.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsDependencyValidatorPlugin.groovy:
##########
@@ -78,6 +87,53 @@ class GrailsDependencyValidatorPlugin implements
Plugin<Project> {
task.onlyIf { Task t -> !shouldSkip(t.project) }
task.doLast { Task t -> validateDependencies(project) }
Review Comment:
Accessing `project` at execution time. We should create a proper task class
and set the values as providers.
##########
grails-forge/grails-forge-cli/src/main/java/org/grails/forge/cli/InteractiveShell.java:
##########
@@ -62,7 +61,8 @@ public InteractiveShell(CommandLine commandLine,
}
public void start() {
- AnsiConsole.systemInstall();
+ // no ansi stream installation: JLine's terminal enables
virtual-terminal processing where the
+ // platform needs it, and picocli resolves ansi support itself via
Help.Ansi.AUTO
Review Comment:
This comment seems to be about a deleted line. Do we need it? Or can we
re-phrase it?
##########
grails-test-core/build.gradle:
##########
@@ -54,8 +54,7 @@ dependencies {
api 'org.apache.groovy:groovy'
// command line requirements
- api 'org.jline:jline'
- api 'org.fusesource.jansi:jansi'
+ compileOnly 'org.jline:jline'
Review Comment:
Move to compileOnly ordered group?
--
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]