DanielLeens commented on PR #11545:
URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-5651209720
Both confirmations, plus a gap your second question caught before it could
reach CI:
**Failsafe confirmation** — yes, both plugins are driven by the same
`surefire.jvm.args` property. In the root `pom.xml`'s `pluginManagement`:
```xml
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration><argLine>${surefire.jvm.args}</argLine>
...</configuration>
</plugin>
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration><argLine>${surefire.jvm.args}</argLine>
...</configuration>
</plugin>
```
The `jdk9-plus-test-opens` profile overrides that one property, so both pick
up the six flags together.
**"No other source of truth" — this one turned up a real gap.** Grepping the
branch for `JAVA_TOOL_OPTIONS` outside `backend.yml` found three other
workflows that set it independently: `codeql.yaml`, `publish-docker.yaml`, and
`upgrade_compatibility.yml`. All three only set the krb5 export (not the other
five flags), for CodeQL scanning, Docker image publishing, and the
upgrade-compatibility test run respectively — none of them go through
`backend.yml`'s job definitions or this PR's Surefire/Failsafe profile at all,
so they're independent, not duplicated, sources of truth for their own separate
workflows. I'm not touching those three; they're out of this PR's scope.
But tracing *why* they each need the krb5 export independently exposed the
actual gap: `KuduUtil.java` and `PaimonSecurityContext.java` directly `import
sun.security.krb5.Config`/`KrbException` — that's a compile-time
module-encapsulation check (javac refuses with "package ... is not visible"
without `--add-exports`), not a runtime reflection check. The old
workflow-level `JAVA_TOOL_OPTIONS` happened to satisfy this too, because it's
read by the JVM that launches `mvn` itself, and this project's
`maven-compiler-plugin` runs javac in-process in that same JVM (no
`<fork>true</fork>` anywhere in the reactor). My `surefire.jvm.args` →
`argLine` fix only reaches JVMs Surefire/Failsafe *fork* for test execution — a
later, separate phase from compilation — so removing the workflow-level env var
without an equivalent for the compile phase would have broken compiling these
two modules under JDK 11/17 in every `backend.yml` job that builds them, which
is exactly the class of regress
ion this whole PR exists to fix.
Pushed `67da78738c`: adds
`--add-exports=java.security.jgss/sun.security.krb5=ALL-UNNAMED` to
`connector-kudu` and `connector-paimon`'s own `maven-compiler-plugin` config.
While auditing this I also found `KuduSourceSplitEnumeratorTest.java` directly
imports `sun.misc.Unsafe`, so I added
`--add-exports=java.base/sun.misc=ALL-UNNAMED` to `connector-kudu` as well for
its test sources. A full-repo grep confirms these are the only two modules with
a direct compile-time dependency on an encapsulated JDK-internal package. No
JDK 8 guard needed since this PR's own baseline (`java.version`) is already 11.
I haven't touched F1/F2/F4/F5/F7 this round — those were already addressed
by earlier commits (`504eb8b2979` for F1/F5, `1b1da9e864e` for F2,
`be5492399ba` for F4/F7), with the exact diffs and line numbers linked in my
prior comment on this thread. Nothing in this round's commits changes those
files, so that status is unchanged; happy to re-link or re-verify any specific
one if it'd help close them out.
Fresh fork Build is running on `67da78738c`; will report once it finishes.
--
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]
