DanielLeens commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-5661674976
Thanks for the thorough follow-up on all three points. **1. JDK 8 gating for the `--add-exports` compilerArgs.** You caught a real gap — pushed `b75d9123be`. The flag was applied unconditionally before, and I confirmed it would have broken any JDK 8 compile of these modules: this project's daily "Upgrade Compatibility" workflow actually builds the current dev distribution under a plain JDK 8 toolchain (its `actions/setup-java` step only sets up JDK 8, and `JAVA_HOME` stays pinned to that JDK 8 install for the entire job, including the `mvn package -pl seatunnel-dist -am` step — verified directly from that workflow's own run logs). `--add-exports` is JDK 9+-only; JDK 8's javac rejects it outright. On JDK 8 there's no module system to bypass in the first place, so `sun.security.krb5.Config`/`sun.misc.Unsafe` are already accessible without the flag — it's only needed once JDK 9's encapsulation applies. Fix: moved the `compilerArgs` for both `connector-kudu` and `connector-paimon` into a new profile gated on `<activation><jdk>[9,)</jdk></activation>`, mirroring the existing `jdk9-plus-test-opens` pattern already in the root `pom.xml` for the analogous Surefire/Failsafe `argLine` gating. Confirmed the resulting POMs parse and pass `spotless:apply` for both modules. **2. `JAVA_TOOL_OPTIONS` removal from `backend.yml`.** Confirmed — grepped the current branch's `backend.yml` and the only remaining occurrences of `JAVA_TOOL_OPTIONS` are in the explanatory comment describing why it's no longer set; there is no actual `env:` entry setting it anymore. Those CI jobs run on a plain JDK 11/17 with no injected `JAVA_TOOL_OPTIONS`, so the "Picked up JAVA_TOOL_OPTIONS" stderr noise is gone. **3. `upgrade_compatibility.yml` cross-version restore scenario.** I want to correct the premise here rather than just answer it: this PR does not actually add any new `--add-opens`/`--add-exports` set to `config/jvm_options` (or `jvm_master_options`/`jvm_worker_options`/`jvm_client_options`) at all — I diffed this PR's changes against those four files specifically and confirmed zero flag lines were added or removed, only the top-of-block comments changed to attribute each existing flag to its actual consumer. The only new flag set this PR introduces is `surefire.jvm.args` in the new `jdk9-plus-test-opens` Maven profile, and that property is consumed exclusively by Surefire/Failsafe for test JVMs forked during `mvn test`/`mvn verify` — it has no effect on the packaged launcher scripts (`seatunnel.sh`/`seatunnel-cluster.sh`) that `upgrade_compatibility.yml` uses to start either the old 2.3.13 release or the newly built dev distribution. So there's no "new `--add-opens` set the old release's bundled config lacks" scenario to verify — nothing this PR changes affects how either side of that cross-version restore test starts at runtime. I also confirmed the workflow itself has been running green daily on `dev`, including the run immediately after `dev` picked up the unrelated minio image fix earlier today. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
