Re: [PR] GEODE-10531: Remove SecurityManager Usage from OSProcess.java - Java 21 Blocker [geode]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #7971:
URL: https://github.com/apache/geode/pull/7971#discussion_r4001998909


##########
geode-logging/src/main/java/org/apache/geode/logging/internal/OSProcess.java:
##########
@@ -197,6 +195,9 @@ public static int bgexec(String[] cmdarray, File workdir, File logfile, boolean
       throw new IOException(String.format("the executable %s does not exist",
           cmd.getPath()));
     }
+    // GEODE-10531: ProcessBuilder checks the shell, not the requested executable. Retain
+    // this check until support for the legacy SecurityManager policy is explicitly retired.
+    @SuppressWarnings("removal")
     SecurityManager security = System.getSecurityManager();

Review Comment:
   This adds the deprecated `SecurityManager` API back into `OSProcess` rather than removing its usage, so it no longer matches the PR title/GEODE-10531 objective. The README describes this as an unapproved compatibility exception and requires the policy to be recorded in the issue and PR description before merge, but the current PR description still does not document that exception; please align the acceptance criteria and description (or implement the requested removal) before merging.



-- 
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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org