jaikiran commented on PR #216:
URL: https://github.com/apache/ant/pull/216#issuecomment-2551818231
Hello Hannes, caching the value is fine and what you propose here is
reasonable. My only suggestion would be to remove the reference and usage of
Java 12 check. I think if Java version is lesser than 18, then we should merely
rely on the system property value instead of doing additional version checks.
Something like (the following untested code):
```
// Ant will not set the SecurityManager if Java version is 18 or higher.
// For versions lower than Java 18, Ant will not set the SecurityManager
// if the "java.security.manager" system property is set to "disallow".
private static final boolean IS_SET_SECURITYMANAGER_ALLOWED =
!JavaEnvUtils.isAtLeastJavaVersion("18")
&&
!"disallow".equals(System.getProperty("java.security.manager"));
```
The reason I say we should avoid the check for Java 12 or such is because
although the "allow" and "disallow" values for the "java.security.manager"
system property were introduced in that version, there has been efforts to
allow those values in older releases too. More details in
https://bugs.openjdk.org/browse/JDK-8301118. So in context of Ant, it would be
better to leave out that extra check for Java 12.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
