jamesfredley commented on code in PR #15698:
URL: https://github.com/apache/grails-core/pull/15698#discussion_r3424576194
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -593,6 +597,23 @@ class GrailsGradlePlugin extends GroovyPlugin {
configureToolchainForForkTasks(project)
}
+ protected void configureBootRunPidFile(Project project) {
+ project.afterEvaluate {
+ project.tasks.withType(BootRun).configureEach { BootRun task ->
+ Provider<RegularFile> defaultPidFile =
project.layout.buildDirectory.file(RUN_APP_PID_FILE_NAME)
+
+ if (!task.systemProperties[CLI_PID_FILE_PROPERTY]) {
+ task.systemProperty(CLI_PID_FILE_PROPERTY,
defaultPidFile.get().asFile.absolutePath)
Review Comment:
Addressed in 9a132a87fa. Rather than a `doFirst`, the path is now resolved
lazily at execution time via a `CommandLineArgumentProvider`
(`RunAppPidFileProvider`), mirroring the existing `GrailsAppBaseDirProvider` in
this file. That keeps the build-directory resolution out of configuration time
and stays configuration-cache safe, while remaining unit-testable. Happy to
switch to a plain `doFirst` if you would rather.
##########
grails-profiles/base/commands/run-app.groovy:
##########
@@ -22,15 +22,29 @@ try {
arguments << '--quiet'
}
- arguments << '-Dgrails.management.endpoints.shutdown.enabled=true'
+ // Tell the forked application where to write its PID file so that
stop-app can terminate it,
+ // even from a separate CLI invocation. The grails. prefix is stripped by
GrailsGradlePlugin
+ // when forwarding the property into the forked JVM.
+ File pidFile = org.grails.cli.gradle.RunningApplicationProcess.pidFile(
+ buildDir,
+
commandLine.systemProperties[org.grails.cli.gradle.RunningApplicationProcess.PID_FILE_CLI_PROPERTY]
as String,
+
System.getProperty(org.grails.cli.gradle.RunningApplicationProcess.PID_FILE_CLI_PROPERTY),
+
config.getProperty(org.grails.cli.gradle.RunningApplicationProcess.PID_FILE_CLI_PROPERTY,
String))
+ pidFile.parentFile?.mkdirs()
Review Comment:
Agreed - removed in 9a132a87fa. `run-app` no longer passes
`-Dgrails.cli.pid.file`; the `bootRun` configuration in `GrailsGradlePlugin` is
the single source of the PID file path for both `run-app` and `stop-app`.
##########
grails-profiles/base/commands/run-app.groovy:
##########
@@ -70,6 +84,11 @@ try {
}
}
+ if(org.grails.cli.gradle.RunningApplicationProcess.isRunning(pidFile)) {
+ console.error "An application started with run-app is already running
for this project. Run 'stop-app' first."
Review Comment:
Done in 9a132a87fa - the PID file is now hard-coded to `build/run-app.pid`
(no command-line/system/config override), so `run-app`'s already-running guard
and `stop-app` always agree on one location.
##########
grails-profiles/base/commands/run-app.groovy:
##########
@@ -125,6 +144,12 @@ catch(org.gradle.tooling.BuildCancelledException e) {
return true
}
catch(Throwable e) {
+ // A deliberate stop-app terminates the bootRun process, which surfaces
here as a build
+ // failure; report it as a clean shutdown rather than a startup failure.
+
if(org.grails.cli.gradle.RunningApplicationProcess.isStopRequested(buildDir)) {
Review Comment:
You are right that for the same-session interactive case a TERM is just a
graceful shutdown. The reason the marker stays: `stop-app` calls
`ProcessHandle.destroy()` on only the forked application JVM, not the Gradle
process. So unlike Ctrl+C of `./gradlew bootRun` (which signals the whole
process group and surfaces as a build cancellation), the `bootRun` build here
ends with a non-zero child exit (e.g. 143 on Unix) and the Tooling API reports
a build failure. A foreground / separate-terminal `run-app` blocked on that
build needs the `run-app.stopping` marker to report a clean stop instead of a
startup failure. I kept it but documented exactly this in `stop-app.groovy` and
`RunningApplicationProcess`.
If you would prefer, an alternative is to send a Gradle build cancellation
for the same-session case and reserve the PID/TERM path for cross-invocation
stops - happy to go that route in a follow-up.
--
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]
