codeconsole commented on PR #16180:
URL: https://github.com/apache/grails-core/pull/16180#issuecomment-5636465061
Thank you for actually building the image — that is the half nobody had run,
and it turned a "not executed yet" into a diagnosis. Everything below is fixed
or accepted.
**High — the native half cannot pass, and the README recipe does not produce
a working image.** Correct, and the README was worse than incomplete: it was
missing the precondition that made it work the one time it did. That run used
an unreleased build of GROOVY-12234 (apache/groovy#2766) forced onto
`runtimeClasspath`; nothing about it was in the README, so following the README
could only ever reproduce your result. Confirmed your diagnosis against the
jars: `groovy-5.1.2` has `makeBootHandle` but neither `aotDispatch` nor the
`ensureInitialized` guard, while the GROOVY-12234 build has `AotDispatch`,
`aotDispatch` and `ensureInitialized`.
The "Native image verification" section is rewritten. It now leads with the
fact that `-PnativeTests` is expected to fail, gives both failure modes in a
table (non-indy defines call-site classes, indy NPEs at `makeBootHandle`),
names GROOVY-12234 and oracle/graal#10200 as the two preconditions, says the
module has been run successfully exactly once and against what, and states that
indy is necessary but not sufficient. Your point that "no released Grails can
produce a working image" was true for the wrong reason in isolation is called
out explicitly. The `UnsupportedFeatureError` and parallelism observations are
now attributed to the toolchain they came from.
**Medium — the dead `catch`.** Right, and it is the more useful message of
the two. `check` now treats `actual == code` as "its bundle never reached the
message source"; the `NoSuchMessageException` wording is gone from the README
and the Javadoc, which instead say that a missing bundle surfaces as a code
resolving to itself because `use-code-as-default-message` is on.
**Low — commented-out AOT probe.** Removed.
**Low — `grails { indy = true }` duplicates the convention.** Kept, comment
rewritten to say why: so the JVM half is compiled the same way as the image
rather than being a second dialect of the same fixture, and noting it changes
nothing about the outcome on the pinned Groovy.
**Low — the JDK justification.** Fixed. It now says `GroovyCompile` has no
`release` option so the Groovy sources in `grails-gradle` target the running
JDK, and that publish-JDK and image-JDK do not have to agree because
`toolchainDetection = false` makes the plugin honour `GRAALVM_HOME`.
**Low — GraalVM plugin version.** 1.1.7 → 1.1.12, with a comment on why it
is pinned in `end-to-end/gradle.properties` rather than `dependencies.gradle`.
**Nits.** All taken: explicit exit code on failure via a single `close()` in
`finally`; `i18nCheckOnNative` takes its path from `nativeCompile`'s
`outputFile` provider; the `toolchainDetection` comment now says what the
setting does; `l'application`; `vulnerability-scan` applied to both new
modules; README table back in alphabetical order.
**On the draft flag** — the description claimed this "lets a missing hint
actually be falsified", which is not true yet, and still referred to being a
draft. Rewritten to say what the module does and does not currently prove.
**On where this belongs.** Leaving it on 8.0.x rather than moving it to
9.0.x. The JVM half stands on its own here — it guards the descriptor → base
name → hint pipeline, the multi-word plugin normalisation and the dotted base
name, and it is what caught #16179 — and the native half is inert behind
`-PnativeTests` with no CI job, so it costs 8.0.x nothing while it cannot pass.
Written this way it starts passing when the preconditions land instead of
having to be written then, and it reaches 9.0.x by forward-merge anyway. What
should be decided separately, as you say, is whether native image is an 8.0
claim at all, given `configureNativeImage` and the upgrade guide currently
imply it is.
--
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]
