jdaugherty commented on code in PR #15698:
URL: https://github.com/apache/grails-core/pull/15698#discussion_r3408205716
##########
grails-core/src/main/resources/META-INF/spring-configuration-metadata.json:
##########
@@ -78,6 +82,12 @@
"description": "The default package used when generating artefacts
with grails create-* commands.",
"defaultValue": "Set by project template"
},
+ {
+ "name": "grails.cli.pid.file",
Review Comment:
The documentation mismatches the value?
##########
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:
Why do we need to configure this at all in run-app? The gradle behavior
should suffice for both
##########
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:
Technically the port can be dynamic so multiple could run. If we are going
to prevent this, we should hard code the location of the pid file
##########
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:
If stop app is killing with TERM, it's no different than control c / a
graceful shutdown. I don't understand why this is then necessary
##########
grails-core/src/main/groovy/grails/boot/GrailsApp.groovy:
##########
@@ -64,6 +65,16 @@ class GrailsApp extends SpringApplication {
private static final String GRAILS_BANNER = 'grails-banner.txt'
private static final String SPRING_PROFILES = 'spring.profiles.active'
+ /**
+ * System property holding the path of the PID file the application should
write on startup.
+ * It is set by the CLI {@code run-app} command as {@code
grails.cli.pid.file}; the
+ * {@code grails.} prefix is stripped when the property is forwarded into
this forked JVM by
+ * {@code GrailsGradlePlugin}, so it is read here without the prefix. When
present, the PID
+ * file lets {@code stop-app} terminate this process. It is never set for
a normally deployed
+ * application, so production runs are unaffected.
+ */
+ private static final String CLI_PID_FILE_PROPERTY = 'cli.pid.file'
Review Comment:
We should scope our property names - ie add a grails prefix
##########
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 {
Review Comment:
Why is this after eval? You can use withPlugin to wait for the registration
of the plugin and then configure the task
##########
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:
The resolution of the file should occur in the doFirst
--
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]
