[11u] RFR: 8242154: Backport parts of JDK-4947890 to OpenJDK 11u

[Severin Gehwolf]

Hi Christoph,

On Thu, 2020-04-09 at 20:38 +0000, Langer, Christoph wrote:
> Hi Severin,
> 
> thanks for having a look at this.
> 
> > -----Original Message-----
> > From: Severin Gehwolf <sgehwolf at redhat.com>
> > Sent: Donnerstag, 9. April 2020 14:12
> > To: Langer, Christoph <christoph.langer at sap.com>; jdk-updates-
> > dev at openjdk.java.net
> > Subject: Re: [11u] RFR: 8242154: Backport parts of JDK-4947890 to OpenJDK
> > 11u
> > 
> > Hi Christoph,
> > 
> > On Sat, 2020-04-04 at 12:23 +0000, Langer, Christoph wrote:
> > > Hi,
> > > 
> > > please review this partial backport of JDK-4947890 [0] to OpenJDK
> > > 11u.
> > > 
> > > I'm currently working on backporting 8232080: jlink plugins for
> > > vendor information and command-line options [1]. That patch adds a
> > > jlink plugin for setting customized values for system properties
> > > java.vendor.version, java.vendor.url and java.vendor.url.bug in a
> > > jlinked runtime image by modifying the bytecode of class
> > > java.lang.VersionProps. Amongst other things, JDK-4947890 created the
> > > foundation for this patch by moving the initialization of
> > > java.vendor.url and java.vendor.url.bug to java.lang.VersionProps.
> > > Before, it was initialized in libjava's System.c. While it's not
> > > desirable to backport JDK-4947890 as a whole to 11u, I'd like to
> > > bring in the part that moves initialization of java.vendor,
> > > java.vendor.url and java.vendor.url.bug to java.lang.VersionProps.
> > > Since it's only a part of the original JDK-4947890, I created the new
> > > JDK11 specific bug JDK- 8242154.
> > 
> > OK.
> > 
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8242154
> > > Original change of JDK-4947890:
> > http://hg.openjdk.java.net/jdk/jdk/rev/0bdbf854472f
> > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8242154.11u.0/
> > 
> > in System.c:
> > 
> > -    PUTPROP(props, "java.version", VERSION_SHORT);
> > -    PUTPROP(props, "java.vendor", VENDOR);
> > -    PUTPROP(props, "java.vendor.url", VENDOR_URL);
> > -    PUTPROP(props, "java.vendor.url.bug", VENDOR_URL_BUG);
> > -
> > 
> > Aren't you removing "PUTPROP(props, "java.version", VERSION_SHORT)";
> > too eagerly here?
> 
> I don't think so.

Well, my point was more about the following: For this partial backport
it should be sufficient to only remove PUTPROP calls for *vendor*
related properties. I.e. this isn't part of 4947890 while the others
are. I agree it shouldn't matter from a functional perspective. It's
just a bit confusing.

Patch seems OK otherwise.

Thanks,
Severin

> The removed statements put the named properties into a Properties
> object that is handed down to private static native method
> java.lang.System:: initProperties(Properties props).
> This method is actually called twice. Once in System:: initPhase1.
> But after it's called there, (line 1948), VersionProps.init() is
> invoked as well (line 1968).
> Then, the other occasion is System::setProperties(Properties props),
> in case the passed props object is null. And for that case, I added
> the call to VersionProps.init() right after initProperties (line 776
> of System.java). The only drawback of that could be that now a few
> properties could be initialized to a value which weren't before when
> calling System.setProperties(null); That would affect
> "java.version.date", "java.runtime.version", "java.runtime.name" and
> "java.vendor.version"; But does that matter? How likely is it that
> somebody relies on that behavior?
> 
> Best regards
> Christoph
>