Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v2]

[Kevin Rushforth]

On Thu, 26 May 2022 23:05:32 GMT, Joe Darcy  wrote:

>> Time to start getting ready for JDK 20...
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to review feedback.

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk/pull/8236

Re: RFR: JDK-8284858: Start of release updates for JDK 20

[Kevin Rushforth]

On Thu, 14 Apr 2022 05:09:14 GMT, Joe Darcy  wrote:

> Time to start getting ready for JDK 20...

You also need to change the JBS version from 19 to 20 in 
[`.jcheck/conf`](https://github.com/openjdk/jdk/blob/6a33974a6b8a629744c6d76c3b4fa1f772e52ac8/.jcheck/conf#L4):

-

PR: https://git.openjdk.java.net/jdk/pull/8236

Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe [v2]

[Kevin Rushforth]

On Thu, 12 May 2022 04:15:50 GMT, Alexander Matveev  
wrote:

>> - It is not possible to support native JDK commands such as "java" inside 
>> Mac App Store bundles due to embedded info.plist. Workarounds suggested in 
>> JDK-8286122 does not seems to be visible.
>>  - With proposed fix we will enforce "--strip-native-commands" option for 
>> jlink, so native JDK commands are not included when generating Mac App Store 
>> bundles.
>>  - Custom runtime provided via --runtime-image should not contain native 
>> commands as well, otherwise jpackage will throw error.
>>  - Added two tests to validate fix.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8286122: [macos]: App bundle cannot upload to Mac App Store due to 
> info.plist embedded in java exe [v2]

I'm just catching up with this thread. Based on the analysis, it's pretty clear 
that any solution that actually allows bundling native launchers is going to 
take more research, and is out of scope for this bug. Some combination of 
[JDK-8286850](https://bugs.openjdk.java.net/browse/JDK-8286850) and/or 
providing Java launchers that don't have an `Info.plist` is likely needed to 
provide a complete solution.

Given that, I think that this PR seems a reasonable fix to avoid creating an 
app bundle that can't be uploaded to the app store.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/8666

Re: RFR: 8284444: Sting typo [v3]

[Kevin Rushforth]

On Wed, 6 Apr 2022 14:12:49 GMT, Daniel Jeliński  wrote:

>> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources.properties
>>  line 63:
>> 
>>> 61: message.creating-association-with-null-extension=Creating association 
>>> with null extension.
>>> 62: message.wrong-tool-version=Detected [{0}] version {1} but version {2} 
>>> is required.
>>> 63: message.version-string-too-many-components=Version string may have up 
>>> to 3 components - major.minor.build .
>> 
>> I wonder whether the space before the period is required at the end of the 
>> sentence. Perhaps, it's to separate a property name from the end of the 
>> sentence.
>
> right; without the space the period would appear to be part of the version 
> pattern.

Yes, I believe this is why it was done.

-

PR: https://git.openjdk.java.net/jdk/pull/8125

Re: RFR: 8284444: Sting typo [v3]

[Kevin Rushforth]

On Wed, 6 Apr 2022 16:47:17 GMT, Daniel Jeliński  wrote:

>> This patch adds missing `r` in `string`s
>
> Daniel Jeliński has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - revert xalan changes
>  - revert icu changes

The `jpackage` part of the fix looks good.  Btw, there are translated files for 
3 other languages (German, Japanese, Simplified Chinese), so I double-checked 
the translation for each, and they already say "string" (I guess whoever 
translated it just assumed it was meant to be "string" when they took the 
English phrase).

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/8125

Re: RFR: 8284444: Sting typo

[Kevin Rushforth]

On Wed, 6 Apr 2022 12:07:30 GMT, Daniel Jeliński  wrote:

> This patch adds missing `r` in `string`s

This PR cuts across many areas, so will need multiple reviewers. Regarding the 
LCMS file, we typically don't make these kind of changes in third-party code, 
since it will cause our code to diverge from the upstream code, which can lead 
to merge conflicts down the road. @prrace will need to approve this or else you 
will need to revert that file.

-

PR: https://git.openjdk.java.net/jdk/pull/8125

Re: RFR: 8283805: [REDO] JDK 19 L10n resource files update - msgdrop 10

[Kevin Rushforth]

On Wed, 30 Mar 2022 00:43:49 GMT, Alisen Chung  wrote:

> redo of 8280400

Since this is identical to the original fix, I would expect the same Tier2 test 
failure as reported in 
[JDK-8283804](https://bugs.openjdk.java.net/browse/JDK-8283804).

-

PR: https://git.openjdk.java.net/jdk/pull/8026

Re: RFR: 8283806: [BACKOUT] JDK 19 L10n resource files update - msgdrop 10

[Kevin Rushforth]

On Mon, 28 Mar 2022 21:20:00 GMT, Alisen Chung  wrote:

> This reverts commit c0aecd15ae8d7abf37901f785fccaff2317c3b23.

I confirm that this is an exact backout of 
[JDK-8280400](https://bugs.openjdk.java.net/browse/JDK-8280400).

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/8005

Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines

[Kevin Rushforth]

On Mon, 7 Mar 2022 16:40:15 GMT, Lance Andersen  wrote:

> What problem are you having editing the PR header? You should be able to do 
> so as the author of the PR

Exactly. You should see an "Edit" button near the right edge of the PR title. 
See the attached image:

![PR-title](https://user-images.githubusercontent.com/34689748/157079404-eadbe8be-ae94-41e0-b17b-0d1a8026b9da.png)

-

PR: https://git.openjdk.java.net/jdk/pull/7268

Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines

[Kevin Rushforth]

On Mon, 7 Mar 2022 17:12:25 GMT, Magnus Ihse Bursie  wrote:

> TheShermanTanker is not the author of this PR, he's just assisting the author 
> by creating the JBS issue.

Ah, that explains it then.

-

PR: https://git.openjdk.java.net/jdk/pull/7268

Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines

[Kevin Rushforth]

On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan  wrote:

> Hi
> 
> I have reviewed the code for removing double semicolons at the end of lines
> 
> all the best
> matteo

But as the JBS title and PR title now match, this is a moot point.

-

PR: https://git.openjdk.java.net/jdk/pull/7268

Re: RFR: 8282551: Properly initialize L32X64MixRandom state

[Kevin Rushforth]

On Wed, 2 Mar 2022 15:26:35 GMT, Devin Smith  wrote:

> Thanks for the help. I signed the OCA on Jan 17, 2022 and got confirmation it 
> was approved on Jan 20, 2022.

Correct. Reviewers may know that this has been recorded if the PR has no `oca` 
label (you can see above that the `oca` label was removed on Jan 20).

-

PR: https://git.openjdk.java.net/jdk/pull/7120

Re: RFR: 8279995: jpackage --add-launcher option should allow overriding description

[Kevin Rushforth]

On Wed, 9 Feb 2022 07:37:42 GMT, Alexander Matveev  wrote:

> Added ability to override description for additional launchers via 
> "description" property.

This will need a CSR.

-

PR: https://git.openjdk.java.net/jdk/pull/7399

Re: [jdk18] RFR: 8279833: Loop optimization issue in String.encodeUTF8_UTF16

[Kevin Rushforth]

On Thu, 13 Jan 2022 15:23:25 GMT, Claes Redestad  wrote:

> I've taken up the (bad?) habit of creating it manually to keep a tab on my 
> current tasks and intents.

I do that too in some cases, and for the same reason. The only potential 
downside is if you create a concrete version for a specific update release and 
it misses that one and hits the next one, but for that case you can use 
`NN-pool` as the fixversion on the backport (and Skara will do the right thing).

-

PR: https://git.openjdk.java.net/jdk18/pull/99

Re: [jdk18] RFR: 8279833: Loop optimization issue in String.encodeUTF8_UTF16

[Kevin Rushforth]

On Thu, 13 Jan 2022 15:05:42 GMT, Claes Redestad  wrote:

> > (This a backport but is using the mainline bug number; already resolved).
> 
> I think that's intentional? I entered "Backport COMMITHASH" as the PR subject 
> as per instructions and the bots have taken things from there.

Yes, it's both intentional and necessary. We only use the JBS bug ID of the 
main bug, and never the JBS ID of the backport record (presuming the latter is 
even created ahead of time, which it usually isn't) in pull requests, commit 
messages, etc.

-

PR: https://git.openjdk.java.net/jdk18/pull/99

Re: RFR: 8268081: Upgrade Unicode Data Files to 14.0.0

[Kevin Rushforth]

On Wed, 5 Jan 2022 22:42:38 GMT, Naoto Sato  wrote:

> Please review the changes for upgrading the Unicode support in the JDK, from 
> version 13 to version 14. Corresponding CSR has also been drafted.

The CSR is now approved. This comment should be sufficient to wake up the bot.

-

PR: https://git.openjdk.java.net/jdk/pull/6974

Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

[Kevin Rushforth]

On Tue, 7 Dec 2021 13:27:44 GMT, Alan Bateman  wrote:

>> @jddarcy hi Joe, what's the next step with the CSR now? 
>> https://bugs.openjdk.java.net/browse/JDK-8277755
>> thanks
>
>> @jddarcy hi Joe, what's the next step with the CSR now? 
>> https://bugs.openjdk.java.net/browse/JDK-8277755
>> thanks
> 
> For the CSR then you need to press "Finalize".

@AlanBateman will need to review your request.

-

PR: https://git.openjdk.java.net/jdk/pull/6481

Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

[Kevin Rushforth]

On Fri, 10 Dec 2021 17:01:41 GMT, Andrew Leonard  wrote:

>> Looks like the CSR has been approved.  I have a mach5 run that should 
>> hopefully finish sooner rather than later and if that remains happy, I will 
>> approve the PR
>
> @LanceAndersen let me know if mach5 looks good please, then I think we're 
> good to go..

@andrew-m-leonard if this enhancement is now intended to go into JDK 19, then 
you can simply integrate it into jdk mainline when it is ready. In that case, 
the fix version of the [CSR](https://bugs.openjdk.java.net/browse/JDK-8277755) 
should be adjusted to 19 to match.

If, however, you would still like to get it into JDK 18, you will need to make 
a late enhancement request per [JEP 3](https://openjdk.java.net/jeps/3), and 
then recreate this PR against the jdk18 stabilization repo.

-

PR: https://git.openjdk.java.net/jdk/pull/6481

Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v8]

[Kevin Rushforth]

On Tue, 7 Dec 2021 10:42:50 GMT, Roman Kennke  wrote:

>> The caches in ObjectStreamClass basically map WeakReference to 
>> SoftReference, where the ObjectStreamClass also 
>> references the same Class. That means that the cache entry, and thus the 
>> class and its class-loader, will not get reclaimed, unless the GC determines 
>> that memory pressure is very high.
>> 
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
>> all its classes alive much longer than necessary: as soon as a ClassLoader 
>> (and all its classes) become unreachable, there is no point in retaining the 
>> stuff in OSC's caches.
>> 
>> The proposed change is to use WeakReference instead of SoftReference for the 
>> values in caches.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [ ] tier4
>
> Roman Kennke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add improved test by @plevart

This is a P4 bug. If the priority is correct, it does not meet the criteria to 
get it into JDK 18 during RDP1, as indicated in [JEP 
3](https://openjdk.java.net/jeps/3).

If this is objectively a P3 bug, and really does need to go into JDK 18, then 
you will need to close this PR and open a new pull request in the jdk18 repo.

-

PR: https://git.openjdk.java.net/jdk/pull/6375

Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

[Kevin Rushforth]

On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which adds `jlink.debug=true` system 
> property while launching `jpackage` tests?
> 
> The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
> take into account the part where the `jpackage` tool gets launched as a 
> `ToolProvider` from some of these tests. This resulted in a large number of 
> tests failing (across different OS) in `tier2` with errors like:
> 
> 
> Error: Invalid Option: [-J-Djlink.debug=true]
> 
> 
> In this current PR, the changed code now takes into account the possibility 
> of `jpackage` being launched as a `ToolProvider` and in such cases doesn't 
> add this option. To achieve this, the adding of this argument is delayed till 
> when the actual execution is about to happen and thus it's now done in the 
> `adjustArgumentsBeforeExecution()` method of the jpackage test framework.
> 
> With this change, I have now run the `jdk:tier2` locally on a macos instance 
> and the tests have all passed:
> 
> 
> Test results: passed: 3,821; failed: 3
> Report written to 
> jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
> Results written to 
> jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
> Error: Some tests failed or other problems occurred.
> Finished running test 'jtreg:test/jdk:tier2'
> Test report is stored in 
> build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier2   3824  3821 3 0 <<
> ==
> 
> The 3 failing tests are unrelated to this change and belong to the 
> `java/nio/channels/DatagramChannel/` test package.
> Furthermore, I've looked into the generated logs of the following tests to 
> verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
> doesn't in those that failed previously in `tier2`:
> 
> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
> test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
> test/jdk/tools/jpackage/share/ArgumentsTest.java
> 
> A sample from one of the logs where this system property is expected to be 
> passed along:
> 
>> TRACE: exec: Execute 
>> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input 
>> ./test/input --dest ./test/output --name "Name With Space" --type pkg 
>> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); 
>> inherit I/O...
> 
> 
> I would still like to request someone with access to CI or other OSes (like 
> Windows and Linux) to help test `tier2` against this PR.

@sashamatveev should be able to run the tier2 tests for you as part of his 
(re)reviewing the fix.

-

PR: https://git.openjdk.java.net/jdk/pull/6534

Re: [External] : Re: jpackage - notarizing

[Kevin Rushforth]

By way of clarification, JNF was never part of the JDK. What we removed 
was the JDK's *usage* of JNF. And we did so because Apple deprecated it 
and never even provide a JNF framework for aarch64.


Applications either need to migrate off JNF and find an alternative, or 
take Apple's open-source project, build it yourself, and include it with 
your application. Either way, this isn't a JDK problem.


-- Kevin


On 11/22/2021 11:56 AM, Michael Hall wrote:



On Nov 22, 2021, at 12:39 PM, Alan Snyder  wrote:



On Nov 22, 2021, at 10:12 AM, Kevin Rushforth  
wrote:

JNF was removed from the JDK if that's what you are asking.



Indeed, that is why there is an issue.

The JDK may not be using JNF, but library developers still use it.

The JDK replacement for JNF is not supported for use outside the JDK.

JNF is not just convenient, there is at least one essential use for which there 
is no adequate replacement.

See https://bugs.openjdk.java.net/browse/JDK-8274596




I have a fair amount of code that would break if some JNF option wasn’t 
available. I have no estimate for how much rewriting I would need to do. Two of 
the projects I needed to recompile to get notarized needed it. I don’t actively 
do anything with it anymore but have some older code that did.

Re: jpackage - notarizing

[Kevin Rushforth]

JNF was removed from the JDK if that's what you are asking.

-- Kevin


On 11/22/2021 10:11 AM, Alan Snyder wrote:

Well, if you look at the JNF part of the tree, the only include I see that 
comes from the JDK is jni.h.

Isn’t that what you were questioning?





On Nov 22, 2021, at 10:06 AM, Michael Hall  wrote:




On Nov 22, 2021, at 11:59 AM, Alan Snyder mailto:javali...@cbfiddle.com>> wrote:

I’m talking about the Xcode project for JNF.


I thought I was too. Unless it’s changed since I cloned or the one I’m looking at is 
somehow different. The URL I saved was  https://github.com/apple/openjdk/ 
 which looks to build JNF. Not that much a 
concern as of yet if the SDK still has it.

Thanks again.

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v4]

[Kevin Rushforth]

On Tue, 16 Nov 2021 12:48:03 GMT, kabutz  wrote:

>> BigInteger currently uses three different algorithms for multiply. The 
>> simple quadratic algorithm, then the slightly better Karatsuba if we exceed 
>> a bit count and then Toom Cook 3 once we go into the several thousands of 
>> bits. Since Toom Cook 3 is a recursive algorithm, it is trivial to 
>> parallelize it. I have demonstrated this several times in conference talks. 
>> In order to be consistent with other classes such as Arrays and Collection, 
>> I have added a parallelMultiply() method. Internally we have added a 
>> parameter to the private multiply method to indicate whether the calculation 
>> should be done in parallel.
>> 
>> The performance improvements are as should be expected. Fibonacci of 100 
>> million (using a single-threaded Dijkstra's sum of squares version) 
>> completes in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with 
>> the sequential multiply() method. This is on my 1-8-2 laptop. The final 
>> multiplications are with very large numbers, which then benefit from the 
>> parallelization of Toom-Cook 3.  Fibonacci 100 million is a 347084 bit 
>> number.
>> 
>> We have also parallelized the private square() method. Internally, the 
>> square() method defaults to be sequential.
>> 
>> 
>> Benchmark  (n)  Mode  Cnt  Score 
>>  Error  Units
>> BigIntegerParallelMultiply.multiply100ss4 68,043 
>> ±   25,317  ms/op
>> BigIntegerParallelMultiply.multiply   1000ss4   1073,095 
>> ±  125,296  ms/op
>> BigIntegerParallelMultiply.multiply  1ss4  25317,535 
>> ± 5806,205  ms/op
>> BigIntegerParallelMultiply.parallelMultiply100ss4 56,552 
>> ±   22,368  ms/op
>> BigIntegerParallelMultiply.parallelMultiply   1000ss4536,193 
>> ±   37,393  ms/op
>> BigIntegerParallelMultiply.parallelMultiply  1ss4   9274,657 
>> ±  826,197  ms/op
>
> kabutz has updated the pull request with a new target base due to a merge or 
> a rebase. The pull request now contains four commits:
> 
>  - Update comments
>  - Added parallelMultiply() method to BigInteger to allow large 
> multiplications to run in parallel
>  - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control 
> points in bounding box
>
>Addressing some of Laurent's code review recommendations/comments:
>
>1. use the convention t for the parametric variable x(t),y(t)
>2. solve the quadratic equation using QuadCurve2d.solveQuadratic() or like 
> Helpers.quadraticRoots()
>3. always use braces for x = (a < b) ? ...
>4. always use double-precision constants in math or logical operations: (2 
> * x => 2.0 * x) and (coefficients[3] != 0) => (coefficients[3] != 0.0)
>
>(There are two additional recommendations not in this commit that I'll ask 
> about shortly.)
>
>See https://github.com/openjdk/jdk/pull/6227#issuecomment-959757954
>  - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control 
> points in bounding box
>
>The bug writeup indicated they wanted Path2D#getBounds2D() to be more 
> accurate/concise. They didn't explicitly say they wanted CubicCurve2D and 
> QuadCurve2D to become more accurate too. But a preexisting unit test failed 
> when Path2D#getBounds2D() was updated and those other classes weren't. At 
> this point I considered either:
>A. Updating CubicCurve2D and QuadCurve2D to use the new more accurate 
> getBounds2D() or
>B. Updating the unit test to forgive the discrepancy.
>
>I chose A. Which might technically be seen as scope creep, but it feels 
> like a more holistic/better approach.
>
>This also includes a new unit test (in Path2D/UnitTest.java) that fails 
> without the changes in this commit.

You still have the extra unwanted commits in your branch.

Normally you should not force push to your branch once a review is started, but 
in this case the only other choice would be to abandon this PR and create a new 
one.

Regardless of whether you close this PR and create a new one or continue with 
this PR, you should rebase your branch on top of the latest jdk master such 
that there is only a single commit with the changes you are proposing. It 
should _not_ touch any files from `java.desktop`.

-

PR: https://git.openjdk.java.net/jdk/pull/6391

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger

[Kevin Rushforth]

On Mon, 15 Nov 2021 15:50:39 GMT, kabutz  wrote:

> BigInteger currently uses three different algorithms for multiply. The simple 
> quadratic algorithm, then the slightly better Karatsuba if we exceed a bit 
> count and then Toom Cook 3 once we go into the several thousands of bits. 
> Since Toom Cook 3 is a recursive algorithm, it is trivial to parallelize it. 
> I have demonstrated this several times in conference talks. In order to be 
> consistent with other classes such as Arrays and Collection, I have added a 
> parallelMultiply() method. Internally we have added a parameter to the 
> private multiply method to indicate whether the calculation should be done in 
> parallel.
> 
> The performance improvements are as should be expected. Fibonacci of 100 
> million (using a single-threaded Dijkstra's sum of squares version) completes 
> in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with the 
> sequential multiply() method. This is on my 1-8-2 laptop. The final 
> multiplications are with very large numbers, which then benefit from the 
> parallelization of Toom-Cook 3.  Fibonacci 100 million is a 347084 bit number.
> 
> We have also parallelized the private square() method. Internally, the 
> square() method defaults to be sequential.
> 
> 
> Benchmark  (n)  Mode  Cnt  Score  
> Error  Units
> BigIntegerParallelMultiply.multiply100ss4 68,043 
> ±   25,317  ms/op
> BigIntegerParallelMultiply.multiply   1000ss4   1073,095 
> ±  125,296  ms/op
> BigIntegerParallelMultiply.multiply  1ss4  25317,535 
> ± 5806,205  ms/op
> BigIntegerParallelMultiply.parallelMultiply100ss4 56,552 
> ±   22,368  ms/op
> BigIntegerParallelMultiply.parallelMultiply   1000ss4536,193 
> ±   37,393  ms/op
> BigIntegerParallelMultiply.parallelMultiply  1ss4   9274,657 
> ±  826,197  ms/op

Additionally, you should probably discuss new functionality such as this on the 
appropriate mailing list, `core-libs-dev at openjdk.java.net` in this case.

Finally, this will need a CSR if it is to proceed.

-

PR: https://git.openjdk.java.net/jdk/pull/6391

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger

[Kevin Rushforth]

On Mon, 15 Nov 2021 15:50:39 GMT, kabutz  wrote:

> BigInteger currently uses three different algorithms for multiply. The simple 
> quadratic algorithm, then the slightly better Karatsuba if we exceed a bit 
> count and then Toom Cook 3 once we go into the several thousands of bits. 
> Since Toom Cook 3 is a recursive algorithm, it is trivial to parallelize it. 
> I have demonstrated this several times in conference talks. In order to be 
> consistent with other classes such as Arrays and Collection, I have added a 
> parallelMultiply() method. Internally we have added a parameter to the 
> private multiply method to indicate whether the calculation should be done in 
> parallel.
> 
> The performance improvements are as should be expected. Fibonacci of 100 
> million (using a single-threaded Dijkstra's sum of squares version) completes 
> in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with the 
> sequential multiply() method. This is on my 1-8-2 laptop. The final 
> multiplications are with very large numbers, which then benefit from the 
> parallelization of Toom-Cook 3.  Fibonacci 100 million is a 347084 bit number.
> 
> We have also parallelized the private square() method. Internally, the 
> square() method defaults to be sequential.
> 
> 
> Benchmark  (n)  Mode  Cnt  Score  
> Error  Units
> BigIntegerParallelMultiply.multiply100ss4 68,043 
> ±   25,317  ms/op
> BigIntegerParallelMultiply.multiply   1000ss4   1073,095 
> ±  125,296  ms/op
> BigIntegerParallelMultiply.multiply  1ss4  25317,535 
> ± 5806,205  ms/op
> BigIntegerParallelMultiply.parallelMultiply100ss4 56,552 
> ±   22,368  ms/op
> BigIntegerParallelMultiply.parallelMultiply   1000ss4536,193 
> ±   37,393  ms/op
> BigIntegerParallelMultiply.parallelMultiply  1ss4   9274,657 
> ±  826,197  ms/op

This PR mistakenly includes commits from another in-progress PR.

You need to remove these stray commits or else close this PR, create a new 
branch without them, and resubmit.

-

Changes requested by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/6391

Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Kevin Rushforth]

I've made an
application image with jpackage.  I've moved the default location of the
runtime in the application image
...
If I keep the runtime in $APPDIR\runtime, things don't fail in this way.


If it works when using the default location of the Java runtime and no 
other changes, I'd guess that this is a bug in the jpackage app 
launcher, or maybe in the code that creates the runtime (e.g., if 
something isn't copied to the right place when not in the default 
location). Hopefully Andy or Alexey can comment.


-- Kevin


On 9/20/2021 2:37 PM, Scott Palmer wrote:

This is likely not the right place to ask this (sorry).. but I'm trying to
get info to write a bug report and want to make sure I'm not doing
something stupid first.

I've created a JRE image from JDK 17 with jlink.  I've made an
application image with jpackage.  I've moved the default location of the
runtime in the application image and modified the launcher .cfg file
accordingly by adding a line to the [Application] section

app.runtime=$APPDIR\..\my_own_folder\where_the_jre_is_deeper\jre

My application launches.  It shows a JavaFX GUI.  It does a bunch of stuff
that "works", but then one thread fails with this exception:

Exception in thread "Reader-BG-1" java.lang.InternalError: platform
encoding not initialized
 at java.base/java.net.Inet6AddressImpl.lookupAllHostAddr(Native
Method)
 at
java.base/java.net.InetAddress$PlatformNameService.lookupAllHostAddr(InetAddress.java:933)
 at
java.base/java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1519)
 at
java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:852)
 at
java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1509)
 at
java.base/java.net.InetAddress.getAllByName(InetAddress.java:1367)
 at
java.base/java.net.InetAddress.getAllByName(InetAddress.java:1301)
 at java.base/java.net.InetAddress.getByName(InetAddress.java:1251)
 at
java.base/java.net.InetSocketAddress.(InetSocketAddress.java:229)
 at java.base/sun.net.NetworkClient.doConnect(NetworkClient.java:178)
 at
java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:498)
 at
java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:603)
 at
java.base/sun.net.www.protocol.https.HttpsClient.(HttpsClient.java:266)
 at
java.base/sun.net.www.protocol.https.HttpsClient.New(HttpsClient.java:380)
 at
java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.getNewHttpClient(AbstractDelegateHttpsURLConnection.java:189)
 at
java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1242)
 at
java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:1128)
 at
java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:175)
 at
java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1665)
 at
java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1589)
 at
java.base/java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:529)
 at
java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:308)

If I keep the runtime in $APPDIR\runtime, things don't fail in this way.

Smells like a bug in the app launcher to me.. but maybe it's in the runtime?

Any assistance would be appreciated (including telling me where I should go
to ask this, if this list isn't appropriate).

Thanks,

Scott

Re: [jdk17] RFR: JDK-8273592: Backout JDK-8271868

[Kevin Rushforth]

On Fri, 10 Sep 2021 13:18:49 GMT, Andy Herrick  wrote:

> JDK-8271868 was pushed to JDK17 instead of jdk17u.
> This change is to back it out

I fetched the PR commit and confirmed that it is a correct backout (revert) of 
JDK-8271868.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk17/pull/307

Re: [jdk17] RFR: JDK-8272639: jpackaged applications using microphone on mac

[Kevin Rushforth]

On Thu, 9 Sep 2021 20:14:01 GMT, Andy Herrick  wrote:

> backport from jdk-18

This must _not_ be integrated into this repo.

See [JDK-8273592](https://bugs.openjdk.java.net/browse/JDK-8273592) for an 
explanation as to why. There is no more content approved for JDK 17. This 
should go into [openjdk/jdk17u](https://github.com/openjdk/jdk17u).

-

PR: https://git.openjdk.java.net/jdk17/pull/306

[jdk17] Withdrawn: JDK-8272639: jpackaged applications using microphone on mac

[Kevin Rushforth]

On Thu, 9 Sep 2021 20:14:01 GMT, Andy Herrick  wrote:

> backport from jdk-18

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk17/pull/306

Re: RFR: 8273140: Replace usages of Enum.class.getEnumConstants() with Enum.values() where possible [v3]

[Kevin Rushforth]

On Mon, 30 Aug 2021 19:23:57 GMT, Сергей Цыпанов 
 wrote:

>> Just a very tiny clean-up.
>> 
>> There are some places in JDK code base where we call 
>> `Enum.class.getEnumConstants()` to get all the values of the referenced 
>> `enum`. This is excessive, less-readable and slower than just calling 
>> `Enum.values()` as in `getEnumConstants()` we have volatile access:
>> 
>> public T[] getEnumConstants() {
>> T[] values = getEnumConstantsShared();
>> return (values != null) ? values.clone() : null;
>> }
>> 
>> private transient volatile T[] enumConstants;
>> 
>> T[] getEnumConstantsShared() {
>> T[] constants = enumConstants;
>> if (constants == null) { /* ... */ }
>> return constants;
>> }
>> 
>> Calling values() method is slightly faster:
>> 
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class EnumBenchmark {
>> 
>>   @Benchmark
>>   public Enum[] values() {
>> return Enum.values();
>>   }
>> 
>>   @Benchmark
>>   public Enum[] getEnumConstants() {
>> return Enum.class.getEnumConstants();
>>   }
>> 
>>   private enum Enum {
>> A,
>> B
>>   }
>> }
>> 
>> 
>> BenchmarkMode  Cnt   
>>   Score Error   Units
>> EnumBenchmark.getEnumConstants   avgt   15   
>>   6,265 ±   0,051   ns/op
>> EnumBenchmark.getEnumConstants:·gc.alloc.rateavgt   15  
>> 2434,075 ±  19,568  MB/sec
>> EnumBenchmark.getEnumConstants:·gc.alloc.rate.norm   avgt   15   
>>  24,002 ±   0,001B/op
>> EnumBenchmark.getEnumConstants:·gc.churn.G1_Eden_Space   avgt   15  
>> 2433,709 ±  70,216  MB/sec
>> EnumBenchmark.getEnumConstants:·gc.churn.G1_Eden_Space.norm  avgt   15   
>>  23,998 ±   0,659B/op
>> EnumBenchmark.getEnumConstants:·gc.churn.G1_Survivor_Space   avgt   15   
>>   0,009 ±   0,003  MB/sec
>> EnumBenchmark.getEnumConstants:·gc.churn.G1_Survivor_Space.norm  avgt   15   
>>  ≈ 10⁻⁴  B/op
>> EnumBenchmark.getEnumConstants:·gc.count avgt   15   
>> 210,000counts
>> EnumBenchmark.getEnumConstants:·gc.time  avgt   15   
>> 119,000ms
>> 
>> EnumBenchmark.values avgt   15   
>>   4,164 ±   0,134   ns/op
>> EnumBenchmark.values:·gc.alloc.rate  avgt   15  
>> 3665,341 ± 120,721  MB/sec
>> EnumBenchmark.values:·gc.alloc.rate.norm avgt   15   
>>  24,002 ±   0,001B/op
>> EnumBenchmark.values:·gc.churn.G1_Eden_Space avgt   15  
>> 3660,512 ± 137,250  MB/sec
>> EnumBenchmark.values:·gc.churn.G1_Eden_Space.normavgt   15   
>>  23,972 ±   0,529B/op
>> EnumBenchmark.values:·gc.churn.G1_Survivor_Space avgt   15   
>>   0,017 ±   0,003  MB/sec
>> EnumBenchmark.values:·gc.churn.G1_Survivor_Space.normavgt   15   
>>  ≈ 10⁻⁴  B/op
>> EnumBenchmark.values:·gc.count   avgt   15   
>> 262,000counts
>> EnumBenchmark.values:·gc.timeavgt   15   
>> 155,000ms
>
> Сергей Цыпанов has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8273140: Fix copyright year

You missed two of the copyright year problems.

src/java.desktop/share/classes/sun/font/EAttribute.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.

You need to also fix this copyright year. It must be `2005, 2021,`

src/java.sql/share/classes/java/sql/JDBCType.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.

This one, too.

-

PR: https://git.openjdk.java.net/jdk/pull/5303

Withdrawn: 8272137: Make Collection and Optional classes streamable

[Kevin Rushforth]

On Mon, 9 Aug 2021 12:28:23 GMT, CC007  
wrote:

> create Streamable and ParallelStreamable interface and use them in Collection 
> and Optional

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/5050

Re: RFR: 8272137: Make Collection and Optional classes streamable

[Kevin Rushforth]

On Sun, 15 Aug 2021 02:45:17 GMT, CC007  
wrote:

>> create Streamable and ParallelStreamable interface and use them in 
>> Collection and Optional
>
> I read through [these 
> posts](http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-June/thread.html#1910)
>  and while I did see good reasons for not moving stream to Iterable, I didn't 
> see any mention of a separate Streamable-like interface, so could you 
> elaborate on that it "did not carry its weight"?

@CC007 Please follow the course of action that @briangoetz has requested. This 
needs to be discussed on the appropriate mailing list (core-libs-dev in this 
case) prior to submitting a pull request. It is premature to review this PR.

I invite you to refer to the [OpenJDK Developers' 
Guide](https://openjdk.java.net/guide/), specifically the [Socialize your 
change](https://openjdk.java.net/guide/#socialize-your-change) section.

-

PR: https://git.openjdk.java.net/jdk/pull/5050

Re: [jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

[Kevin Rushforth]

On Thu, 29 Jul 2021 19:05:58 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk17/pull/293

Re: RFR: 8271396: Spelling errors

[Kevin Rushforth]

On Wed, 28 Jul 2021 17:08:01 GMT, Emmanuel Bourg 
 wrote:

>> I'm happy to sponsor this change, but could you please update the copyright 
>> year where necessary, e.g. in 
>> src/java.desktop/unix/classes/sun/awt/X11/XMSelection.java: 
>> `Copyright (c) 2003, 2018, Oracle...` -> `Copyright (c) 2003, 2021, 
>> Oracle...`
>
> @FrauBoes thank you, the PR has been updated to modify the copyright year

@ebourg for future PRs please do not force push after the PR is out for review. 
Just push incremental commits normally. The Skara tooling will squash them all 
into a single commit.

-

PR: https://git.openjdk.java.net/jdk/pull/2385

Re: jpackage MacOS Notarization

[Kevin Rushforth]

Full support for notarization in jpackage was added in JDK 17. Can you 
try an early access build of JDK 17 [1] and see if that works for you?


-- Kevin

[1] https://jdk.java.net/17

On 7/28/2021 8:27 AM, Daniel Peintner wrote:

All,

I am trying to notarize an app (built with jpackage) for MacOS.

jpackage at first *seems* to properly sign all resources with the available
--mac-sign options et cetera.

Having said that, there are still remaining issues
1. The app cannot be properly installed
(without hacks like xattr -d com.apple.quarantine /Applications/myAPP.app
).
2. I am also not able to properly notarize the app.

According to online resources there seem to exist issues in *past* about
notarization but according to [1, 2] the issues are fixed.

As mentioned, I still have issues :-(
Am I really the only one still having problems?

Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without success)

The issue seems to boil down to 2 errors (attached the error log from Apple
notarization process).
* app Folder
* libjli.dylib

I thought I better ask first on the mailing list before creating an actual
bug report.

Note1: I used to use the *old* javapackager that worked with the same
signature/credentials.
Note2: running jpackage without --mac-sign options causes many more errors
in notarization (Hence, jpackage signs most resources but fails with some)

Any help / hint appreciated.

Thanks,

-- Daniel

[1] https://bugs.openjdk.java.net/browse/JDK-8257488
[2] https://bugs.openjdk.java.net/browse/JDK-8251892



## APPLE LOG ##

{
   "logFormatVersion": 1,
   "jobId": "...",
   "status": "Invalid",
   "statusSummary": "Archive contains critical validation errors",
   "statusCode": 4000,
   "archiveFilename": "myAPP-21.07.28.dmg",
   "uploadDate": "2021-07-28T14:31:24Z",
   "sha256": "...",
   "ticketContents": null,
   "issues": [
 {
   "severity": "error",
   "code": null,
   "path": "myAPP-21.07.28.dmg/myAPP.app/Contents/MacOS/myAPP",
   "message": "The signature of the binary is invalid.",
   "docUrl": null,
   "architecture": "x86_64"
 },
 {
   "severity": "error",
   "code": null,
   "path": 
"myAPP-21.07.28.dmg/myAPP.app/Contents/runtime/Contents/MacOS/libjli.dylib",
   "message": "The signature of the binary is invalid.",
   "docUrl": null,
   "architecture": "x86_64"
 }
   ]
}

Re: [jdk17] RFR: 8271155: Wrong path separator in env variable

[Kevin Rushforth]

On Thu, 22 Jul 2021 19:35:59 GMT, Alexey Semenyuk  wrote:

> Replace `";"` with `FileUtils::pathSeparator` in the expression adding 'app' 
> dir to env variable in jpackage app launcher.

Alexey filed [JDK-8271170](https://bugs.openjdk.java.net/browse/JDK-8271170) to 
cover this.

-

PR: https://git.openjdk.java.net/jdk17/pull/271

Re: [jdk17] RFR: 8271155: Wrong path separator in env variable

[Kevin Rushforth]

On Thu, 22 Jul 2021 19:35:59 GMT, Alexey Semenyuk  wrote:

> Replace `";"` with `FileUtils::pathSeparator` in the expression adding 'app' 
> dir to env variable in jpackage app launcher.

Looks good. Is there a unit test associated with this? If not, do you think one 
would be useful?

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk17/pull/271

Re: RFR: JDK-8269387: jpackage --add-launcher should have option to not create shortcuts for additional launchers

[Kevin Rushforth]

On Thu, 8 Jul 2021 19:25:33 GMT, Andy Herrick  wrote:

> JDK-8269387: jpackage --add-launcher should have option to not create 
> shortcuts for additional launchers

This will need a CSR (so you or someone with a Reviewer role should indicate 
this with the `/csr` command).

Also, can you summarize the changes in this PR, including the added command 
line options?

-

PR: https://git.openjdk.java.net/jdk/pull/4730

Re: RFR: 8269383: (bf) ByteOrder should expose methods to test if platform is big or little endian

[Kevin Rushforth]

On Fri, 25 Jun 2021 13:30:56 GMT, Yi Yang  wrote:

> Hi, can I have a review of this change that adds two new utility methods for 
> java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of 
> ByteOrder.nativeOrder() is to check if the underlying platform is 
> little-endian/big-endian(e.g. #4596 ). There is no reason to only provide 
> ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods 
> blank.
> 
> Thanks!

If this is accepted as a new enhancement, it will need a CSR.

Personally, I don't see enough justification for such a convenience method, but 
this isn't my area, so my opinion doesn't carry any weight.

-

PR: https://git.openjdk.java.net/jdk/pull/4595

Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException [v3]

[Kevin Rushforth]

On Thu, 10 Jun 2021 08:11:42 GMT, Rafael Winterhalter 
 wrote:

>> Please add an overview to test before integrating; thanks.
>
>> Please add an overview to test before integrating; thanks.
> 
> Sorry, I missed that one. Not sure what you mean by *overview*? I was not 
> sure if there's an official tag for something like this but added a 
> description as a comment.

@raphw Please do not force push to your branch after a PR is under review. It 
makes it difficult for reviewers to see incremental diffs.

-

PR: https://git.openjdk.java.net/jdk/pull/3940

Re: RFR: 8191441: (Process) add Readers and Writer access to java.lang.Process streams [v3]

[Kevin Rushforth]

On Mon, 24 May 2021 18:55:33 GMT, Roger Riggs  wrote:

>> src/java.base/share/classes/java/lang/Process.java line 771:
>> 
>>> 769: 
>>> 770: /**
>>> 771:  * {@return Charset for the native encoding or {@link 
>>> Charset#defaultCharset()}
>> 
>> Need some edit here?
>
> Looks odd, but is the correct syntax for the new {@return javadoc tag}.
> See https://bugs.openjdk.java.net/browse/JDK-8075778

Hmm. Unless I'm missing something, you have mismatched curly braces.

-

PR: https://git.openjdk.java.net/jdk/pull/4134

Re: RFR: JDK-8267423: Fix copyrights in jpackage tests

[Kevin Rushforth]

On Fri, 21 May 2021 12:22:16 GMT, Andy Herrick  wrote:

> JDK-8267423: Fix copyrights in jpackage tests

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk/pull/4144

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Kevin Rushforth]

On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

This isn't a review of the code changes, although I did take a quick look at 
the manual changes, and they look fine.

I did a local build of the PR branch on Windows, Mac, and Linux, and then did a 
build / test of JavaFX using that locally built JDK to find all the places 
where I need to add `-Djava.security.manager=allow` to the JavaFX tests to fix 
[JDK-8264140](https://bugs.openjdk.java.net/browse/JDK-8264140). It's working 
as expected.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: [External] : Re: Proposal to add JavaScript platform to jpackage

[Kevin Rushforth]

Regardless of whether you integrate into jpackage or provide a new 
"jspackage" tool, this presupposes the capability of running a set of 
Java class files on top of a JavaScript engine. Pointing to a 
third-party library that happens to implement this isn't sufficient to 
define this new platform. Given that you are targeting a platform / 
runtime environment that isn't part of the JDK, then I don't see why the 
packaging tool for that platform should be part of the JDK.


-- Kevin


On 5/8/2021 8:47 AM, Andrew Oliver wrote:
Much of the feedback (public and private) from this proposal has 
focused on integration with jpackage.  There is concern that jpackage 
is strictly for platforms where the OpenJDK runtime is bundled with 
the class files in an installer for distribution on that platform.  I 
have a couple of questions for this list:


1. Is it the intent that jpackage _only_ be used for platforms where 
the OpenJDK runtime is bundled, in whole or in part?  The descriptions 
from the jpackage docs mentions taking a Java run-time image as input, 
but then says it produces an application that "includes all the 
necessary dependencies": "The |jpackage| tool will take as input a 
Java application and a Java run-time image, and produce a Java 
application image that includes all the necessary dependencies. It 
will be able to produce a native package in a platform-specific 
format, such as an exe on Windows or a dmg on macOS."


In this case, the Java application will be converted to JavaScript, 
and the required code to meet JLS semantics will be provided.  Since 
the runtime binary (java.exe) isn't needed, it is not a necessary 
dependency and is omitted.  The resulting native package is provided 
in a platform-specific format (WAR or ZIP), as appropriate for a web 
application.  To me, this certainly meets the letter of the jpackage 
description, but perhaps not the spirit.  I'm interested in more 
feedback on this point.


2. If the answer to (1) above is 'yes', jpackage is only suitable if 
you plan to bundle OpenJDK VM code, then I will propose a new tool, 
separate from jpackage.  The tentative name is "jspackage".


The jspackage command will take as input a Java application, and 
produce a web application that includes all of the necessary 
dependencies.  It will produce a cross-platform web application bundle 
in either WAR or ZIP format.



On Mon, Apr 26, 2021 at 5:39 AM Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> wrote:


Without commenting on the value proposition of what you propose to
do, I
am fairly certain that jpackage is not the way to do it. The job of
jpackage is to take an application, bundle it with a Java Runtime,
and
create a native package / installer from it. What you are describing
goes far beyond that. You are describing a new capability of the JDK
that would take Java bytecode and compile it to run it on top of a
JavaScript engine.

> jpackage will use a JavaScript AOT compiler (TeaVM) to convert
the Java
> code to JavaScript, with the main class compiled to a JavaScript
method
> called 'main()'.

This is a good indicator that your proposal isn't simply targeting
a new
platform that already exists, and for which there is a Java
runtime that
supports running on this platform.

-- Kevin


On 4/25/2021 5:10 PM, Andrew Oliver wrote:
> While I agree it is a somewhat different platform than Linux,
Mac, or
> Windows, I do think the web is a platform worth targeting.  And
when seen
> through just a slightly different lens, it is more like the
others than it
> might first seem:
>
> On the platform:
> * It is difficult for users to run Java bytecode or JARs directly
> * Bytecode needs some form of transformation to operate efficiently
> * Packaging into a platform-specific format is required for easy
> distribution
> * Without a packager tool, developers have to surmount significant
> obstacles to distribute on the platform, reducing the appeal and
adoption
> of Java
>
> Yes, there are maven and gradle plugins available to allow Java
to target
> the JavaScript platform.  ( For example,
> https://teavm.org/docs/intro/getting-started.html

<https://urldefense.com/v3/__https://teavm.org/docs/intro/getting-started.html__;!!GqivPVa7Brio!JcjBmV16Taw_X560U_rEcpDd8zUmWkgoUcX2JAvq-FLhJ7rhN6mYnTPPu95aKDkiQ6ZG$>
)
>
> However, for many users a browser-friendly solution with a small
number of
> dependencies is going to be the only option.  Take, for example,
new users,
> students, and educational settings.  In many cases, programming
assignments
> are required to be posted on the web.  If the JDK could tar

Re: RFR: 8266179: [macos] jpackage should specify architecture for produced pkg files [v4]

[Kevin Rushforth]

On Mon, 3 May 2021 22:52:21 GMT, Alexander Matveev  wrote:

>> jpackage should specify architecture for produced PKG files via 
>> hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable 
>> on x64 without specifying hostArchitectures which is not correct and if 
>> install on arm Mac it will request Rosetta 2. With proposed fix by setting 
>> hostArchitectures="x86_x64" if installer contains x64 binaries, it will be 
>> installable on x64 Mac and will require Rosetta 2 on arm Mac. 
>> hostArchitectures will be set to arm64 if installer contain aarch64 binaries 
>> and will gave error when run on x64 Mac and will be installable on arm Mac 
>> without triggering installation of Rosetta 2.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266179: [macos] jpackage should specify architecture for produced pkg 
> files [v4]

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk/pull/3807

Re: RFR: 8266179: [macos] jpackage should specify architecture for produced pkg files [v4]

[Kevin Rushforth]

On Mon, 3 May 2021 22:52:21 GMT, Alexander Matveev  wrote:

>> jpackage should specify architecture for produced PKG files via 
>> hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable 
>> on x64 without specifying hostArchitectures which is not correct and if 
>> install on arm Mac it will request Rosetta 2. With proposed fix by setting 
>> hostArchitectures="x86_x64" if installer contains x64 binaries, it will be 
>> installable on x64 Mac and will require Rosetta 2 on arm Mac. 
>> hostArchitectures will be set to arm64 if installer contain aarch64 binaries 
>> and will gave error when run on x64 Mac and will be installable on arm Mac 
>> without triggering installation of Rosetta 2.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266179: [macos] jpackage should specify architecture for produced pkg 
> files [v4]

I presume you've done a CI test build on machines of both architectures?

-

PR: https://git.openjdk.java.net/jdk/pull/3807

Re: RFR: 8266179: [macos] jpackage should specify architecture for produced pkg files

[Kevin Rushforth]

On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev  
wrote:

> jpackage should specify architecture for produced PKG files via 
> hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable 
> on x64 without specifying hostArchitectures which is not correct and if 
> install on arm Mac it will request Rosetta 2. With proposed fix by setting 
> hostArchitectures="x86_x64" if installer contains x64 binaries, it will be 
> installable on x64 Mac and will require Rosetta 2 on arm Mac. 
> hostArchitectures will be set to arm64 if installer contain aarch64 binaries 
> and will gave error when run on x64 Mac and will be installable on arm Mac 
> without triggering installation of Rosetta 2.

The fix looks good. Would it be feasible to include an automated test for this?

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/3807

Re: Proposal to add JavaScript platform to jpackage

[Kevin Rushforth]

Without commenting on the value proposition of what you propose to do, I 
am fairly certain that jpackage is not the way to do it. The job of 
jpackage is to take an application, bundle it with a Java Runtime, and 
create a native package / installer from it. What you are describing 
goes far beyond that. You are describing a new capability of the JDK 
that would take Java bytecode and compile it to run it on top of a 
JavaScript engine.



jpackage will use a JavaScript AOT compiler (TeaVM) to convert the Java
code to JavaScript, with the main class compiled to a JavaScript method
called 'main()'.


This is a good indicator that your proposal isn't simply targeting a new 
platform that already exists, and for which there is a Java runtime that 
supports running on this platform.


-- Kevin


On 4/25/2021 5:10 PM, Andrew Oliver wrote:

While I agree it is a somewhat different platform than Linux, Mac, or
Windows, I do think the web is a platform worth targeting.  And when seen
through just a slightly different lens, it is more like the others than it
might first seem:

On the platform:
* It is difficult for users to run Java bytecode or JARs directly
* Bytecode needs some form of transformation to operate efficiently
* Packaging into a platform-specific format is required for easy
distribution
* Without a packager tool, developers have to surmount significant
obstacles to distribute on the platform, reducing the appeal and adoption
of Java

Yes, there are maven and gradle plugins available to allow Java to target
the JavaScript platform.  ( For example,
https://teavm.org/docs/intro/getting-started.html )

However, for many users a browser-friendly solution with a small number of
dependencies is going to be the only option.  Take, for example, new users,
students, and educational settings.  In many cases, programming assignments
are required to be posted on the web.  If the JDK could target
self-contained web applications, as per this proposal, students could
easily post their assignments for the whole class to see.  This would be
much more reasonable than asking students to learn Java and maven and POM
files (and I'm saying that as a fan of maven).

Lest people misinterpret the above as suggesting this JEP is useful only in
an educational context, many business projects these days need to be web
applications.  Users are often unwilling or unable to download and install
applications for short, quick, or one-off transactions.  Thus there is a
large market for projects that absolutely require a web presence.  This JEP
would help illustrate how Java could be used even for front-end web
development.  Yes, large-scale projects would likely use maven or gradle.
But for quick proofs-of-concept, little could make it easier to demonstrate
the ability to do front-end development in Java then easily packaging a
Java code into a ZIP and deploying on any web server (or a WAR on an
application server, if desired).

   -Andrew

On Sat, Apr 24, 2021 at 10:39 PM Scott Palmer  wrote:


This doesn’t seem like something that should be the job of jpackage.  The
jpackage tool is currently used for producing platform-specific packages or
installers targeted at end-users that include native launchers and a JRE.
Web-based applications are an entirely different beast. This seems like
more of a job for a Maven or Gradle plugin.

Regards,

Scott



On Apr 24, 2021, at 5:59 PM, Andrew Oliver <93q...@gmail.com> wrote:

Below is a Java Enhancement Proposal for your consideration to add
JavaScript to jpackage as a new target platform.  I would appreciate
feedback on the proposal contents.  I am also interested in learning

about

the process, specifically what approvals are required prior to start of
implementation, should sufficient consensus be reached.

( To view this proposal as a web page, please visit:
https://frequal.com/TeaVM/openjdk/jdk-list-draft1.html )

Thank you!

  -Andrew Oliver

Title: Add JavaScript platform to jpackage
Author: Andrew Oliver
Created: 2021/04/24
Type: Feature
State: Draft
Exposure: Open
Component: tools/jpackage
Scope: JDK
Discussion: core-libs-dev@openjdk.java.net
Template: 1.0

Summary
---

jpackage already allows packaging Java applications for several

platforms.

This proposal adds a new platform: JavaScript.

This effort will enable jpackage to convert bytecode from the provided
classes into JavaScript, and generate the required HTML to invoke the
specified main method when opened in a web browser. These files will be
bundled into a WAR file for easy deployment.

Goals
-

*   Enabling JVM languages to build client-side web applications
*   Allow easy generation of JavaScript from JVM bytecode
*   Allow easy deployment and execution of generated JavaScript in web
browsers
*   Allow easy deployment of the generated JavaScript in all web server
environments
*   Java web application container (like Tomcat)
*   Static file web servers
*   Static file web hosting services

Non-Goals
---

Re: RFR: JDK-8265078: jpackage tests on Windows leave large temp files [v2]

[Kevin Rushforth]

On Wed, 14 Apr 2021 17:36:21 GMT, Alexey Semenyuk  wrote:

> postVisitDirectory() and visitFile() methods can be potentially called 
> concurrently by walkFileTree()

I don't think they can.

> Javadoc ... doesn't say anything about synchronization, so I think it is fair 
> to assume it is not guaranteed.

Given that the `Files` class says nothing about running its various file 
walking operations in parallel, I think you can be confident that the visitor 
methods are only ever run on the same thread that calls walkFileTree. I would 
consider it a bug if the thread used to callback into the visitor were 
different from the calling thread.

Still, I think using `AtomicReference` is fine here, so this is a moot point 
for this PR.

-

PR: https://git.openjdk.java.net/jdk/pull/3473

Re: RFR: JDK-8265078: jpackage tests on Windows leave large temp files [v3]

[Kevin Rushforth]

On Tue, 13 Apr 2021 22:50:12 GMT, Andy Herrick  wrote:

>> two changes:
>> One to jpackage, when recursively removing directory, when IOException 
>> occurs, record it and continue (deleting as much as possible) before 
>> throwing the exception.
>> The other to tests, when running jpackage via ProcessBuilder.execute(), set 
>> the "TMP" environment variable to the current value of System Property 
>> "java.io.tmpdir".  This causes the sub-process (jpackage) to output tmp 
>> files to the tmp file location used by the test. (So the test harness can 
>> clean up after test exits).
>
> Andy Herrick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8265078: jpackage tests on Windows leave large temp files

This looks fine. I see that this bug is listed as Windows-specific. Are any 
similar changes needed for other platforms, or do they already write test files 
only to `java.io.tmpdir`?

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/3473

Re: RFR: JDK-8265078: jpackage tests on Windows leave large temp files [v2]

[Kevin Rushforth]

On Tue, 13 Apr 2021 21:05:26 GMT, Andy Herrick  wrote:

>> two changes:
>> One to jpackage, when recursively removing directory, when IOException 
>> occurs, record it and continue (deleting as much as possible) before 
>> throwing the exception.
>> The other to tests, when running jpackage via ProcessBuilder.execute(), set 
>> the "TMP" environment variable to the current value of System Property 
>> "java.io.tmpdir".  This causes the sub-process (jpackage) to output tmp 
>> files to the tmp file location used by the test. (So the test harness can 
>> clean up after test exits).
>
> Andy Herrick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8265078: jpackage tests on Windows leave large temp files

Are you sure you need an `AtomicReference` to set the exception? I don't see 
any use of multiple threads, but I might be missing something. If you do need 
it, you need to fix the order of arguments.

src/jdk.jpackage/share/classes/jdk/jpackage/internal/IOUtils.java line 96:

> 94: Files.delete(dir);
> 95: } catch (IOException ex) {
> 96: exception.compareAndSet(ex, null);

The arguments are backwards. The first argument is the one used for comparison, 
and if the current reference is equal to the first, it is set to the second 
value.

-

Changes requested by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/3473

Re: RFR: 8189198: Add "forRemoval = true" to Applet API deprecations [v2]

[Kevin Rushforth]

On Thu, 25 Mar 2021 22:58:53 GMT, Andy Herrick  wrote:

>> implementation of
>> JDK-8256145: JEP 398: Deprecate the Applet API for Removal
>
> Andy Herrick 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 301 additional 
> commits since the last revision:
> 
>  - 8189198: Add "forRemoval = true" to Applet API deprecations
>  - Merge branch 'master' into 8189198
>  - 8260862: JFR: New configure command for the jfr tool
>
>Reviewed-by: mgronlun
>  - 8264161: BigDecimal#stripTrailingZeros can throw undocumented 
> ArithmeticException
>
>Reviewed-by: bpb
>  - 8262081: 
> vmTestbase/nsk/jdi/ThreadDeathRequest/addThreadFilter/addthreadfilter001/TestDescription.java
>  failed with "ERROR: eventSet1.size() != 3 :: 2"
>
>Reviewed-by: cjplummer, lmesnik, sspitsyn
>  - 8261502: ECDHKeyAgreement: Allows alternate ECPrivateKey impl and revised 
> exception handling
>
>Reviewed-by: jnimeh
>  - 8253795: Implementation of JEP 391: macOS/AArch64 Port
>8253816: Support macOS W^X
>8253817: Support macOS Aarch64 ABI in Interpreter
>8253818: Support macOS Aarch64 ABI for compiled wrappers
>8253819: Implement os/cpu for macOS/AArch64
>8253839: Update tests and JDK code for macOS/Aarch64
>8254941: Implement Serviceability Agent for macOS/AArch64
>8255776: Change build system for macOS/AArch64
>8262903: [macos_aarch64] Thread::current() called on detached thread
>
>Co-authored-by: Vladimir Kempik 
>Co-authored-by: Bernhard Urban-Forster 
>Co-authored-by: Ludovic Henry 
>Co-authored-by: Monica Beckwith 
>Reviewed-by: erikj, ihse, prr, cjplummer, stefank, gziemski, aph, 
> mbeckwit, luhenry
>  - 4833719: (bf) Views of MappedByteBuffers are not MappedByteBuffers, and 
> cannot be forced
>
>Reviewed-by: adinn
>  - 8264165: jpackage BasicTest fails after JDK-8220266: Check help text 
> contains plaform specific parameters
>
>Reviewed-by: herrick, dcubed
>  - 8263454: com.apple.laf.AquaFileChooserUI ignores the result of 
> String.trim()
>
>Reviewed-by: serb, pbansal, kizune, trebari, psadhukhan
>  - ... and 291 more: 
> https://git.openjdk.java.net/jdk/compare/e2285595...026f09c8

Looks good.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/2920

Re: RFR: 8189198: Add "forRemoval = true" to Applet API deprecations

[Kevin Rushforth]

On Wed, 10 Mar 2021 18:33:37 GMT, Andy Herrick  wrote:

> implementation of
> JDK-8256145: JEP 398: Deprecate the Applet API for Removal

src/java.desktop/share/classes/java/applet/package-info.java line 39:

> 37:  * applet development environment.
> 38:  * 
> 39:  * @deprecated.  This package has been deprecated and may be removed in

Package declarations cannot have `@deprecated` tags, so `make docs` fails with 
this change. Also, since there is a tag here, the previous `` is now empty 
and causes a warning. Both problems will be fixed by removing the `@deprecated` 
tag, while leaving the added text.

-

PR: https://git.openjdk.java.net/jdk/pull/2920

Re: RFR: JDK-8264057: [redo] JDK-8248904: Add support to jpackage for the Mac App Store.

[Kevin Rushforth]

On Wed, 24 Mar 2021 11:00:53 GMT, Andy Herrick  wrote:

> Restoring fix to JDK-8248904: Add support to jpackage for the Mac App Store.

I can confirm that this restores the fix for JDK-8248904. There are no diffs in 
any jpackage file between the commit prior to the backout and this PR.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/3172

Re: RFR: JDK-8264055: backout JDK-8248904 in order to resubmit with additional attribution.

[Kevin Rushforth]

On Tue, 23 Mar 2021 18:17:00 GMT, Andy Herrick  wrote:

>> Looks good, although i see the following wasn't backed out:
>> 
>> src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/JavaApp.icns
>> 
>> If this is intentional, then no need to worry about it.
>
>> Looks good, although i see the following wasn't backed out:
>> 
>> ```
>> src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/JavaApp.icns
>> ```
> 
> yes - look again - it was renamed back to java.icns (the 12'th or 22 files 
> changed above)

As long as there are no transient build/test failures, it will be fine. I was 
just comparing the results with what `git revert 
11c8c78c47f21fcd87a5969a859b5c4fced5e47d` generated and noticed this difference.

-

PR: https://git.openjdk.java.net/jdk/pull/3159

Re: RFR: JDK-8264055: backout JDK-8248904 in order to resubmit with additional attribution.

[Kevin Rushforth]

On Tue, 23 Mar 2021 17:09:20 GMT, Andy Herrick  wrote:

> We are backing out the fix to "JDK-8248904 Add support to jpackage for the 
> Mac App Store " in order to resubmit with added contributor attribution for 
> Erwin Morrhey
> This is the backout.

Looks good, although i see the following wasn't backed out:

src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/JavaApp.icns

If this is intentional, then no need to worry about it.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/3159

Re: RFR: JDK-8263545: Convert jpackage to use Stream.toList()

[Kevin Rushforth]

On Sun, 14 Mar 2021 18:22:50 GMT, Ian Graves  wrote:

> This converts jpackage to use `Stream.toList()` instead of 
> `Stream.collect(Collectors.toList())`. One piece of code was modified to not 
> mutate a list in addition to one test that used a mutating sort on a list. 
> The rest of the changes are simple substitutions.

@igraves as a "best practice" try to avoid doing a force-push to a branch with 
an active code review. It makes it hard for reviewers to see what has changed. 
If you need to merge changes from master, use "git merge master" instead.

@alexeysemenyukoracle @sashamatveev Can you re-review this so Ian can integrate 
it?

-

PR: https://git.openjdk.java.net/jdk/pull/2997

Re: RFR: 8263536: Add missing @compile tags to jpackage tests [v2]

[Kevin Rushforth]

On Mon, 15 Mar 2021 19:43:12 GMT, Alexey Semenyuk  wrote:

>> Changes requested by iklam (Reviewer).
>
> @kevinrushforth I plan to resolve JDK-8263474 manually as "Delivered". Would 
> this work?

Yes, that should work.

-

PR: https://git.openjdk.java.net/jdk/pull/2975

Re: RFR: JDK-8262277: URLClassLoader.getResource throws undocumented IllegalArgumentException [v5]

[Kevin Rushforth]

On Mon, 15 Mar 2021 16:29:54 GMT, Craig Andrews 
 wrote:

>> You need to fix this error:
>> 
>>>  Title mismatch between PR and JBS for issue JDK-8262277
>> 
>> by changing the title of this PR to match the JBS title. Then you should be 
>> able to integrate it.
>
>> You need to fix this error:
>> 
>> > Title mismatch between PR and JBS for issue JDK-8262277
>> 
>> by changing the title of this PR to match the JBS title. Then you should be 
>> able to integrate it.
> 
> Done - how's it look now?

You don't need to add yourself as a contributor. The only thing you need to do 
is integrate. Then it is ready to be sponsored.

-

PR: https://git.openjdk.java.net/jdk/pull/2662

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v5]

[Kevin Rushforth]

On Mon, 15 Mar 2021 16:19:20 GMT, Craig Andrews 
 wrote:

>> Marked as reviewed by psadhukhan (Reviewer).
>
> What's the next step in the process for this PR?

You need to fix this error:

>  Title mismatch between PR and JBS for issue JDK-8262277

by changing the title of this PR to match the JBS title. Then you should be 
able to integrate it.

-

PR: https://git.openjdk.java.net/jdk/pull/2662

Re: RFR: 8263536: Add missing @compile tags to jpackage tests [v2]

[Kevin Rushforth]

On Sat, 13 Mar 2021 00:42:13 GMT, Alexander Matveev  
wrote:

>> Alexey Semenyuk 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 one additional 
>> commit since the last revision:
>> 
>>   8263536: Add missing @compile tags to jpackage tests
>
> Marked as reviewed by almatvee (Committer).

Since you are removing the problem listing, would it be better to use the 
parent bug ID -- 8263474 -- rather than a sub-task? How do you plan to resolve 
the parent bug? Manually as "Delivered"? Something else?

-

PR: https://git.openjdk.java.net/jdk/pull/2975

Re: RFR: 8263480: ProblemList two jpackage tests on Windows

[Kevin Rushforth]

On Thu, 11 Mar 2021 23:44:18 GMT, Daniel D. Daugherty  
wrote:

>> A trivial fix to ProblemList two new tests on Windows.
>
> @alexeysemenyukoracle or @kevinrushforth - can either of you folks review 
> this ProblemListing?

Sure, although I a not a Reviewer in the jdk project, so it will need one more. 
Alexey just became a reviewer, but I don't think the census is updated yet.

-

PR: https://git.openjdk.java.net/jdk/pull/2952

Re: RFR: 8263480: ProblemList two jpackage tests on Windows

[Kevin Rushforth]

On Thu, 11 Mar 2021 23:41:53 GMT, Daniel D. Daugherty  
wrote:

> A trivial fix to ProblemList two new tests on Windows.

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk/pull/2952

Re: RFR: JDK-8248904: Add support to jpackage for the Mac App Store

[Kevin Rushforth]

On Wed, 24 Feb 2021 21:59:22 GMT, Andy Herrick  wrote:

> Implementation of Mac App Support including three new mac specific CLI 
> options.

Looks good with a couple questions. Is `JavaApp.icns` intended to be an empty 
file (I see that the file it was renamed from was empty, so probably OK)? The 
rest are inline below.

src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties
 line 188:

> 186: \  over-ridden by adding replacement resources to this 
> directory.\n\
> 187: \  (absolute path or relative to the current directory)\n\
> 188: \  --runtime-image \n\

This seems unrelated to this change.

src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties
 line 247:

> 245: \  --mac-app-store\n\
> 246: \  Indicates that the jpackage output is intended for the\n\
> 247: \  Mac App Store.\n\

Maybe `macOS App Store`?

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/2716

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0 [v3]

[Kevin Rushforth]

On Thu, 11 Feb 2021 19:23:55 GMT, Roger Riggs  wrote:

>> On Mac Os X, the OSVersionTest detected a difference in the version number 
>> reported in the os.version property
>> and the version number provided by `sw_vers -productVersion`.
>> 
>> When the java runtime is built with XCode 11.3, the os.version is reported 
>> as 10.16
>> though the current version numbering is 11.nnn.  
>> 
>> The workaround is to derive the os.version number from the 
>> ProductBuildVersion.
>> When the toolchain is updated to XCode 12.nnn it can be removed.
>> The workaround is enabled only when the environment variable 
>> SYSTEM_VERSION_COMPAT is unset.  
>> When the SYSTEM_VERSION_COMPAT is set in the environment the version number 
>> is reported as reported by Mac OS X.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   optimize case of 10.16 and SYSTEM_VERSION_COMPAT is set

Looks good.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0

[Kevin Rushforth]

On Thu, 11 Feb 2021 17:14:35 GMT, Roger Riggs  wrote:

> On Mac Os X, the OSVersionTest detected a difference in the version number 
> reported in the os.version property
> and the version number provided by `sw_vers -productVersion`.
> 
> When the java runtime is built with XCode 11.3, the os.version is reported as 
> 10.16
> though the current version numbering is 11.nnn.  
> 
> The workaround is to derive the os.version number from the 
> ProductBuildVersion.
> When the toolchain is updated to XCode 12.nnn it can be removed.
> The workaround is enabled only when the environment variable 
> SYSTEM_VERSION_COMPAT is unset.  
> When the SYSTEM_VERSION_COMPAT is set in the environment the version number 
> is reported as reported by Mac OS X.

I tested this, and get the following behavior on macOS 11.0.1 with a JDK 
compiled with Xcode 11.3.1 and your patch:

SYSTEM_VERSION_COMPAT not set : 11.0
SYSTEM_VERSION_COMPAT=1 : 10.16
SYSTEM_VERSION_COMPAT=0 : 11.0.1

So the fallback path reports what could be considered a more accurate version, 
at least on my laptop. Since I'm not sure what is expected, you can decide what 
to do with this information.

-

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0 [v2]

[Kevin Rushforth]

On Thu, 11 Feb 2021 18:53:08 GMT, Kevin Rushforth  wrote:

>> Roger Riggs has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Correct double-negative in 'other than 10.16'
>
> src/java.base/macosx/native/libjava/java_props_macosx.c line 262:
> 
>> 260: // Copy out the char*
>> 261: osVersionCStr = strdup([nsVerStr UTF8String]);
>> 262: } else if (getenv("SYSTEM_VERSION_COMPAT") == NULL) {
> 
> If version is 10.16 and `SYSTEM_VERSION_COMPAT` is set, you will fall through 
> to the pre-10.9 Mac OS code fallback. Just checking to see if that's what you 
> intended.

FWIW, it seems to work OK using the legacy fallback path (reports 10.16 if I 
set `SYSTEM_VERSION_COMPAT=1`).

-

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0 [v2]

[Kevin Rushforth]

On Thu, 11 Feb 2021 18:34:54 GMT, Roger Riggs  wrote:

>> On Mac Os X, the OSVersionTest detected a difference in the version number 
>> reported in the os.version property
>> and the version number provided by `sw_vers -productVersion`.
>> 
>> When the java runtime is built with XCode 11.3, the os.version is reported 
>> as 10.16
>> though the current version numbering is 11.nnn.  
>> 
>> The workaround is to derive the os.version number from the 
>> ProductBuildVersion.
>> When the toolchain is updated to XCode 12.nnn it can be removed.
>> The workaround is enabled only when the environment variable 
>> SYSTEM_VERSION_COMPAT is unset.  
>> When the SYSTEM_VERSION_COMPAT is set in the environment the version number 
>> is reported as reported by Mac OS X.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Correct double-negative in 'other than 10.16'

src/java.base/macosx/native/libjava/java_props_macosx.c line 262:

> 260: // Copy out the char*
> 261: osVersionCStr = strdup([nsVerStr UTF8String]);
> 262: } else if (getenv("SYSTEM_VERSION_COMPAT") == NULL) {

If version is 10.16 and `SYSTEM_VERSION_COMPAT` is set, you will fall through 
to the pre-10.9 Mac OS code fallback. Just checking to see if that's what you 
intended.

-

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0

[Kevin Rushforth]

On Thu, 11 Feb 2021 17:50:20 GMT, Brian Burkhalter  wrote:

>> It's a double negative, unless I'm reading it incorrectly: "other than 
>> pre-10.16" I interpret as "not pre-10.16" or "10.16".
>
> `// Copy out the char* if running on version 10.x[.y], where x < 16` ?

It will also do it if running on `11.x[.y]` (once Xcode is upgraded), right?

-

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0

[Kevin Rushforth]

On Thu, 11 Feb 2021 17:46:06 GMT, Roger Riggs  wrote:

>> @kevinrushforth I think you are correct. This is actually mea culpa I think 
>> as I had provided in a Slack thread a failed attempt at a patch for this 
>> which contained this comment.
>
> So the words "other than" are too subtle?

It's a double negative, unless I'm reading it incorrectly: "other than 
pre-10.16" I interpret as "not pre-10.16" or "10.16".

-

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8253702: BigSur java/lang/System/OsVersionTest.java: 10.16 != 11.0

[Kevin Rushforth]

On Thu, 11 Feb 2021 17:14:35 GMT, Roger Riggs  wrote:

> On Mac Os X, the OSVersionTest detected a difference in the version number 
> reported in the os.version property
> and the version number provided by `sw_vers -productVersion`.
> 
> When the java runtime is built with XCode 11.3, the os.version is reported as 
> 10.16
> though the current version numbering is 11.nnn.  
> 
> The workaround is to derive the os.version number from the 
> ProductBuildVersion.
> When the toolchain is updated to XCode 12.nnn it can be removed.
> The workaround is enabled only when the environment variable 
> SYSTEM_VERSION_COMPAT is unset.  
> When the SYSTEM_VERSION_COMPAT is set in the environment the version number 
> is reported as reported by Mac OS X.

src/java.base/macosx/native/libjava/java_props_macosx.c line 250:

> 248: 
> 249: NSString *nsVerStr;
> 250: // Copy out the char* if running on version other than pre-10.16 
> Mac OS (10.16 == 11.x)

Isn't the comment backwards from what the test does? I would think it should be 
"if running on version other than 10.16 Mac OS ...". Or am I missing something?

-

PR: https://git.openjdk.java.net/jdk/pull/2530

Re: RFR: 8223188: Removed unnecessary #ifdef __cplusplus from .cpp sources

[Kevin Rushforth]

On Mon, 8 Feb 2021 22:26:22 GMT, Alexey Semenyuk  wrote:

> Remove needless `#ifdef __cplusplus` from .cpp sources

@alexeysemenyukoracle FYI, there was no need to force-push (or even push at 
all) to your branch. It doesn't matter at all what the commit message(s) of the 
commit(s) in the source branch are. Skara will use the title of the PR, which 
needs to match the JBS bug, as the final commit message.

-

PR: https://git.openjdk.java.net/jdk/pull/2466

Re: RFR: 8253497: Core Libs Terminology Refresh

[Kevin Rushforth]

On Mon, 14 Dec 2020 19:36:48 GMT, Brent Christian  wrote:

> This is part of an effort in the JDK to replace archaic/non-inclusive words 
> with more neutral terms (see JDK-8253315 for details).
> 
> Here are the changes covering core libraries code and tests.  Terms were 
> changed as follows:
> 1. grandfathered -> legacy
> 2. blacklist -> filter or reject
> 3. whitelist -> allow or accept
> 4. master -> coordinator
> 5. slave -> worker
> 
> Addressing similar issues in upstream 3rd party code is out of scope of this 
> PR.  Such changes will be picked up from their upstream sources.

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk/pull/1771

Re: RFR: 8256167: Convert JDK use of `Reference::get` to `Reference::refersTo`

[Kevin Rushforth]

On Thu, 3 Dec 2020 22:54:54 GMT, Mandy Chung  wrote:

> This patch replaces some uses of `Reference::get` to `Reference::refersTo` to 
> avoid keeping a referent strongly reachable that could cause unnecessary 
> delay in collecting such object.   I only made change in some but not all 
> classes in core libraries when working with Kim on `Reference::refersTo`.
> The remaining uses are left for the component owners to convert at 
> appropriate time.

You have a typo that will cause a compilation error.

src/java.base/share/classes/java/util/WeakHashMap.java line 437:

> 435: int index = indexFor(h, tab.length);
> 436: Entry e = tab[index];
> 437: while (e != null && !(e.hash == h && matchesKey(e, k))

This doesn't compile, which is why the checks from the GitHub actions build 
failed.

-

Changes requested by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/1609

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

[Kevin Rushforth]

On Tue, 17 Nov 2020 22:12:19 GMT, Jim Laskey  wrote:

>> @kevinrushforth What is the recommended approach to remove the doc 
>> edit/commit?
>
> javadoc can be found at 
> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html

Presuming your master branch is in sync with the upstream jdk repo, something 
like the following:

git checkout master -- doc
git add doc
git commit -m "restore doc files"

-

PR: https://git.openjdk.java.net/jdk/pull/1273

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

[Kevin Rushforth]

On Tue, 17 Nov 2020 20:56:52 GMT, Jim Laskey  wrote:

> my local branch seems to have the right sources for doc

Maybe, but your branch on GitHub does not.

-

PR: https://git.openjdk.java.net/jdk/pull/1273

Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v3]

[Kevin Rushforth]

On Fri, 13 Nov 2020 15:05:15 GMT, Andy Herrick  wrote:

>> JDK-8189198: Add "forRemoval = true" to Applet APIs
>
> Andy Herrick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8189198: Add "forRemoval = true" to Applet APIs

src/java.desktop/share/classes/java/applet/package-info.java line 40:

> 38:  * 
> 39:  * Deprecated.
> 40:  * This package has been deprecated and may be removed in a future 
> version of the Java Platform.

That should be `@deprecated This package ...`. See 
[java/rmi/activation/package-info.java#L41](https://github.com/openjdk/jdk/blob/master/src/java.rmi/share/classes/java/rmi/activation/package-info.java#L41).

-

PR: https://git.openjdk.java.net/jdk/pull/1127

Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v2]

[Kevin Rushforth]

On Fri, 13 Nov 2020 09:31:53 GMT, Alan Bateman  wrote:

>> Andy Herrick 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 six additional 
>> commits since the last revision:
>> 
>>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>>  - Merge branch 'master' into JDK-8189198
>>  - Merge branch 'master' into JDK-8189198
>>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>
> src/java.naming/share/classes/javax/naming/Context.java line 1087:
> 
>> 1085: @Deprecated(since="16", forRemoval=true)
>> 1086: String APPLET = "java.naming.applet";
>> 1087: };
> 
> Probably should be since="9" (the deprecation in JDK-8051422 pre-dates the 
> enhanced deprecation work).

Good point, since it was in fact deprecated in 9.

-

PR: https://git.openjdk.java.net/jdk/pull/1127

Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs

[Kevin Rushforth]

On Mon, 9 Nov 2020 13:56:33 GMT, Andy Herrick  wrote:

> JDK-8189198: Add "forRemoval = true" to Applet APIs

@andyherrick can you enter the `/csr needed` command? I would, but it needs to 
be done by either the author of the PR or a Reviewer.

-

PR: https://git.openjdk.java.net/jdk/pull/1127

Re: 8248122: java.base should not handle JavaFX application in a specific way

[Kevin Rushforth]

As mentioned in the pull request, this cannot be done as proposed 
without causing a behavioral regression and breaking JavaFX 
applications. If done, it should be done carefully using a similar 
process to the deprecate-for-removal in one release (to give 
applications time to react and adapt) and then remove in a future release.


Before getting into a discussion of how to do it, though, can you say 
more about the reasons you want to do it?


-- Kevin


On 10/31/2020 9:05 AM, Kartik Ohri wrote:

Hi!
JavaFX is no longer a part of OpenJDK. It does not make sense to treat it
specially in the JDK. Hence, as suggested in JDK-
8248122
, the Launcher class
should be refactored to remove the JavaFX specific handling code.
Accordingly, I have proposed a patch here
.

All suggestions are welcome. Thank you.
Regards,
Kartik

Re: RFR: 8248122: java.base should not handle JavaFX application in a specific way

[Kevin Rushforth]

On Sat, 31 Oct 2020 16:09:18 GMT, Kevin Rushforth  wrote:

>> JavaFX is no longer a part of OpenJDK. It makes sense to not treat it 
>> specially in the JDK. Hence, refactoring the Launcher class to remove JavaFX 
>> specific code.
>> 
>> Further investigation reveals that some JavaFX specific code is also present 
>> in the `javadoc` tool. For instance,
>> https://github.com/openjdk/jdk/blob/master/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java#L90-L96
>> https://github.com/openjdk/jdk/blob/master/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java#L347-L353
>> I think we should remove this code as well and the related tests for it.
>
> This will cause a regression in behavior. It will break existing JavaFX 
> applications that do not have a main program. It could also break 
> applications that create or use certain JavaFX objects in the class 
> initializer of their JavaFX application.
> 
> There was [some initial 
> discussion](https://bugs.openjdk.java.net/browse/JDK-8202553?focusedCommentId=14176584&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14176584)
>  around doing this as a follow-on to the removal of JavaFX from JDK 11, but 
> if it is to be done, it needs to be discussed first. It would need to be done 
> using a process similar to deprecation-for-removal. We would need to make 
> changes in the JDK and/or JavaFX to warn about this in one release, and then 
> remove it in the following. A CSR would be needed for both steps.
> 
> I note that while I disagree with the rationale described in 
> [JDK-8248122](https://bugs.openjdk.java.net/browse/JDK-8248122) for making 
> this change, I am not necessarily opposed to the change itself.

This also needs to be discussed on the openjfx-dev mailing list, since it will 
have behavioral compatibility implications for JavaFX.

-

PR: https://git.openjdk.java.net/jdk/pull/978

Re: RFR: 8248122: java.base should not handle JavaFX application in a specific way

[Kevin Rushforth]

On Sat, 31 Oct 2020 15:25:13 GMT, Kartik Ohri 
 wrote:

> JavaFX is no longer a part of OpenJDK. It makes sense to not treat it 
> specially in the JDK. Hence, refactoring the Launcher class to remove JavaFX 
> specific code.
> 
> Further investigation reveals that some JavaFX specific code is also present 
> in the `javadoc` tool. For instance,
> https://github.com/openjdk/jdk/blob/master/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java#L90-L96
> https://github.com/openjdk/jdk/blob/master/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java#L347-L353
> I think we should remove this code as well and the related tests for it.

This will cause a regression in behavior. It will break existing JavaFX 
applications that do not have a main program. It could also break applications 
that create or use certain JavaFX objects in the class initializer of their 
JavaFX application.

There was [some initial 
discussion](https://bugs.openjdk.java.net/browse/JDK-8202553?focusedCommentId=14176584&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14176584)
 around doing this as a follow-on to the removal of JavaFX from JDK 11, but if 
it is to be done, it needs to be discussed first. It would need to be done 
using a process similar to deprecation-for-removal. We would need to make 
changes in the JDK and/or JavaFX to warn about this in one release, and then 
remove it in the following. A CSR would be needed for both steps.

I note that while I disagree with the rationale described in 
[JDK-8248122](https://bugs.openjdk.java.net/browse/JDK-8248122) for making this 
change, I am not necessarily opposed to the change itself.

-

Changes requested by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/978

Re: RFR: JDK-8254843: Exception launching app on windows in some cases

[Kevin Rushforth]

On Fri, 16 Oct 2020 17:59:14 GMT, Andy Herrick  wrote:

> JDK-8254843: Exception launching app on windows in some cases
> loading splashscreen.dll in WinLaunchercpp would load java.dll from path 
> instead of runtime/bin causing jni launcher to
> crash. instead we just use what used to be the fallback, using 
> loadDllWithAddDllDirectory().

Looks good. I verified that it fixes the problem.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/706

Re: RFR: JDK-8252870: Finalize (remove "incubator" from) jpackage [v2]

[Kevin Rushforth]

On Tue, 13 Oct 2020 14:48:40 GMT, Andy Herrick  wrote:

>> JDK-8252870: Finalize (remove "incubator" from) jpackage
>
> Andy Herrick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8252870: Finalize (remove "incubator" from) jpackage
> - reverting two auto-generated files, and changing module-info to "@since 
> 16"

Looks good. I double-checked all of the file renames and all are exactly as 
expected. The diffs got a bit confused in a
couple cases, where it thought that a renamed and modified file was copied from 
somewhere else (see inline comments),
but that can happen given that git doesn't actually track renames and copies).

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/633

Re: RFR: JDK-8252870: Finalize (remove "incubator" from) jpackage [v2]

[Kevin Rushforth]

On Tue, 13 Oct 2020 21:30:05 GMT, Alexander Matveev  
wrote:

>> Andy Herrick has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   JDK-8252870: Finalize (remove "incubator" from) jpackage
>> - reverting two auto-generated files, and changing module-info to 
>> "@since 16"
>
> src/jdk.jpackage/linux/classes/module-info.java.extra line 29:
> 
>> 27: jdk.jpackage.internal.LinuxAppBundler,
>> 28: jdk.jpackage.internal.LinuxDebBundler,
>> 29: jdk.jpackage.internal.LinuxRpmBundler;
> 
> Not sure why it was changed. Looks like git recorded file renaming 
> incorrectly.

Yes, sometime GIt / Github get a bit confused about identifying rename/copy 
when generating a diff for a renamed file
with changes. The result is correct, but the diff looks odd.

> src/jdk.jpackage/macosx/classes/module-info.java.extra line 30:
> 
>> 28: jdk.jpackage.internal.MacAppStoreBundler,
>> 29: jdk.jpackage.internal.MacDmgBundler,
>> 30: jdk.jpackage.internal.MacPkgBundler;
> 
> Looks like another rename issue.

Yes (or more precisely another issue displaying the diffs for a renamed file 
with changes).

-

PR: https://git.openjdk.java.net/jdk/pull/633

Re: [BUG] Java 15: DecimalFormatSymbols overrides equals but not hashCode

[Kevin Rushforth]

I see this in DecimalFormatSymbols:


 /**
  * Override hashCode.
  */
>>>    private volatile int hashCode;
 @Override
 public int hashCode() {

Although, I'm not sure why the intervening private field would prevent 
javadoc from generating at least a method with an empty doc.


-- Kevin

On 9/15/2020 12:36 PM, Brian Burkhalter wrote:

Hello,

The override of hashCode() looks like it is still there in JDK 15 [1]. I don’t 
know however why it does not appear as such in the javadoc [2].

Brian

[1] 
http://hg.openjdk.java.net/jdk/jdk15/file/fb7064dc63f9/src/java.base/share/classes/java/text/DecimalFormatSymbols.java#l760
[2] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/text/DecimalFormatSymbols.html


On Sep 15, 2020, at 12:14 PM, Rob Spoor  wrote:

In Java 14 and before, DecimalFormatSymbols had overrides for both equals and 
hashCode. In Java 15, the override for hashCode has disappeared, and it now 
inherits hashCode from java.lang.Object. That means it now violates the 
contract for equals + hashCode: two equal DecimalFormatSymbols instances can 
have different hash codes.

Re: RFR: 8223187: Investigate setLocale() call in jpackage native launcher

[Kevin Rushforth]

On Sat, 12 Sep 2020 12:41:50 GMT, Kevin Rushforth  wrote:

>> setlocale() affects several C functions. We do not use most of these 
>> functions. We only using isspace() and toLower().
>> Based on how we use it I do not see any needs for setlocale(). After 
>> removing it I retested jpackage by changing
>> locally on machine and using different language as input parameters for 
>> jpackage. No issues found.
>
> Looks good. I confirmed that these are the only two calls to `setlocale` in 
> the jpackage sources.

Note: the title of the JBS bug was updated. Can you update the PR title to 
match?

-

PR: https://git.openjdk.java.net/jdk/pull/138

Re: RFR: 8223187: Investigate setLocale() call in jpackage native launcher

[Kevin Rushforth]

On Sat, 12 Sep 2020 02:15:29 GMT, Alexander Matveev  
wrote:

> setlocale() affects several C functions. We do not use most of these 
> functions. We only using isspace() and toLower().
> Based on how we use it I do not see any needs for setlocale(). After removing 
> it I retested jpackage by changing
> locally on machine and using different language as input parameters for 
> jpackage. No issues found.

Looks good. I confirmed that these are the only two calls to `setlocale` in the 
jpackage sources.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/138

Re: RFR: 8252999: Cleanup: replace .equals("") with .isEmpty() within all codebase

[Kevin Rushforth]

On Thu, 10 Sep 2020 11:21:28 GMT, David Holmes  wrote:

>> The code in java.base was updated to use String::isEmpty in JDK 12 
>> (JDK-8215281). There was follow-up in JDK 13 to do
>> the same in the java.desktop module (JDK-8223237). Changing the remaining 
>> usages make sense although I see that more
>> more than half are in tests.  It would be good to hear from security-dev on 
>> the changes to the Apache Santuario code
>> (in java.xml.crypto module) in case it would be better to contribute those 
>> upstream instead. Ditto for the Apache Xalan
>> code (in the java.xml module) but it may be significantly forked already 
>> that it doesn't matter.  I assume you can use
>> JDK-8215014 rather than creating a new issue.
>
> This should be broken up to deal with the files in different functional areas 
> under different bugids and PRs. Otherwise
> the cross-posting to so many lists is prohibitive. Files in different areas 
> need to be reviewed by different teams.
> Thank you.

@dholmes-ora raises a good point. Hopefully there is a balance point between a 
dozen different bugs / pull requests
each targeting one area and one bug / PR targeting a dozen separate areas. I 
don't have a good general rule to suggest.
Maybe @AlanBateman or @jddarcy can offer some advice?

-

PR: https://git.openjdk.java.net/jdk/pull/29

Re: RFR: 8252999: replace all String.equals("") usages with String.isEmpty()

[Kevin Rushforth]

On Wed, 9 Sep 2020 07:57:48 GMT, Dmitriy Dumanskiy 
 wrote:

>> @doom369 jcheck requires an associated issue
>
> @mrserb hello. Thanks for the review. Any further actions required from me?

Before this Enhancement can be formally reviewed, you will need a JBS bug ID. 
If you are already working with a
Committer or Reviewer in the `jdk` project who has agreed to sponsor your 
change, they can file the Enhancement
request. Otherwise, you can file it using the web interface at 
[bugreport.java.com](https://bugreport.java.com/). Once
you have a JBS bug ID, you need to edit the PR title to include that bug ID 
(without the `JDK-`) replacing
"Improvement".

Since this PR cuts across many functional areas (each gray label represents a 
functional area), you should expect a
longer review process, since someone from each functional area will need to 
look at the changes in their area (like
@mrserb started to do for the `2d` area).

-

PR: https://git.openjdk.java.net/jdk/pull/29

Re: RFR: 8225251: Minimise import statements in jpackage sources

[Kevin Rushforth]

That a reasonably common pattern in JUnit tests, but expanding them to 
individual static imports is, of course, fine as well.


-- Kevin


On 6/19/2020 9:08 AM, Alexey Semenyuk wrote:

Alexander,

There is still
---
import static org.junit.Assert.*;
---
in unit tests. Is this intended?

- Alexey

On 6/19/2020 4:46 AM, alexander.matv...@oracle.com wrote:

Please review the jpackage fix for bug [1] at [2].

Cleanup import statements.

[1] https://bugs.openjdk.java.net/browse/JDK-8225251
[2] http://cr.openjdk.java.net/~almatvee/8225251/webrev.00/

Thanks,
Alexander

Re: RFR: JDK-8244634:, LoadLibraryW failed from tools/jpackage tests after JDK-8242302

[Kevin Rushforth]

Is there a way you can link the launcher (e.g., something similar to 
RPATH on Unix systems) to look in the right place relative to the 
launcher? Otherwise, the solution with adding to the PATH seems OK to me.


-- Kevin


On 5/12/2020 5:22 AM, Andy Herrick wrote:
Is the problem that by removing the copy of the microsoft dlls from 
the applications bin directory, then on machines that do not have all 
of these dll's already in the PATH (usually in C:\windows\system32) 
they can no longer be found ?


I don't like manually manipulating the PATH env variable either, but 
if so I don't see what else can be done short of putting the app exe 
in the bin dir of the runtime.


/Andy


On 5/11/2020 9:37 PM, Alexander Matveev wrote:

Hi Alexey,

Updating PATH does not look like good solution to me. Did you try to 
load jli.dll by specifying full path to jli.dll when calling 
LoadLibary? If it does not work, then for AddDllDirectory() did you 
used LoadLibrary() or LoadLibraryEx() with 
LOAD_LIBRARY_SEARCH_USER_DIRS? According to doc you need to use 
LoadLibraryEx() with flag when using AddDllDirectory().


Thanks,
Alexander

On 5/11/2020 4:36 PM, Alexey Semenyuk wrote:

Please review fix [2] for jpackage bug [1].

Fix failure to load jli.dll from app launcher. The fix is to append 
path to directory with jli.dll to PATH env variable and load jli.dll 
with altered value of PATH if the first attempt to load jli.dll 
without altering PATH fails.


- Alexey

[1] https://bugs.openjdk.java.net/browse/JDK-8244634

[2] http://cr.openjdk.java.net/~asemenyuk/8244634/webrev.00

Re: jpackage signing fails with Mac jdk-14.0.1+7

[Kevin Rushforth]

Redirecting to core-libs-dev (with a Bcc to code-tools-dev)

Code signing for macOS, along with the addition of notarization, has 
been improved for JDK 15. You might want to try the EA builds of JDK 15, 
which are available here:


https://jdk.java.net/15/

-- Kevin


On 5/1/2020 8:13 AM, Adam Carroll wrote:

Using JDK 14.0.1 on the Mac, jpackage fails when signing is requested.
This problem was observed using AdoptOpenJDK.  I reported the problem to
that project and they suggested that  I report the problem here.

Platform:

Mac OS Catalina v10.15.4

Architecture:

x86

Description:

This problem was seen using AdoptOpenJDK 14.0.1+7.

Using the Mac signing option for jpackage ... --mac-sign ... I see the
following error (extra path information removed):

/var/folders/rh/./HelloFX.app: is already signed

However, if the --mac-sign option is removed, the build works without a
problem.

Reproducing the problem:

I've created a minimal, single-class JavaFX application along with the
necessary scripts to reproduce the problem:
https://github.com/AdamCarroll/jdk14-jpackage-mac

First clone the repo:

$ git clone g...@github.com:AdamCarroll/jdk14-jpackage-mac.git

Checkout the tag:

$ git checkout 1.0.0

Run the build (very fast as there's only one class):

$ ./gradlew clean build

Now run the jpackage command with the --mac-sign option as follows (this is
included in the file bin/create-signed-dmg.sh):

$ jpackage \
 --type dmg \
 --module-path 'build/modules' \
 --verbose \
 --add-modules javafx.controls \
 --input 'build/libraries' \
 --dest "build/bundle" \
 --name HelloFX \
 --main-jar 'jdk14-jpackage-mac.jar' \
 --main-class 'demo.HelloFX' \
 --mac-sign

You will now see a long error that includes the following:

/var/folders/rh/...jdk.incubator.jpackage/HelloFX.app: is already signed
java.io.IOException: Command [codesign, -s, Developer ID Application: Your
Name Here (XX), -,
/var/folders/rh/...jdk.incubator.jpackage.../HelloFX.app] exited with 1 code
at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Executor.executeExpectSuccess(Executor.java:73)
at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.IOUtils.exec(IOUtils.java:179)
at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.IOUtils.exec(IOUtils.java:150)
...

If you now run the same command but without the --mac-sign option (or
alternatively use the script bin/create-unsigned-dmg.sh), everything works
without problems.

You can find the original issue report to the AdoptOpenJDK repository here:
https://github.com/AdoptOpenJDK/openjdk-build/issues/1718

Re: jpackage with a single java property

[Kevin Rushforth]

No, I doubt this is a bug. If the following worked:

--java-options -Dfoo="bar 2"

meaning if the entire string `-Dfoo=bar 2` was treated as a single 
argument, then the following would fail:


--java-options "--ea -Dfoo=bar"

since it would also be treated as a single argument rather than two 
separate args as intended.


-- Kevin


On 3/4/2020 5:04 AM, Andy Herrick wrote:

A quick test shows me  that this form works fine:

    --java-options "-Dfoo='bar 2'"

where this form fails:

   --java-options -Dfoo="bar 2"

Initially I can see no reason both form shouldn't work, so this looks 
like a bug to me.


/Andy


On 3/4/2020 5:20 AM, Alexander Scherbatiy wrote:

Hello,

CSR for JEP 343: Packaging Tool [1] has description for option value 
which consists of a list of strings:


"If an option value is otherwise a list of strings, they must 
separated by space characters.  Since the shell would otherwise take 
them as separate  arguments, the list must be quoted. "


For example:

  --java-options -server --java-options "--ea -Dfoo=bar"


What about a single java property which value contains space 
characters (-Dfoo="bar 2")? Should it be possible to pass it in the way:


--java-options -Dfoo="bar 2"

or the whole property should be quoted like:

--java-options "-Dfoo='bar 2'"


[1] https://bugs.openjdk.java.net/browse/JDK-8213087

Thanks,

Alexander.

Re: jpackage current status

[Kevin Rushforth]

On 2/24/2020 12:31 PM, Michael Hall wrote:



On Feb 24, 2020, at 1:48 PM, Michael Hall  wrote:




On Feb 24, 2020, at 1:15 PM, Kevin Rushforth  wrote:

Since your ToolProvider-based program doesn't explicitly require 
jdk.incubator.jpackage, it won't be in the module graph. It should work fine if 
you run with:

$ java --add-modules jdk.incubator.jpackage ...


I’m not understanding the module subtleties yet but yes that does work command 
line. Other than -add-modules into the runtime are there any special 
considerations for using it from an application?

Ah, the obvious. Same solution for application also works. I can 
programmatically invoke jpackage with this.


Good.


I am still wondering for the application embedded runtime exec not finding 
linked native commands if this is expected not to work or is considered an 
issue?


This remains a question for me. Should Runtime exec find the native commands 
included with an application embedded JRE? It currently doesn’t seem to on OS X.


Unless that JRE's bin directory is in your PATH, I wouldn't expect it to 
find it.


-- Kevin

Re: jpackage current status

[Kevin Rushforth]

Since your ToolProvider-based program doesn't explicitly require 
jdk.incubator.jpackage, it won't be in the module graph. It should work 
fine if you run with:


$ java --add-modules jdk.incubator.jpackage ...

-- Kevin


On 2/24/2020 8:23 AM, Michael Hall wrote:



On Feb 22, 2020, at 7:02 PM, Michael Hall > wrote:


Currently, at least for Runtime exec including the commands with the 
application does no good.


Just to check would this be known expected behavior?
Or a known issue?
Or something that should have a bug report filed against it?

I additionally looked at using ToolProvider with jpackage as it is 
indicated to support that but it does not appear to work either at 
this time.
This can be verified independently of my application. I’m not real 
familiar but based on a googled example.


import java.util.spi.ToolProvider;


public class ToolProviderTest {

public static void main(String[] args) {
ToolProvider tool = ToolProvider.findFirst(args[0])
.orElseThrow(() -> new IllegalStateException("can not find tool " + 
args[0]));

System.out.println("tool " + tool);
}

}

(base) Michaels-MBP:~ mjh$ java -cp . ToolProviderTest jlink
tool jdk.tools.jlink.internal.Main$JlinkToolProvider@1f32e575
(base) Michaels-MBP:~ mjh$ java -cp . ToolProviderTest jar
tool sun.tools.jar.JarToolProvider@2a84aee7
(base) Michaels-MBP:~ mjh$ java -cp . ToolProviderTest jpackage
Exception in thread "main" java.lang.IllegalStateException: can not 
find tool jpackage

at ToolProviderTest.lambda$main$0(ToolProviderTest.java:8)
at java.base/java.util.Optional.orElseThrow(Optional.java:401)
at ToolProviderTest.main(ToolProviderTest.java:8)
(base) Michaels-MBP:~ mjh$ java --list-modules | grep jpackage
jdk.incubator.jpackage@14

Probably due to jpackage incubator status? But for now programmatic 
testing of jpackage does not seem possible.

Re: jpackage current status

[Kevin Rushforth]

The Failure to GetJREPath is due to JDK-8238225 [1]. Eclipse was 
specifically mentioned as being affected by this. It is already fixed in 
JDK 15.


-- Kevin

https://bugs.openjdk.java.net/browse/JDK-8238225


On 2/22/2020 7:32 AM, Michael Hall wrote:



On Feb 21, 2020, at 11:18 AM, Michael Hall <mailto:mik3h...@gmail.com>> wrote:




On Feb 21, 2020, at 11:12 AM, Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> wrote:


I doubt this has anything to do with jpackage being in incubator or 
not. Fundamentally, just copying binary launchers into another JDK 
image like you are doing is only going to work by accident, if it 
works at all. If you need jpackage (or javac or jar or ...) to be in 
a JDK image, then you should jlink it yourself, include all of the 
modules you need, and don't strip the executables.


-- Kevin



I was doing that for applications going back to before jlink - 
application build tools have not handled this real consistently and 
often just strip. Possibly now you are correct. jpackage does use 
jlink though? So why would my own be better than what the tool does? 
I believe I showed the module is present in the runtime in use.
But again doing jlink separately might in this instance be the best 
practice. I will try it.


Well this went a little wrong and is sort of getting messed up.
I tried a jlink jre. The app froze (OS X) and had to be force quit.
Left it at that for the time being.
This morning Eclipse won’t start.

Double clicking the app executable shows…

/Users/mjh/java-2019-06/Eclipse.app/Contents/MacOS/eclipse ; exit;
Error: could not find libjava.dylib
Failed to GetJREPath()

I tried falling back to the previous ea.
java -version
openjdk version "14-ea" 2020-03-17

But same thing.

Some searching seems to be showing this as the problem here…
https://bugs.openjdk.java.net/browse/JDK-8213362
For both versions. Current and earlier ea libjli.dylib appears to be 
both in the lib and MacOS directories.


I will probably try reinstalling Eclipse with the earlier ea in 
JavaVirtualMachines. A setup that had been working.


Just to mention something else a little off that needs working around 
although I don’t think it should mess anything up. Some of these 
commands are still not default available on OS X. No jpackage of 
course, but no jlink either. jdeps is there but I don’t remember if I 
added a link myself or not there.


For jpackage and jlink I do this…

PACKAGER=`/usr/libexec/java_home`/bin/jpackage

${PACKAGER} \
--input ../HalfPipe12.app/Contents/Java \
--icon GenericApp.icns \

The entire jlink is…
LINKER=`/usr/libexec/java_home`/bin/jlink

${LINKER}  --strip-debug --no-header-files --no-man-pages \
 --bind-services \
 --add-modules java.compiler,java.desktop,java.logging,java.management,java.prefs,java.se 
<https://urldefense.com/v3/__http://java.se__;!!GqivPVa7Brio!LN8Jwr5g_1Q6p_o6Ji2du-gkN-xEns-PN8EhrZx95tLW1Cx8wJziaz3hHiZEIOnJfl_I$>,java.rmi,java.scripting,java.sql,java.xml,jdk.attach,jdk.jshell,jdk.crypto.ec,jdk.incubator.jpackage 
\

 --output runtime

Trying to follow the defaults indicated for jpackage.

Re: jpackage current status

[Kevin Rushforth]

I doubt this has anything to do with jpackage being in incubator or not. 
Fundamentally, just copying binary launchers into another JDK image like 
you are doing is only going to work by accident, if it works at all. If 
you need jpackage (or javac or jar or ...) to be in a JDK image, then 
you should jlink it yourself, include all of the modules you need, and 
don't strip the executables.


-- Kevin


On 2/21/2020 9:04 AM, Michael Hall wrote:



On Feb 21, 2020, at 9:39 AM, Michael Hall  wrote:


You can't copy launchers in this way as it requires the module to be in the 
run-time image.

If I add modules it into the build runtime I think I’m ok but haven’t tried it 
yet.

jpackage seems to need more than just the module in the run-time image.
I did the jpackage build again including it in the --add-modules

This now shows it there…

exec java --list-modules | grep jpackage
System.in:35:jdk.incubator.jpackage@14

However, I still get this…

exec jpackage --version
java.io.IOException: Cannot run program "jpackage": error=2, No such file or 
directory
at java.base/java.lang.ProcessBuilder.start(Unknown Source)
at java.base/java.lang.ProcessBuilder.start(Unknown Source)

Running it directly now works…

./jpackage --version
WARNING: Using incubator modules: jdk.incubator.jpackage
14

Which I think again shows the module is included in the run-time image.

Possibly there is some other dependency. I’ll have to figure that out or wait 
until jpackage is out of incubator.

Re: RFR: JDK-8238168: Remove Copyright from WinLauncher.template

[Kevin Rushforth]

Looks good.

+1

-- Kevin

On 1/29/2020 10:34 AM, Andy Herrick wrote:

Please review trivial jpackage fix to [1] at [2]

[1] https://bugs.openjdk.java.net/browse/JDK-8238168

[2] http://cr.openjdk.java.net/~herrick/8238168/webrev.01/


/Andy

Re: RFR: JDK-8237607: [macos] Signing app bundle with jpackage fails if runtime is already signed

[Kevin Rushforth]

Looks good to me.

+1

-- Kevin


On 1/22/2020 8:37 PM, Alexander Matveev wrote:

Please review the jpackage fix for bug [1] at [2].

- Fixed by forcing signing even if runtime already signed.
- Original implementation was not taken in consideration that runtime 
can be signed (JDK itself is signed from which jpackage is used or 
runtime provided via --runtime-image).
- Signature of binaries files in runtime will not be change, with this 
fix we will generate new _CodeSignature/CodeResources file which 
contains signatures of all files inside runtime folder signed with 
user provided certificate.


[1] https://bugs.openjdk.java.net/browse/JDK-8237607

[2] http://cr.openjdk.java.net/~almatvee/8237607/webrev.00/

Thanks,
Alexander