Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-19 Thread Volker Simonis 
On Fri, Feb 16, 2018 at 8:17 PM,  wrote:

> 2018/2/16 10:59:57 -0800, volker.simo...@gmail.com:
> > On Fri, Feb 16, 2018 at 7:02 PM, mark.reinh...@oracle.com wrote:
> >> Of course it's possible.  The specification need merely say that
> >> `java.vendor.version` is a standard system property that may, or may
> >> not, have a value.  (Or, if you like, whose value may be `null`.)
> >
> > Sorry but I still don't get it. Do you agree that you can't assign NULL
> to
> > a system property because you'll get a NPE?
>
> I agree that `System.setProperty("java.vendor.version", null)` will
> throw an NPE, but I don't see how this fact is relevant.
>
> > You could of course not assign it at all (as it is done now) in which
> case
> > System.getProperty("java.vendor.version") would return NULL. But that
> means
> > "it is not defined" which is different from "is has no value".
>
> Let's not get caught up in fine philosophical distinctions between "not
> defined", "has no value", and `null`.  Either `System::getProperty`
> returns a non-`null` value, or it doesn't.  That's all that matters.
>
> >You can
> > still call System.getProperties().containsKey​("java.vendor.version")
> and
> > it would return false which violates that specification because it
> mandates
> > that a property with the "java.vendor.version" exists.
>
> The current specification mandates this.  That's precisely the bug here.
> We can revise it so that it doesn't.
>
>
Re-targeted to 11 with priority P2 and reassigned to you.

Thanks,
Volker


> - Mark
>

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-16 Thread mark . reinhold 
2018/2/16 10:59:57 -0800, volker.simo...@gmail.com:
> On Fri, Feb 16, 2018 at 7:02 PM, mark.reinh...@oracle.com wrote:
>> Of course it's possible.  The specification need merely say that
>> `java.vendor.version` is a standard system property that may, or may
>> not, have a value.  (Or, if you like, whose value may be `null`.)
> 
> Sorry but I still don't get it. Do you agree that you can't assign NULL to
> a system property because you'll get a NPE?

I agree that `System.setProperty("java.vendor.version", null)` will
throw an NPE, but I don't see how this fact is relevant.

> You could of course not assign it at all (as it is done now) in which case
> System.getProperty("java.vendor.version") would return NULL. But that means
> "it is not defined" which is different from "is has no value".

Let's not get caught up in fine philosophical distinctions between "not
defined", "has no value", and `null`.  Either `System::getProperty`
returns a non-`null` value, or it doesn't.  That's all that matters.

>You can
> still call System.getProperties().containsKey​("java.vendor.version") and
> it would return false which violates that specification because it mandates
> that a property with the "java.vendor.version" exists.

The current specification mandates this.  That's precisely the bug here.
We can revise it so that it doesn't.

- Mark

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-16 Thread Volker Simonis 
On Fri, Feb 16, 2018 at 7:02 PM,  wrote:

> 2018/2/14 8:04:15 -0800, volker.simo...@gmail.com:
> > On Wed, Feb 14, 2018 at 4:26 PM, mark.reinh...@oracle.com wrote:
> >> This is a bug in the specification, not the implementation.  As I just
> >> wrote in a comment on 8197927:
> >>
> >> JEP 322 expresses the intended behavior: If `--with-vendor-version` is
> >> not specified at build time then the `java.vendor.version` property has
> >> no value.  (That's different from the empty string, which is a value.)
> >> The bug is in the specification, which should be revised so as not to
> >> require that this property have a value.  Fixing the spec is not
> >> critical for 10; I suggest downgrading 8197927 to P2 and targeting 11.
> >
> > What you mean by "not require the 'java.vendor.version' property to
> > have a value"? We can not set a system property to NULL because
> > System.setProperty() will throw a NPE in that case. So either the
> > specification requires 'java.vendor.version' as mandatory (as it is
> > now) in which case it does need to have a value different from NULL.
> > Or we change the specification such that 'java.vendor.version' isn't a
> > mandatory property any more. But it is not possible to have
> > 'java.vendor.version' as mandatory property with "no value" as
> > envisioned by the JEP.
>
> Of course it's possible.  The specification need merely say that
> `java.vendor.version` is a standard system property that may, or may
> not, have a value.  (Or, if you like, whose value may be `null`.)
>
>
Sorry but I still don't get it. Do you agree that you can't assign NULL to
a system property because you'll get a NPE?

You could of course not assign it at all (as it is done now) in which case
System.getProperty("java.vendor.version") would return NULL. But that means
"it is not defined" which is different from "is has no value". You can
still call System.getProperties().containsKey​("java.vendor.version") and
it would return false which violates that specification because it mandates
that a property with the "java.vendor.version" exists.

Or am I missing something?

> Do you propose to change the specification such that
> > 'java.vendor.version' isn't a mandatory property any more? That would
> > be fine for me and I agree that we could target this bug to 11 if
> > that's what you propose. We could actually list "java.vendor.version"
> > under "Implementation Note:" which contains some additional,
> > non-mandatory properties.
>
> It'd be a standard property, not an implementation-specific property.
>
> Feel free to reassign 8197927 to me and I'll fix it in 11, since it was
> my oversight in the first place.
>
> - Mark
>

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-16 Thread mark . reinhold 
2018/2/14 8:04:15 -0800, volker.simo...@gmail.com:
> On Wed, Feb 14, 2018 at 4:26 PM, mark.reinh...@oracle.com wrote:
>> This is a bug in the specification, not the implementation.  As I just
>> wrote in a comment on 8197927:
>> 
>> JEP 322 expresses the intended behavior: If `--with-vendor-version` is
>> not specified at build time then the `java.vendor.version` property has
>> no value.  (That's different from the empty string, which is a value.)
>> The bug is in the specification, which should be revised so as not to
>> require that this property have a value.  Fixing the spec is not
>> critical for 10; I suggest downgrading 8197927 to P2 and targeting 11.
> 
> What you mean by "not require the 'java.vendor.version' property to
> have a value"? We can not set a system property to NULL because
> System.setProperty() will throw a NPE in that case. So either the
> specification requires 'java.vendor.version' as mandatory (as it is
> now) in which case it does need to have a value different from NULL.
> Or we change the specification such that 'java.vendor.version' isn't a
> mandatory property any more. But it is not possible to have
> 'java.vendor.version' as mandatory property with "no value" as
> envisioned by the JEP.

Of course it's possible.  The specification need merely say that
`java.vendor.version` is a standard system property that may, or may
not, have a value.  (Or, if you like, whose value may be `null`.)

> Do you propose to change the specification such that
> 'java.vendor.version' isn't a mandatory property any more? That would
> be fine for me and I agree that we could target this bug to 11 if
> that's what you propose. We could actually list "java.vendor.version"
> under "Implementation Note:" which contains some additional,
> non-mandatory properties.

It'd be a standard property, not an implementation-specific property.

Feel free to reassign 8197927 to me and I'll fix it in 11, since it was
my oversight in the first place.

- Mark

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Volker Simonis 
On Wed, Feb 14, 2018 at 4:26 PM,   wrote:
> 2018/2/14 2:20:22 -0800, volker.simo...@gmail.com:
>> can I please get a review for the following tiny fix:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
>> https://bugs.openjdk.java.net/browse/JDK-8197927
>>
>> The new Java 10 specification makes the 'java.vendor.version' property
>> mandatory [1] but the current implementations doesn't allow to set it
>> to an empty string.
>
> This is a bug in the specification, not the implementation.  As I just
> wrote in a comment on 8197927:
>
> JEP 322 expresses the intended behavior: If `--with-vendor-version` is
> not specified at build time then the `java.vendor.version` property has
> no value.  (That's different from the empty string, which is a value.)
> The bug is in the specification, which should be revised so as not to
> require that this property have a value.  Fixing the spec is not
> critical for 10; I suggest downgrading 8197927 to P2 and targeting 11.
>

What you mean by "not require the 'java.vendor.version' property to
have a value"? We can not set a system property to NULL because
System.setProperty() will throw a NPE in that case. So either the
specification requires 'java.vendor.version' as mandatory (as it is
now) in which case it does need to have a value different from NULL.
Or we change the specification such that 'java.vendor.version' isn't a
mandatory property any more. But it is not possible to have
'java.vendor.version' as mandatory property with "no value" as
envisioned by the JEP.

Do you propose to change the specification such that
'java.vendor.version' isn't a mandatory property any more? That would
be fine for me and I agree that we could target this bug to 11 if
that's what you propose. We could actually list "java.vendor.version"
under "Implementation Note:" which contains some additional,
non-mandatory properties.

> - Mark

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread mark . reinhold 
2018/2/14 2:20:22 -0800, volker.simo...@gmail.com:
> can I please get a review for the following tiny fix:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
> https://bugs.openjdk.java.net/browse/JDK-8197927
> 
> The new Java 10 specification makes the 'java.vendor.version' property
> mandatory [1] but the current implementations doesn't allow to set it
> to an empty string.

This is a bug in the specification, not the implementation.  As I just
wrote in a comment on 8197927:

JEP 322 expresses the intended behavior: If `--with-vendor-version` is
not specified at build time then the `java.vendor.version` property has
no value.  (That's different from the empty string, which is a value.)
The bug is in the specification, which should be revised so as not to
require that this property have a value.  Fixing the spec is not
critical for 10; I suggest downgrading 8197927 to P2 and targeting 11.

- Mark

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Volker Simonis 
Thanks Thomas.

The sole reason for making it P1 is that it is currently not possible
to build a Java 10 conforming version of OpenJDK with an empty vendor
version.

The specification doesn't mandate a non-empty vendor version and I
don't think OpenJDK should mandate on either.

Regards,
Volker

On Wed, Feb 14, 2018 at 11:50 AM, Thomas Stüfe  wrote:
> Fix is fine, trivial and I do not think there is any risk attached to it. I
> am not in any position to comment whether this is P1. Copyright year needs
> adjusting.
>
> Kind Regards, Thomas
>
> On Wed, Feb 14, 2018 at 11:20 AM, Volker Simonis 
> wrote:
>>
>> Hi,
>>
>> can I please get a review for the following tiny fix:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
>> https://bugs.openjdk.java.net/browse/JDK-8197927
>>
>> The new Java 10 specification makes the 'java.vendor.version' property
>> mandatory [1] but the current implementations doesn't allow to set it
>> to an empty string.
>>
>> Currently, if we don't configure the build with
>> --with-vendor-version=XXX the 'java.vendor.version' property won't be
>> set at all, which violates the spec. Setting it to an empty string
>> will be ignored and has the same behavior like not setting it at all.
>> This also makes default OpenJDK builds (which haven't been configured
>> with --with-vendor-version non-compliant).
>>
>> The fix is trivial: unconditionally set the 'java.vendor.version'
>> property to the value of VENDOR_VERSION_STRING in
>> VersionProps.java.template, even if VENDOR_VERSION_STRING is the empty
>> string (which is the default if --with-vendor-version wasn't given at
>> config time).
>>
>> Thank you and best regards,
>> Volker
>>
>>
>> [1]
>> http://cr.openjdk.java.net/~iris/se/10/pfd/java-se-10-pfd-spec/apidiffs/java/lang/System-report.html#method:getProperties()
>
>

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Thomas Stüfe 
Fix is fine, trivial and I do not think there is any risk attached to it. I
am not in any position to comment whether this is P1. Copyright year needs
adjusting.

Kind Regards, Thomas

On Wed, Feb 14, 2018 at 11:20 AM, Volker Simonis 
wrote:

> Hi,
>
> can I please get a review for the following tiny fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
> https://bugs.openjdk.java.net/browse/JDK-8197927
>
> The new Java 10 specification makes the 'java.vendor.version' property
> mandatory [1] but the current implementations doesn't allow to set it
> to an empty string.
>
> Currently, if we don't configure the build with
> --with-vendor-version=XXX the 'java.vendor.version' property won't be
> set at all, which violates the spec. Setting it to an empty string
> will be ignored and has the same behavior like not setting it at all.
> This also makes default OpenJDK builds (which haven't been configured
> with --with-vendor-version non-compliant).
>
> The fix is trivial: unconditionally set the 'java.vendor.version'
> property to the value of VENDOR_VERSION_STRING in
> VersionProps.java.template, even if VENDOR_VERSION_STRING is the empty
> string (which is the default if --with-vendor-version wasn't given at
> config time).
>
> Thank you and best regards,
> Volker
>
>
> [1] http://cr.openjdk.java.net/~iris/se/10/pfd/java-se-10-pfd-
> spec/apidiffs/java/lang/System-report.html#method:getProperties()
>

[1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Volker Simonis 
Hi,

can I please get a review for the following tiny fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
https://bugs.openjdk.java.net/browse/JDK-8197927

The new Java 10 specification makes the 'java.vendor.version' property
mandatory [1] but the current implementations doesn't allow to set it
to an empty string.

Currently, if we don't configure the build with
--with-vendor-version=XXX the 'java.vendor.version' property won't be
set at all, which violates the spec. Setting it to an empty string
will be ignored and has the same behavior like not setting it at all.
This also makes default OpenJDK builds (which haven't been configured
with --with-vendor-version non-compliant).

The fix is trivial: unconditionally set the 'java.vendor.version'
property to the value of VENDOR_VERSION_STRING in
VersionProps.java.template, even if VENDOR_VERSION_STRING is the empty
string (which is the default if --with-vendor-version wasn't given at
config time).

Thank you and best regards,
Volker


[1] 
http://cr.openjdk.java.net/~iris/se/10/pfd/java-se-10-pfd-spec/apidiffs/java/lang/System-report.html#method:getProperties()