JDK-8285497: Add system property for Java SE specification maintenance version #8437
Conversation
|
👋 Welcome back darcy! A progress list of the required criteria for merging this PR into |
Webrevs
|
|
Hello Joe, should the property description have a note stating what kind of a value this property holds, if at all present? Would it be free form text or an integer value? |
|
Should this be added to the default permissions in the |
Seems reasonable; good catch. |
src/java.base/share/classes/java/lang/VersionProps.java.template
Outdated
Show resolved
Hide resolved
| permission java.util.PropertyPermission | ||
| "java.specification.maintenance.version", "read"; |
There was a problem hiding this comment.
For consistency, you should probably also add a hard-coded permission to the sun.security.provider.PolicyFile.initStaticPolicy() method which is used as a fallback if there is a problem parsing the java.policy file. I doubt this will cause any issues as I don't suspect many/any applications will suddenly start reading this property and running with an SM but again it is mainly for consistency as all the other default permissions in this file are also granted in that method.
There was a problem hiding this comment.
Will do; thanks. Just to double-check, under the current proposal, this property will be undefined most of the time. I assume it is fine for the permissions to grant the ability to read a property that is not actually there.
jddarcy
changed the title
There is some discussion about the sequence of values in comments on the JBS issue. I think it would be reasonable to have the sequence: undefined -> 1 -> 2 -> 3 ... where 1, 2, 3 are strings. I'll update the text to be more explicit about the eventual sequence once the discussion is concluded; thanks. |
Will do; I'll update the CSR to reflect that change as well as to be more explicit about the sequence of values, once that is determined. |
CSR now updated accordingly. |
| @@ -109,6 +109,10 @@ class VersionProps { | |||
| props.put("java.class.version", CLASSFILE_MAJOR_MINOR); | |||
|
|
|||
| props.put("java.specification.version", VERSION_SPECIFICATION); | |||
|
|
|||
| // Uncomment next props.put call after the first maintenance release for a | |||
There was a problem hiding this comment.
Is "after" correct? I think this value should be set to the target MR at the beginning of RI development for the release, similar to what is done for other version identification system properties.
There was a problem hiding this comment.
Well, sure -- this versioning info should be updated in the same way we update versions for all the early access builds of a release.
| * After a first maintenance release this property will | ||
| * have the value {@code "1"}; after a second maintenance | ||
| * release, this property will have the value {@code "2"}, | ||
| * and so on. |
There was a problem hiding this comment.
There should be no requirement that values be allocated sequentially. In other words, if JCP MR does not require an RI, then it should not be surprising if there is no JDK build with maintenance version . As an example, JSR 337 MR 1 and MR 2 both used the same RI. If this system property had existed during development of MR 1, it would return "1". Since no RI was submitted for MR 2, a build returning "2" should never exist. MR 3 did update the RI, so it would return "3".
There was a problem hiding this comment.
Is there intent to backport this to the RI for JSR 337 MR 4?
I think that would be helpful, if not strictly necessary.
There was a problem hiding this comment.
Is there intent to backport this to the RI for JSR 337 MR 4?
I think that would be helpful, if not strictly necessary.
PS And if backported, the spec could be updated for earlier release trains to start at a different value like "4".
There was a problem hiding this comment.
@irisclark does raise an interesting point: If, say, MR 2 doesn’t require a change to the RI then the MR 1 RI is also the MR 2 RI, but its java.specification.maintenance.version property will report that it’s the MR 1 RI.
One way to fix this would be to require an RI update with every MR just to update this property, even if no other code in the RI changes — but we prefer to avoid doing RI builds unnecessarily.
Another way to fix it would be to finesse the specification of this property, perhaps:
* <tr><th scope="row">{@systemProperty java.specification.maintenance.version}</th>
* <td>Java Runtime Environment specification maintenance version, which may
* be interpreted as a non-zero integer. If defined, the value of this
* property is the identifying number of the most recent <a
* href="https://jcp.org/en/procedures/jcp2#3.6.4">specification
* maintenance release</a> that required a change to the runtime</a>
* <em>(optional)</em>.
* </td></tr>
There was a problem hiding this comment.
In the latest push, to address the concerns raised updated the proposed wording to, in plain text:
Java Runtime Environment specification maintenance version, not defined before the specification implemented by this runtime has undergone a maintenance release (optional). When defined, the value of this property may be interpreted as a positive integer. The value indicates the latest maintenance release the runtime is known to support. A later release may be supported by the environment. To indicate the first maintenance release this property will have the value "1"; to indicate the second maintenance release, this property will have the value "2", and so on.
There was a problem hiding this comment.
PS CSR Updated to reflect this push; please review: https://bugs.openjdk.java.net/browse/JDK-8285764
There was a problem hiding this comment.
The negative definition above permits the property always to be undefined, but we do want it to be defined when meaningful. It’s also getting to be an awful lot of text to add to the otherwise terse tabular summary of system properties in the System::getProperties() specification.
Consider using this text in the table:
Java Runtime Environment specification maintenance version, which may be interpreted as a positive integer (optional, see below)
and then this in a paragraph following the table:
The {@code java.specification.maintenance.version} property is defined if the specification implemented by this runtime at the time of its construction had undergone a maintenance release. When defined, its value identifies that maintenance release. To indicate the first maintenance release this property will have the value {@code "1"}; to indicate the second maintenance release this property will have the value {@code "2"}, and so on.
| * After a first maintenance release this property will | ||
| * have the value {@code "1"}; after a second maintenance | ||
| * release, this property will have the value {@code "2"}, | ||
| * and so on. |
There was a problem hiding this comment.
The negative definition above permits the property always to be undefined, but we do want it to be defined when meaningful. It’s also getting to be an awful lot of text to add to the otherwise terse tabular summary of system properties in the System::getProperties() specification.
Consider using this text in the table:
Java Runtime Environment specification maintenance version, which may be interpreted as a positive integer (optional, see below)
and then this in a paragraph following the table:
The {@code java.specification.maintenance.version} property is defined if the specification implemented by this runtime at the time of its construction had undergone a maintenance release. When defined, its value identifies that maintenance release. To indicate the first maintenance release this property will have the value {@code "1"}; to indicate the second maintenance release this property will have the value {@code "2"}, and so on.
|
PR and CSR updated as suggested. |
| * time of its construction had undergone a <a | ||
| * href="https://jcp.org/en/procedures/jcp2#3.6.4">maintenance | ||
| * release</a>. When defined, its value identifies that | ||
| * maintenance release. To indicate the first maintenance release |
There was a problem hiding this comment.
The final sentence can be shortened, and looking at it now the semicolon should just be a comma:
* maintenance release. To indicate the first maintenance release
* this property will have the value {@code "1"}, to indicate the
* second maintenance release it will have the value {@code "2"},
* and so on.
Otherwise, this looks good to me.
|
@jddarcy This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
|
/integrate |
openjdk
bot
removed
ready
Add a new system property, java.specification.maintenance.version, to return the maintenance release number of the Java SE specification being implemented. The property is unset, optional in the terminology of System.getProperties, for an initial release of a specification.
Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8285764
I'll update copyright years before an integration.
Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8437/head:pull/8437$ git checkout pull/8437Update a local copy of the PR:
$ git checkout pull/8437$ git pull https://git.openjdk.java.net/jdk pull/8437/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 8437View PR using the GUI difftool:
$ git pr show -t 8437Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8437.diff