DanielLeens commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-5649548649
Pushed `67d5410bfb`, which closes F3, F6, and F8. **F6/F8** — added a `jdk9-plus-test-opens` profile to the root `pom.xml`, activated by JDK version, that sets `surefire.jvm.args` to the same six flags on Surefire/Failsafe-forked test JVMs. This is additive to the launcher-script injection from `504eb8b2979`: the scripts already cover a real or containerized cluster, and this profile covers the other place the flags are actually needed — host-side JVMs Maven forks to run unit and engine e2e tests, including Hazelcast-backed in-process tests that never touch the launcher scripts. With both in place, `.github/workflows/backend.yml`'s workflow-level `JAVA_TOOL_OPTIONS` is redundant and removed. It previously applied to every job regardless of whether that job ran a JVM at all, so this also removes the `Picked up JAVA_TOOL_OPTIONS` noise you flagged. A side benefit: `./mvnw test`/`./mvnw verify` on JDK 11/17 now gets the same flags locally that CI does, which the env-only approach didn't provide. **F3** — replaced the single blanket comment in each `config/jvm_*_options` file with a comment naming the consumer above each individual flag. Two consumers are confirmed by grepping this codebase for direct use of each JDK API: `java.net` (`URLClassLoader#addURL` reflection in `AbstractPluginDiscovery`, `ConfigValidationUtils`, and the Flink starter plugin loaders) and the krb5 export (`KuduUtil`, `PaimonSecurityContext`). I could not find a first-party call site in this codebase for `java.lang`, `java.nio`, `java.util`, or `sun.nio.ch` — the existing top-of-file comment already attributed these to Hazelcast's own internals, and I've repeated that attribution per-flag rather than invent a precision I can't confirm. I did not trim any flag. Verifying that a flag is genuinely unused would mean checking every consumer including inside the Hazelcast dependency itself, not just this codebase's own call sites — removing one that's still needed by Hazelcast internals risks silently breaking a Kerberos-secured or Hazelcast-internal path with no test coverage of its own. That's a materially larger and riskier change than closing the CI-parity gap this PR needs, so I've gone with your option (b): a short justification per flag, rather than option (a). Fork Build is running on the new head; I'll ping again once it's green. -- 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]
