Follow up wrt #191 by using NativeImageUtil.isRunningInNativeImage() in databind

[JooHyukKim]

Motivation

Seems like the TODO: was written before exposing NativeImageUtil.isRunningInNativeImage() in databind by FasterXML/jackson-databind#4060

[JooHyukKim]

@stevenschlansker Could you please take a look? Just to make sure :)

[cowtowncoder]

Not sure this can be done, since method Javadoc states:

     * This check cannot be a constant, because
     * the static initializer may run early during build time

[JooHyukKim]

I see. I missed the JavaDoc. Maybe we can instead inline the check inside ‘setupModule()’?

[cowtowncoder]

Yes, I think inlining would make sense: it will only be called once anyway.

[JooHyukKim]

Hmmmm strange, the doc says the check cannot be constant, but with current jackson-databind version, the check is still partly static. If we look at the RUNNING_IN_SVM in below, it is static final, so we can't use that method (according to JavaDoc).

[JooHyukKim]

For other modules to use NativeImageUtil.isRunningInNativeImage(), what I would expect is

public class NativeImageUtil {
   
    private static final boolean RUNNING_IN_SVM = isRunningInNativeImage();

    /**
     * Check whether we're running in substratevm native image runtime mode.
     * This check cannot be a constant, because
     * the static initializer may run early during build time
     *<p>
     * NOTE: {@code public} since 2.16 (before that, {@code private}).
     */
    public static boolean isRunningInNativeImage() {
        return System.getProperty("org.graalvm.nativeimage.imagecode") != null && "runtime".equals(System.getProperty("org.graalvm.nativeimage.imagecode"));
    }
   // ...rest omitted

.... as such so that actual check is done at call time. 🤔 Sorry if I misunderstood the whole mechanism though.

[yawkat]

as discussed in #216, isRunningInNativeImage() should not be used here. There needs to be a new getter for RUNNING_IN_SVM that can be used instead.

[yawkat]

maybe isRunningInNativeImage() should be renamed also. i did not think enough about the name when i wrote it, since it wasn't public api. now that it is, a better name would be appropriate.

[JooHyukKim]

maybe isRunningInNativeImage() should be renamed also. i did not think enough about the name when i wrote it, since it wasn't public api. now that it is, a better name would be appropriate.

+1. Method names are also subject to change.

[JooHyukKim]

So I....

1. Made a PR in Update NativeImageUtil method name and expose new getter jackson-databind#4063.
2. Adopted new getter early (CI will temporarily fail)

[cowtowncoder]

Since the method was non-public before 2.16, it's fine to change the name I think.

[stevenschlansker]

I think you missed updating this line with the new name.

[JooHyukKim]

Right, thank you! @stevenschlansker 🙏🏽🙏🏽