RFR: 8257733: Move module-specific data from make to respective module
A lot (but not all) of the data in make/data is tied to a specific module. For instance, the publicsuffixlist is used by java.base, and fontconfig by java.desktop. (A few directories, like mainmanifest, is *actually* used by make for the whole build.) These data files should move to the module they belong to. The are, after all, "source code" for that module that is "compiler" into resulting deliverables, for that module. (But the "source code" language is not Java or C, but typically a highly domain specific language or data format, and the "compilation" is, often, a specialized transformation.) This misplacement of the data directory is most visible at code review time. When such data is changed, most of the time build-dev (or the new build label) is involved, even though this has nothing to do with the build. While this is annoying, a worse problem is if the actual team that needs to review the patch (i.e., the team owning the module) is missed in the review. - Commit messages: - Update references in test - Step 2: Update references - First stage, move actual data files Changes: https://git.openjdk.java.net/jdk/pull/1611/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8257733 Stats: 78 lines in 1575 files changed: 3 ins; 1 del; 74 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module
On Thu, 3 Dec 2020 23:44:20 GMT, Magnus Ihse Bursie wrote: > A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. To facilitate review, here is a list on how the different directories under make/data has moved. **To java.base:** * blacklistedcertsconverter * cacerts * characterdata * charsetmapping * cldr * currency * lsrdata * publicsuffixlist * tzdata * unicodedata **To java.desktop:** * dtdbuilder * fontconfig * macosxicons * x11wrappergen **To jdk.compiler:** * symbols **To jdk.jdi:** * jdwp **Remaining in make:** * bundle * docs-resources * macosxsigning * mainmanifest - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 10:29:48 GMT, Magnus Ihse Bursie wrote: >> A lot (but not all) of the data in make/data is tied to a specific module. >> For instance, the publicsuffixlist is used by java.base, and fontconfig by >> java.desktop. (A few directories, like mainmanifest, is *actually* used by >> make for the whole build.) >> >> These data files should move to the module they belong to. The are, after >> all, "source code" for that module that is "compiler" into resulting >> deliverables, for that module. (But the "source code" language is not Java >> or C, but typically a highly domain specific language or data format, and >> the "compilation" is, often, a specialized transformation.) >> >> This misplacement of the data directory is most visible at code review time. >> When such data is changed, most of the time build-dev (or the new build >> label) is involved, even though this has nothing to do with the build. While >> this is annoying, a worse problem is if the actual team that needs to review >> the patch (i.e., the team owning the module) is missed in the review. > > To facilitate review, here is a list on how the different directories under > make/data has moved. > > **To java.base:** > * blacklistedcertsconverter > * cacerts > * characterdata > * charsetmapping > * cldr > * currency > * lsrdata > * publicsuffixlist > * tzdata > * unicodedata > > **To java.desktop:** > * dtdbuilder > * fontconfig > * macosxicons > * x11wrappergen > > **To jdk.compiler:** > * symbols > > **To jdk.jdi:** > * jdwp > > **Remaining in make:** > * bundle > * docs-resources > * macosxsigning > * mainmanifest Are you proposing any text or guidelines to be added to JEP 201 as part of this? I think the location of jdwp.spec and its location in the build tree may need to be looked at again. It was convenient to have it in the make tree, from which the protocol spec, and source code for the front end (module jdk.jdi) and a header file for the back end (module jdk.jdwp.agent) are created. Given that the JDWP protocol is standard (not JDK-specific) then there may be an argument to put it in src/java.se instead of a JDK-specific module. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v5]
On Wed, 2 Dec 2020 16:35:24 GMT, Daniel Fuchs wrote: >> Kartik Ohri has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Align -> and remove trailing whitespace > > Marked as reviewed by dfuchs (Reviewer). @pconcannon, If everything is in order, can you please approve the changes on Github. I'll then issue the integrate command. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 11:14:49 GMT, Alan Bateman wrote: >> To facilitate review, here is a list on how the different directories under >> make/data has moved. >> >> **To java.base:** >> * blacklistedcertsconverter >> * cacerts >> * characterdata >> * charsetmapping >> * cldr >> * currency >> * lsrdata >> * publicsuffixlist >> * tzdata >> * unicodedata >> >> **To java.desktop:** >> * dtdbuilder >> * fontconfig >> * macosxicons >> * x11wrappergen >> >> **To jdk.compiler:** >> * symbols >> >> **To jdk.jdi:** >> * jdwp >> >> **Remaining in make:** >> * bundle >> * docs-resources >> * macosxsigning >> * mainmanifest > > Are you proposing any text or guidelines to be added to JEP 201 as part of > this? > > I think the location of jdwp.spec and its location in the build tree may need > to be looked at again. It was convenient to have it in the make tree, from > which the protocol spec, and source code for the front end (module jdk.jdi) > and a header file for the back end (module jdk.jdwp.agent) are created. Given > that the JDWP protocol is standard (not JDK-specific) then there may be an > argument to put it in src/java.se instead of a JDK-specific module. @AlanBateman Well, I don't know about updating JEP 201. Do you mean that `data` should be added to the list `classes`, `native`, `conf`, `legal` as presented under the heading "New scheme"? That list does not seem to have been kept up to date anyway. A quick glance also shows that we now have at least `man` and `lib` as well in this place. So either we say there's precedence for not updating the list, in which case I will do nothing. Or we should bring JEP 201 up-to-date with current practices, which then of course should include `data` but also all other new directories that has been added since JEP 201 was written. I really don't care either way, but my personal opinion is that JEP 201 presented a view on how the plan was to re-organize things, given the knowledge and state of affairs at that time, but how we keep the source code organized and structured from there on, is a living, day-to-day engineering effort that is just hampered by having to update a formal document, that serves little purpose now that it has been implemented. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 11:37:41 GMT, Magnus Ihse Bursie wrote: >> Are you proposing any text or guidelines to be added to JEP 201 as part of >> this? >> >> I think the location of jdwp.spec and its location in the build tree may >> need to be looked at again. It was convenient to have it in the make tree, >> from which the protocol spec, and source code for the front end (module >> jdk.jdi) and a header file for the back end (module jdk.jdwp.agent) are >> created. Given that the JDWP protocol is standard (not JDK-specific) then >> there may be an argument to put it in src/java.se instead of a JDK-specific >> module. > > @AlanBateman Well, I don't know about updating JEP 201. Do you mean that > `data` should be added to the list `classes`, `native`, `conf`, `legal` as > presented under the heading "New scheme"? That list does not seem to have > been kept up to date anyway. A quick glance also shows that we now have at > least `man` and `lib` as well in this place. So either we say there's > precedence for not updating the list, in which case I will do nothing. Or we > should bring JEP 201 up-to-date with current practices, which then of course > should include `data` but also all other new directories that has been added > since JEP 201 was written. > > I really don't care either way, but my personal opinion is that JEP 201 > presented a view on how the plan was to re-organize things, given the > knowledge and state of affairs at that time, but how we keep the source code > organized and structured from there on, is a living, day-to-day engineering > effort that is just hampered by having to update a formal document, that > serves little purpose now that it has been implemented. And I can certainly move jdwp.spec to java.base instead. That's the reason I need input on this: All I know is that is definitely not the responsibility of the Build Group to maintain that document, and I made my best guess at where to place it. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 11:38:51 GMT, Magnus Ihse Bursie wrote: > And I can certainly move jdwp.spec to java.base instead. If jdwp.spec has to move to the src tree then src/java.se is probably the most suitable home because Java SE specifies JDWP as an optional interface. So nothing to do with java.base and the build will need to continue to generate the sources for the front-end (jdk.jdi) and back-end (jdk.jdwp.agent) as they implement the protocol. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v5]
On Fri, 4 Dec 2020 11:22:34 GMT, Kartik Ohri wrote: > @pconcannon, If everything is in order, can you please approve the changes on > Github. I'll then issue the integrate command. Thanks. Hi @amCap1712, you will have to /integrate first, and then afterwards I will sponsor - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 14:03:08 GMT, Erik Joelsson wrote:
>>> And I can certainly move jdwp.spec to java.base instead.
>>
>> If jdwp.spec has to move to the src tree then src/java.se is probably the
>> most suitable home because Java SE specifies JDWP as an optional interface.
>> So nothing to do with java.base and the build will need to continue to
>> generate the sources for the front-end (jdk.jdi) and back-end
>> (jdk.jdwp.agent) as they implement the protocol.
>
> My understanding of JEPs is that they should be viewed as living documents.
> In this case, I think it's perfectly valid to update JEP 201 with additional
> source code layout. Both for this and for the other missing dirs.
Regarding the chosen layout. Did you consider following the existing pattern of
src//{share,}/data?
-
PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 12:30:02 GMT, Alan Bateman wrote: >> And I can certainly move jdwp.spec to java.base instead. That's the reason I >> need input on this: All I know is that is definitely not the responsibility >> of the Build Group to maintain that document, and I made my best guess at >> where to place it. > >> And I can certainly move jdwp.spec to java.base instead. > > If jdwp.spec has to move to the src tree then src/java.se is probably the > most suitable home because Java SE specifies JDWP as an optional interface. > So nothing to do with java.base and the build will need to continue to > generate the sources for the front-end (jdk.jdi) and back-end > (jdk.jdwp.agent) as they implement the protocol. My understanding of JEPs is that they should be viewed as living documents. In this case, I think it's perfectly valid to update JEP 201 with additional source code layout. Both for this and for the other missing dirs. - PR: https://git.openjdk.java.net/jdk/pull/1611
Integrated: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http
On Sat, 21 Nov 2020 11:45:42 GMT, Kartik Ohri wrote: > Hi! > Kindly review this patch to replace switch statements with switch expressions > (where it makes sense) in the http client modules. The rationale is to > improve readability of the code. > Regards, > Kartik This pull request has now been integrated. Changeset: ac549008 Author:Kartik Ohri URL: https://git.openjdk.java.net/jdk/commit/ac549008 Stats: 271 lines in 17 files changed: 2 ins; 124 del; 145 mod 8257401: Use switch expressions in jdk.internal.net.http and java.net.http Reviewed-by: chegar, dfuchs, pconcannon - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: 8257733: Move module-specific data from make to respective module
On Fri, 4 Dec 2020 14:05:54 GMT, Erik Joelsson wrote:
>> My understanding of JEPs is that they should be viewed as living documents.
>> In this case, I think it's perfectly valid to update JEP 201 with additional
>> source code layout. Both for this and for the other missing dirs.
>
> Regarding the chosen layout. Did you consider following the existing pattern
> of src//{share,}/data?
@erikj79 Good point, that makes much more sense.
-
PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257724: Incorrect package of the linked class in BaseSSLSocketImpl
On Thu, 3 Dec 2020 20:44:46 GMT, Xue-Lei Andrew Fan wrote: > In sun.security.ssl.BaseSSLSocketImpl.java, the package of SocketChannel in > the getChannel() spec is java.nio, which is incorrect. It should be > java.nio.channels. > > Doc cleanup only, no new regression test. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8257724 Marked as reviewed by wetmore (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1605
Re: RFR: 8257724: Incorrect package of the linked class in BaseSSLSocketImpl
On Thu, 3 Dec 2020 20:44:46 GMT, Xue-Lei Andrew Fan wrote: > In sun.security.ssl.BaseSSLSocketImpl.java, the package of SocketChannel in > the getChannel() spec is java.nio, which is incorrect. It should be > java.nio.channels. > > Doc cleanup only, no new regression test. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8257724 Marked as reviewed by valeriep (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1605
Integrated: 8257724: Incorrect package of the linked class in BaseSSLSocketImpl
On Thu, 3 Dec 2020 20:44:46 GMT, Xue-Lei Andrew Fan wrote: > In sun.security.ssl.BaseSSLSocketImpl.java, the package of SocketChannel in > the getChannel() spec is java.nio, which is incorrect. It should be > java.nio.channels. > > Doc cleanup only, no new regression test. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8257724 This pull request has now been integrated. Changeset: fcc84795 Author:Xue-Lei Andrew Fan URL: https://git.openjdk.java.net/jdk/commit/fcc84795 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod 8257724: Incorrect package of the linked class in BaseSSLSocketImpl Reviewed-by: valeriep, wetmore - PR: https://git.openjdk.java.net/jdk/pull/1605
Re: RFR: 8257725: No throws of SSLHandshakeException [v2]
> In the StatusResponseManager.get() method spec, the SSLHandshakeException is
> declared as throws exception. However, no such checked-exception would be
> thrown in the method implementation.
>
> /**
> ...
> * @throws SSLHandshakeException if an unsupported
> * {@code CertStatusRequest} is provided.
> */
> Map get(CertStatusRequestType type,
> CertStatusRequest request, X509Certificate[] chain, long delay,
> TimeUnit unit) {
>
> As the exception is a checked-exception, and is not declared in the method,
> it is safe to remove the throws spec.
>
> Cleanup only, no new regression test.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8257725
Xue-Lei Andrew Fan has updated the pull request with a new target base due to a
merge or a rebase. The incremental webrev excludes the unrelated changes
brought in by the merge/rebase. The pull request contains three additional
commits since the last revision:
- Merge
- 8257725: No throws of SSLHandshakeException
- 8257724: Incorrect package of the linked class in BaseSSLSocketImpl
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/1607/files
- new: https://git.openjdk.java.net/jdk/pull/1607/files/13d84784..b5fc532f
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1607&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1607&range=00-01
Stats: 2303 lines in 75 files changed: 1713 ins; 207 del; 383 mod
Patch: https://git.openjdk.java.net/jdk/pull/1607.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/1607/head:pull/1607
PR: https://git.openjdk.java.net/jdk/pull/1607
Integrated: 8257725: No throws of SSLHandshakeException
On Thu, 3 Dec 2020 20:58:55 GMT, Xue-Lei Andrew Fan wrote:
> In the StatusResponseManager.get() method spec, the SSLHandshakeException is
> declared as throws exception. However, no such checked-exception would be
> thrown in the method implementation.
>
> /**
> ...
> * @throws SSLHandshakeException if an unsupported
> * {@code CertStatusRequest} is provided.
> */
> Map get(CertStatusRequestType type,
> CertStatusRequest request, X509Certificate[] chain, long delay,
> TimeUnit unit) {
>
> As the exception is a checked-exception, and is not declared in the method,
> it is safe to remove the throws spec.
>
> Cleanup only, no new regression test.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8257725
This pull request has now been integrated.
Changeset: d76039d3
Author:Xue-Lei Andrew Fan
URL: https://git.openjdk.java.net/jdk/commit/d76039d3
Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod
8257725: No throws of SSLHandshakeException
Reviewed-by: jnimeh
-
PR: https://git.openjdk.java.net/jdk/pull/1607
RFR: 8257788: Class fields could be local in the SunJSSE provider
In the SunJSSE provider implementation, there are a few class fields are not used other than the constructors. Those fields could be removed and replaced with local variables. Bug: https://bugs.openjdk.java.net/browse/JDK-8257788 - Commit messages: - 8257788: Class fields could be local in the SunJSSE provider Changes: https://git.openjdk.java.net/jdk/pull/1634/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1634&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8257788 Stats: 15 lines in 4 files changed: 0 ins; 11 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1634.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1634/head:pull/1634 PR: https://git.openjdk.java.net/jdk/pull/1634
Re: RFR: 8257788: Class fields could be local in the SunJSSE provider
On Fri, 4 Dec 2020 22:58:07 GMT, Xue-Lei Andrew Fan wrote: > In the SunJSSE provider implementation, there are a few class fields are not > used other than the constructors. Those fields could be removed and replaced > with local variables. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8257788 Looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/1634
Re: RFR: 8242332: Add SHA3 support to SunPKCS11 provider [v2]
> Could someone please help review this RFE? SunPKCS11 provider is updated with > SHA-3 support, including MessageDigest, Hmac Mac, DSA/RSA/RSASSA-PSS/ECDSA > Signature, and Hmac KeyGenerator. > > As SHA-3 can be used as drop-in replacement for SHA-2 which are already > supported by SunPKCS11 provider, the changes for MessageDigest, Mac, and > Signature are straightforward. P11KeyGenerator class is updated to support > general Hmac key generation including SHA-3 and more. > > While testing against NSS 3.57, there are some unexpected NSS errors with > CKM_ECDSA_SHA[224/256/384/512/3_224/3_256/3_384/3_512] and > CKM_DSA_SHA[224/256/384/512/3_224/3_256/3_384/3_512], so I disabled those > mechanisms in the NSS config file for regression tests. For ECDSA signatures, > SunPKCS11 provider will fallback to CKM_ECDSA and do the digesting ourselves. > > Thanks, > Valerie Valerie Peng has updated the pull request incrementally with one additional commit since the last revision: Removed commented out code and minor cleanups on 2 RSASSA-PSS-related tests - Changes: - all: https://git.openjdk.java.net/jdk/pull/1546/files - new: https://git.openjdk.java.net/jdk/pull/1546/files/7b0d7882..4f7488b6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1546&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1546&range=00-01 Stats: 5 lines in 2 files changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1546.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1546/head:pull/1546 PR: https://git.openjdk.java.net/jdk/pull/1546
