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: 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: 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: 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: 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: 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: 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: [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: 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: 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-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: 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: 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: 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: 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: 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 [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: 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: 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: JEP 343: Packaging Tool

[Kevin Rushforth]

Hi Mark,

Thank you for your detailed review of jpackage. Given that RDP1 is only 
a week away, I agree that we don't have time to address your feedback. 
Either Andy or I will move it back to "Candidate" and then proceed as 
you recommend.


-- Kevin


On 6/5/2019 2:16 PM, mark.reinh...@oracle.com wrote:

I saw that you moved JEP 343 to “Proposed to Target,” so I spent a
couple of hours looking at it.  You’ve made good progress but I don’t
think this is in the “nearly finished” state that we ask of features
in the six-month cadence.  I suggest that you move this JEP back to
Candidate for now and continue refining it.  The next feature release,
JDK 14, is just six months away.

Some specific observations and suggestions:

   - At the moment there are 75 open issues in JBS [1], 69 of which are
 P3 or higher.  JDK 13 will enter RDP 1 next week, so there’s not
 much time to make progress on all those issues.

   - The use of the term “installer” in the JEP and in the command-line
 options is confusing.  The tool only creates installers on Windows
 and macOS (pkg); the other formats that it supports (macOS dmg,
 Linux deb/rpm) are OS-specific packages rather than interactive
 installers.  Consider replacing the term “installer” with “package”
 throughout.  This would also align better with the name of the tool
 itself.

   - It’s not clear why there are distinct subcommands to create an image
 vs. to create an OS-specific package.  Given the name of the tool,
 I’d expect creating a package to be the primary behavior.  An option
 to preserve the image, which is just a temporary result, could make
 sense, as well as another option to skip the creation of the
 package, but I don’t understand the need for subcommands.

   - On a Debian machine I tried to create a package from a trivial,
 two-module application using the command

   $ jpackage create-installer -o /tmp -p lib -m org.openjdk.hello -n hello

 This terminated with exit code 255 and an error message.  In Linux,
 and Unix/POSIX generally, an exit code of 255 means that the exit
 status was out of range.  I suggest you exit with the value 1 on
 errors, at least on Linux.

   - The error message from the above command was:

   Bundler RPM Bundle skipped because of a configuration problem: Can not 
find rpmbuild 4 or newer..
   Advice to fix: Install packages needed to build RPM, version 4 or newer.

 I’m on a Debian machine, trying to create an OS-specific (i.e.,
 Debian) image, so this was a confusing message.  (Yes, I know it’s
 possible to create rpms under Debian if you have the right tools
 installed, but that’s not what I was trying to do here.)

   - What’s more, even though the tool exited on error it still produced
 a Debian package in the output directory, but I found it only by
 accident.

   - Looking at the content of the generated Debian package, the control
 file has many fields that don’t have corresponding jpackage options.
 That could be a problem for some developers.

   - The data in the Debian package would place the application into a
 directory named `/usr/bin/hello`, which is completely wrong.  Please
 see the Filesystem Hierarchy Standard [2] and the Debian Policy
 Manual [3] for details.

   - I tried to create a package that would install into the `/opt`
 directory by appending `--install-dir /opt` to the above command
 line.  The data in the resulting package would indeed install into
 `/opt`, but the structure within that directory would be incorrect.
 There should be an `/opt/hello/bin` directory containing the `hello`
 launcher, and the remainder of the content should be organized per
 the usual conventions [4].

To get this into better shape I suggest that you seek advice and, when
appropriate, reviews from developers who have deep experience with the
rpm and deb package formats (there are several such people here in the
OpenJDK Community).  It’d also be good to get feedback from macOS and
Windows packaging experts, but I don’t personally know of any.

- Mark


[1] 
https://bugs.openjdk.java.net/issues/?jql=project%20in%20(JDK)%20AND%20component%20in%20(tools)%20AND%20Subcomponent%20in%20(jpackage)%20and%20statuscategory%20not%20in%20(Done)
[2] http://www.pathname.com/fhs/pub/fhs-2.3.html#USRBINMOSTUSERCOMMANDS
[3] https://www.debian.org/doc/debian-policy/index.html
[4] 
http://www.pathname.com/fhs/pub/fhs-2.3.html#OPTADDONAPPLICATIONSOFTWAREPACKAGES

Re: JEP 343: Packaging Tool

[Kevin Rushforth]

Andy is looking into a change which should satisfy the requirement to be 
able to produce an intermediate app-image (and later create a package 
from an app-image), while also getting rid of the two sub-commands, and 
making the building of the installer (package) the default.


The idea would be to get rid of the two sub-commands, making "jpackage" 
do what "jpackage create-installer" does today, and add the ability to 
create an app image as an alternative to a .deb, .rpm, etc.


Andy can provide more details, but I think this would be a good 
alternative without the need for two separate tools, which necessarily 
would share all of the code to build an application image.


He filed https://bugs.openjdk.java.net/browse/JDK-8225428 to explore this.

-- Kevin


On 6/6/2019 1:46 PM, mark.reinh...@oracle.com wrote:

2019/6/5 16:09:11 -0700, Alan Snyder :

I haven’t used recent versions of this tool, but I have found it
essential to be able to modify the image before the final package is
created. There is no way that this tool can anticipate all of the
custom configuration that might be needed, and I do not want to
duplicate its ability to create an image and to build a package from
the image.

That’s a fair point.

It does suggest, however, that perhaps this should be two separate
tools -- one to create an OS-specific image, and another to create
an OS-specific package.  They should work well together, of course,
but it’s not clear that they need to be merged into a single tool.

- Mark

Re: jpackage DMG creation trouble

[Kevin Rushforth]

This will likely change so that only a single package is created by 
jpackage. The current EA version creates all possible package types for 
a given platform by default, even when that doesn't make sense (as on a 
typical Linux machine which either has Debian or RPM tools, but 
typically not both).


To answer your question, you can do the following today:

    jpackage create-installer --installer-type pkg

-- Kevin


On 6/11/2019 6:04 PM, Jeff Carpenter wrote:

Hi everyone,

I'm working (for jClarity) on building some installers with jpackage. Now 
whenever I build a dmg image, I get both the app image *and *a pkg installer 
inside said dmg. Is there a way I can get just the app image in the dmg?

Thanks,
Jeff

Re: jpackage DMG creation trouble

[Kevin Rushforth]

Or if you only want the app in the dmg:

    jpackage create-installer --installer-type dmg

-- Kevin


On 6/12/2019 6:02 AM, Kevin Rushforth wrote:
This will likely change so that only a single package is created by 
jpackage. The current EA version creates all possible package types 
for a given platform by default, even when that doesn't make sense (as 
on a typical Linux machine which either has Debian or RPM tools, but 
typically not both).


To answer your question, you can do the following today:

    jpackage create-installer --installer-type pkg

-- Kevin


On 6/11/2019 6:04 PM, Jeff Carpenter wrote:

Hi everyone,

I'm working (for jClarity) on building some installers with jpackage. 
Now whenever I build a dmg image, I get both the app image *and *a 
pkg installer inside said dmg. Is there a way I can get just the app 
image in the dmg?


Thanks,
Jeff

Re: RFR: JDK-8225569: jpackage app-image layout

[Kevin Rushforth]

Looks good.

Btw, there is a trailing whitespace you might want to fix (although that 
can be done later as a bulk update if you prefer).


test/jdk/tools/jpackage/createappimage/JPackageCreateAppImageBase.java:50: 
Trailing whitespace


-- Kevin


On 6/14/2019 7:17 AM, Andy Herrick wrote:

Please review the revised jpackage fix [3] for issue [1]

revised to use Path.of() in JPackagePath test helper and to centralize 
code that creates output dirs and/or makes sure they are empty and 
writable.


[3] http://cr.openjdk.java.net/~herrick/8225569/webrev.02/


/Andy

On 6/13/2019 1:21 PM, Andy Herrick wrote:


Sorry - subject was missing, this is for: JDK-8225569: jpackage 
app-image layout


/Andy

On 6/13/2019 12:59 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8225569
[2] http://cr.openjdk.java.net/~herrick/8225569/webrev.01/ 



/Andy

Re: RFR: JDK-8225428: CLI change to remove "mode", rename to "package", and build only one target

[Kevin Rushforth]

Looks good. I presume you will update the HelpResources.properties for 
other languages before you push?


-- Kevin


On 6/20/2019 9:54 AM, Andy Herrick wrote:

On 6/19/2019 1:34 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


JDK-8225428: CLI change to remove "mode", rename to "package", and 
build only one target


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

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


Minor revision [3] of help and message text in response to feedback:

[3] http://cr.openjdk.java.net/~herrick/8225428/webrev.02/

/Andy

Re: RFR: JDK-8226532: cleanup is not called when jpackage command fails.

[Kevin Rushforth]

Looks good.

-- Kevin

On 6/21/2019 6:01 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


Trivial fix to clean up the temp directory created by jpackage.

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

[2] http://cr.openjdk.java.net/~herrick/8226532/

/Andy

Re: RFR: JDK-8226751: "Exception: ..." for missing file

[Kevin Rushforth]

The rest of the CommandLine class uses nio Paths/Files, so the following 
might be a better fit, and also checks whether the file can be read:


    if (!Files.isReadable(Paths.of(name)))

The rest looks fine.

-- Kevin

On 6/29/2019 6:27 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] http://cr.openjdk.java.net/~herrick/8226751/ 



/Andy

Re: RFR: JDK-8227684 : jpackage must handle win32 mangled names in jli.dll

[Kevin Rushforth]

It looks fine.

-- Kevin


On 7/25/2019 6:20 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).



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

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

Submitted by: Robert Lichtenberger

/Andy

Re: RFR: JDK-8227312: Remove pkg bundle from DMG image.

[Kevin Rushforth]

Looks fine.

-- Kevin

On 7/25/2019 6:24 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

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

/Andy

Re: RFR: JDK-8228744: file associations broken on linux.

[Kevin Rushforth]

Looks good.

-- Kevin


On 7/29/2019 12:19 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] http://cr.openjdk.java.net/~herrick/8228744/

/Andy

Re: RFR: JDK-8227172: revert JDK-8225569 on windows

[Kevin Rushforth]

Looks good.

-- Kevin

On 8/10/2019 3:44 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change will remove the "bin" directory on windows and revert to 
putting the executable(s) directly in output root dir. (dir>/)


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

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

/Andy

Re: RFR: JDK-8229795: Investigate registry key usage and need for --win-registry-name option.

[Kevin Rushforth]

The following will take the last in the list of extensions and use that:

+ entryName = ext.toUpperCase() + "File";


Also, did you mean to upper-case the extension? I note that "File" isn't 
upper-case, nor is the prefix when there isn't an extension.


-- Kevin


On 8/19/2019 12:33 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] http://cr.openjdk.java.net/~herrick/8229795/

/Andy

Re: RFR: JDK-8229795: Investigate registry key usage and need for --win-registry-name option.

[Kevin Rushforth]

OK. Looks good.

-- Kevin


On 8/19/2019 2:09 PM, Andy Herrick wrote:


On 8/19/19 4:53 PM, Kevin Rushforth wrote:

The following will take the last in the list of extensions and use that:

+ entryName = ext.toUpperCase() + "File";
entryName is both set and used within the "for (String ext : 
extensions)" block.



Also, did you mean to upper-case the extension? I note that "File" 
isn't upper-case, nor is the prefix when there isn't an extension.


yes - I meant that - my example was "JNLPFile" we use as name of 
"jnlp" association we use in webstart.


I have no example of mime-type association without any file 
extensions, so was just using the name as is.


/Andy



-- Kevin


On 8/19/2019 12:33 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] http://cr.openjdk.java.net/~herrick/8229795/

/Andy

Re: RFR: JDK-8215381: Investigate if current implementation of --license-file is correct for Debian packages

[Kevin Rushforth]

Looks good. I do have one question. I see that you changed the resource 
name from "resource.deb-copyright" to "resource.copyright-file". Will 
this property be used for the RPM copyright / license file, too? If not, 
would it be better to keep "deb" in the name?


-- Kevin


On 8/19/2019 9:25 AM, Alexey Semenyuk wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- change format of the default copyright file to comply with Debian 
packaging requirements;
- install copyright file in proper location to comply with Debian 
packaging requirements;
- introduce --linux-deb-copyright-file command line option to allow 
override of the default copyright file template


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

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

Thanks,
Alexey

Re: RFR: JDK-8225447: Revise Debian packaging

[Kevin Rushforth]

Looks good.

-- Kevin


On 8/19/2019 9:56 AM, Alexey Semenyuk wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- fix all permission related issues reported by lintian
- cleanup control files to address issues reported by lintian
- fix jtreg tests
- change package installation directory of Debian package from 
/opt/ to /opt/ to make it compliant with Debian 
packaging requirements


Errors reported by lintian on simple test package before the fix is 
[3] and after the fix is [4]


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

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

[3] 
http://cr.openjdk.java.net/~asemenyuk/8225447/8225447.lintian.before_the_fix.txt


[4] 
http://cr.openjdk.java.net/~asemenyuk/8225447/8225447.lintian.after_the_fix.txt


Thanks,
Alexey

Re: jdk-14-jpackage+1-33 on jdk.java.net

[Kevin Rushforth]

We believe that we have addressed most of the issues, especially those 
affecting the generated Linux packages, both .deb and .rpm. There is one 
open issue around the naming of the Debian packages that we will address 
in the next EA release. See JDK-8228660 [1] for more information.


We would love to get some feedback from Linux developers to make sure 
that we didn't miss anything else.


Thanks.

-- Kevin

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


On 8/21/2019 3:27 PM, Andy Herrick wrote:
The next EA build of JPackage is available at 
https://jdk.java.net/jpackage/


This build ( jdk-14-jpackage+1-33 ) (2019/8/20) is the next early 
access release based on JDK-14


This release contains fixes to the following issues:

JDK-8229788 Error dialog displays with DLL issue when installing 
WinChooserTest application

JDK-8225447 Revise Debian packaging
JDK-8213941 Debian linux problems in JavaPackager
JDK-8229334 jpackage .exe packages cannot be executed due to 
missing DLL

JDK-8227058 Regressions related to no longer setting user.dir
JDK-8226599 use code coverage results to remove dead code
JDK-8226191 jpackager --license-file option broken on windows for 
jdk installers.
JDK-8215381 Investigate if current implementation of 
--license-file is correct for Debian packages

JDK-8229138 Add --linux-app-release option for DEB and RPM packages
JDK-8229791 Code clean up regressions
JDK-8229786 No output after WinShortcutTest.exe is launched
JDK-8229750 Fix bad merge of JDK-8215447 patch
JDK-8215446 JPackageCreateInstallerInstallDirTest fails on OLE7
JDK-8215447 Investigate if current implementation of 
--license-file is correct for RPM packages

JDK-8227172 revert JDK-8225569 on windows
JDK-8224788 jpackage fails on OS X when using --runtime-image
JDK-8229252 Add descriptions to Windows jtreg tests
JDK-8228744 file associations broken on linux.
JDK-8227312 Remove pkg bundle from DMG image.
JDK-8228722 jpackage RPM tests fail on some versions of rpmbuild
JDK-8222778 Packaging Tool (JEP 343) on Linux/AArch64
JDK-8224627 Creating installer with --runtime-image on OS X fails
JDK-8226904 current working directory wrong running jpackage app
JDK-8224486 Arguments from jpackager cfg file not processed correctly
JDK-8226835 Command window pops up building exe package
JDK-8225092 Several jpackage tests failed when run with jcov enabled

/Andy

Re: RFR: JDK-8228660: .deb files generated by jpackage don't follow naming convention

[Kevin Rushforth]

Looks good to me, too.

-- Kevin

On 8/27/2019 1:29 PM, Andy Herrick wrote:

looks good.

/Andy

On 8/27/2019 4:13 PM, Alexey Semenyuk wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- fix implementation of --linux-app-release CLI option for Debian 
packaging;
- use value of --linux-app-release CLI option to construct Debian 
package name.


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

[2] http://cr.openjdk.java.net/~asemenyuk/8228660/webrev.03

Thanks,
Alexey

Re: RFR: JDK-8229979: jpackage cleanup src files, help text, and javadoc

[Kevin Rushforth]

Looks good.

-- Kevin

On 8/28/2019 6:36 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


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

[2] http://cr.openjdk.java.net/~herrick/8229979/

Thanks,
Andy

Re: jdk-14-jpackage+1-33 on jdk.java.net

[Kevin Rushforth]

Hi Sverre,

Do you have a WiX installed on your machine? That is a prerequisite.

Andy: Do we have a bug filed to produce a better error message in this 
case? If not, we need to file one.


-- Kevin


On 8/29/2019 7:30 AM, Sverre Moe wrote:

It is not working creating native installer on Windows.

It will not take neither exe nor msi as --package-type on Windows.
jdk.jpackage.internal.PackagerException: Error: Invalid or unsupported 
package type: [exe].
        at 
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:614)
        at 
jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:513)

        at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:97)
        at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:51)

The jpackage help output on Windows lists both exe and msi as valid 
package types.


The JDK-8228660 is marked as resolved. I reckon it will make it into 
the next build.


/Sverre


tor. 22. aug. 2019 kl. 02:03 skrev Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>>:


We believe that we have addressed most of the issues, especially
those
affecting the generated Linux packages, both .deb and .rpm. There
is one
open issue around the naming of the Debian packages that we will
address
in the next EA release. See JDK-8228660 [1] for more information.

We would love to get some feedback from Linux developers to make sure
that we didn't miss anything else.

Thanks.

-- Kevin

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


On 8/21/2019 3:27 PM, Andy Herrick wrote:
> The next EA build of JPackage is available at
> https://jdk.java.net/jpackage/
>
> This build ( jdk-14-jpackage+1-33 ) (2019/8/20) is the next early
> access release based on JDK-14
>
> This release contains fixes to the following issues:
>
> JDK-8229788 Error dialog displays with DLL issue when
installing
> WinChooserTest application
> JDK-8225447 Revise Debian packaging
> JDK-8213941 Debian linux problems in JavaPackager
> JDK-8229334 jpackage .exe packages cannot be executed due to
> missing DLL
> JDK-8227058 Regressions related to no longer setting user.dir
> JDK-8226599 use code coverage results to remove dead code
> JDK-8226191 jpackager --license-file option broken on
windows for
> jdk installers.
> JDK-8215381 Investigate if current implementation of
> --license-file is correct for Debian packages
> JDK-8229138 Add --linux-app-release option for DEB and RPM
packages
> JDK-8229791 Code clean up regressions
> JDK-8229786 No output after WinShortcutTest.exe is launched
> JDK-8229750 Fix bad merge of JDK-8215447 patch
> JDK-8215446 JPackageCreateInstallerInstallDirTest fails on OLE7
> JDK-8215447 Investigate if current implementation of
> --license-file is correct for RPM packages
> JDK-8227172 revert JDK-8225569 on windows
> JDK-8224788 jpackage fails on OS X when using --runtime-image
> JDK-8229252 Add descriptions to Windows jtreg tests
> JDK-8228744 file associations broken on linux.
> JDK-8227312 Remove pkg bundle from DMG image.
> JDK-8228722 jpackage RPM tests fail on some versions of rpmbuild
> JDK-8222778 Packaging Tool (JEP 343) on Linux/AArch64
> JDK-8224627 Creating installer with --runtime-image on OS X
fails
> JDK-8226904 current working directory wrong running jpackage app
> JDK-8224486 Arguments from jpackager cfg file not processed
correctly
> JDK-8226835 Command window pops up building exe package
> JDK-8225092 Several jpackage tests failed when run with jcov
enabled
>
> /Andy
>

Re: RFR: JDK-8230519: jpackage "--package-type" values and default

[Kevin Rushforth]

Looks good with one question:

In Arguments.java:

+ if (bundler.isDefault()) {
+ return bundler;
+ } else {
+ savedBundler = bundler;
+ }

When would there be a valid case where you loop through the list of 
bundlers and don't find a default? It may be better to throw an error in 
that case rather than just return the last one found.


-- Kevin


On 9/8/2019 2:50 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix:

Adds "app-image" as a valid value for "--package-type" options, 
meaning build an application image instead of a package.


Changes the default value of "--package-type" to a platform dependent 
default package type.


[1] https://bugs.openjdk.java.net/browse/JDK-8230519
[2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/

Thanks

Andy,

Re: RFR: JDK-8230649: Make jpackage tool an experimental feature

[Kevin Rushforth]

In that case, it may be better to issue any warnings about it being 
experimental directly from jpackage instead of relying on the incubating 
modules.


-- Kevin


On 9/15/2019 1:05 PM, Andy Herrick wrote:
yes - result of this change is as you suggest, everything shows this 
warning, so we will need further discussions as to what it might mean 
to make jpackage "experimental".


/Andy

On 9/15/2019 12:35 PM, Alan Bateman wrote:

On 15/09/2019 14:58, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This is the at least the first small set of changes that need to be 
make to make jpackage an experimental package.


 - Add flags to the build so the module jdk.jpackage is built as an 
experimental module.


 - Modify test programs to tolerate the warning emitted when 
jpackage is run.
I think this change will need discussion. Can you provide a summary 
on what you mean by "experimental package"? I remember seeing Mark's 
comment go by where he suggested that the tool should be an 
experimental feature but I'm not sure if this translates to a warning 
or a configure option.


I see the JIRA issue references the JEP for Incubating Modules but 
I'm not sure that it makes sense here as jdk.jpackage doesn't export 
an API and will eagerly participate in service binding because it 
`provides java.util.spi.ToolProvider`. There are subtle issues around 
incubating modules that want to provide services that were not worked 
out in the JDK 9 time frame. In this case, java.base uses 
ToolProvider so jdk.jpackage will be resolved when it is observable. 
I assum `java -version` will print a warning and that will not be 
welcomed.


-Alan

Re: RFR: JDK-8230649: Make jpackage tool an experimental feature

[Kevin Rushforth]

On 9/16/2019 6:06 AM, Alan Bateman wrote:

On 16/09/2019 13:56, Kevin Rushforth wrote:
In that case, it may be better to issue any warnings about it being 
experimental directly from jpackage instead of relying on the 
incubating modules.
Incubating modules are incubating non-final APIs so I don't think it's 
the right solution here. I suspect what is needed is a combination of 
a warning message (from the tool) and documentation to make it clear 
that that the feature and CLI options aren't yet final.


Yes, that's what I was thinking as well, now that it's clear that the 
incubating module approach isn't the right one to use.


-- Kevin

Re: Comments on jpackage (JEP 343)

[Kevin Rushforth]

Hi Phil,

In the app-image case it always creates a new directory with the name of 
the application underneath the dest/output directory for holding all of 
the files. So you either have:


   /appname.ext

or

   /appname/

So I think "." is a reasonable default if not specified.

As to whether to call it "--dest" or "--output", I don't have a strong 
opinion. There are precedents for both.


-- Kevin


On 9/16/2019 8:24 PM, Philip Race wrote:

> but I'd concede it to be "." as a default

On second thoughts I am not sure about that either.
I find it much cleaner to know what was generated by looking in a new 
directory rather than
hunting in my current directory, especially for the default app-image 
case which will dump

a bunch of unfamiliar files.

-phil.

On 9/16/19, 8:13 PM, Philip Race wrote:

I've been thinking about this.
output is nicely symmetrical with input and in the case of a default 
app-image it is more than a single item
and personally I'd much rather it not clutter my working directory 
and again you get symmetry with input
which you really want to specify  but I'd concede it to be "." as a 
default over changing the name to dest ...


There is also precedent :
jlink uses -output and these two tools are going to be frequently 
used together.


So I would like to see the status quo.

-phil.


On 9/3/19, 11:58 AM, mark.reinh...@oracle.com wrote:
   - The `--output`/`-o` option is confusing.  It doesn’t name the 
output
 itself, but rather a directory into which the single item of 
output
 will be placed.  Typing `-o .` all the time is just annoying.  
It’d
 be more logical to rename this option to `--dest`/`-d` and to 
make it

 optional, with a default value of `.`.

Re: Comments on jpackage (JEP 343)

[Kevin Rushforth]

As to why it has this Requires, perhaps I need to address it at the JavaFX
mailinglist.


Yes, that would be best, as this is no longer related to jpackage.

-- Kevin


On 9/19/2019 12:43 AM, Sverre Moe wrote:

Yes it would seem so.

I am still perplexed to why it has an RPM Requires libavcodec-ffmpeg.so.56.
That one does not exist as far as I can find anywhere. It is not provided
with the ffmpeg package.
I thought perhaps it was the same library as libavcodec.so.56 only named
differently, but considering that also is listed among the Requires it is a
distinct package.
As to why it has this Requires, perhaps I need to address it at the JavaFX
mailinglist.

How does jpackage accumulate its RPM Requires list? I thought it had it in
a default RPM spec file, but I don't see how that can be, we use our own
custom spec file, and jpackage adds many additional Requires not used in
our own. Only when javafx-web is within the built runtime image used with
jpackage, is these extra Requires used.
The RPM spec template file does not have any hard coded Requires, only
those from PACKAGE_DEPENDENCIES. I looked around in the source code, and
could only see this was set from the command line arguments for
dependencies. Does it scan the runtime image to produce its additional
Requires list? I have the javafx as dependencies in my project, but those
are not "scanned" for linux package dependencies.
https://hg.openjdk.java.net/jdk/sandbox/file/bf06a1d3aef6/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/

Trying with the --linux-package-deps none of those dependencies ended up in
the built RPM package. Only those from jpackage and those from our custom
spec file.

/Sverre


tor. 19. sep. 2019 kl. 01:09 skrev Michael Paus :


If you don't use audio or video, you will probably not need it.

Have a look here: https://ffmpeg.org/


Message: 2
Date: Wed, 18 Sep 2019 23:44:19 +0200
From: Sverre Moe 
To: Philip Race 
Cc: core-libs-dev@openjdk.java.net
Subject: Re: Comments on jpackage (JEP 343)
Message-ID:
   
qakvdetirxlfcny0cvh4poezg...@mail.gmail.com>

Content-Type: text/plain; charset="UTF-8"

It is actually the javafx-web module that requires these.

Anyone know what part of the Web module that needs the libavcodec and
libavcoded-ffmpeg?

We are using a small part of the web module, just WebView for Tooltip

with

HTML. No Video or Audio.

/Sverre

Re: Comments on jpackage (JEP 343)

[Kevin Rushforth]

OK, that makes sense. Andy has already implemented this change (pushed 
it to the sandbox), so it will be in the next EA build.


-- Kevin

On 9/19/2019 10:25 AM, mark.reinh...@oracle.com wrote:

jlink’s -o/--output option names exactly the thing being output,
rather than a container for the thing being output.

The jpackage option we’re discussing here names a container for the
thing being output, much like the -d option of javac and javadoc.

Using -d/--dest for jpackage is consistent with the JDK’s other
command-line tools.

- Mark

Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature

[Kevin Rushforth]

On 9/25/2019 7:44 AM, Alan Bateman wrote:

On 25/09/2019 15:33, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


These are the code changes to make jpackage an experimental feature.

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

[2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/
I assume the change to CreateJmods.gmk is not needed as the module 
will be resolved because it provides an implementation of ToolProvider.


I was going to point out the same thing as Alan. The rest looks good, 
although an explicit informational line in the --help message might be 
nice, in case log warnings are going somewhere else.


-- Kevin

Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature

[Kevin Rushforth]

That seems fine then. Thanks.

-- Kevin


On 9/25/2019 8:29 AM, Andy Herrick wrote:

I can will remove the makefile change then.

The warning is going the same place as the help output (Log.info()) 
which is stdout unless a different stream is passed to ToolProvider.


So if we want a more detailed message in Help text we can either show 
the more detailed message in addition to the message shown in all 
cases, or specifically exclude the generic message in the Help case.


/Andy

On 9/25/19 11:18 AM, Kevin Rushforth wrote:



On 9/25/2019 7:44 AM, Alan Bateman wrote:

On 25/09/2019 15:33, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


These are the code changes to make jpackage an experimental feature.

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

[2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/
I assume the change to CreateJmods.gmk is not needed as the module 
will be resolved because it provides an implementation of ToolProvider.


I was going to point out the same thing as Alan. The rest looks good, 
although an explicit informational line in the --help message might 
be nice, in case log warnings are going somewhere else.


-- Kevin

Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature

[Kevin Rushforth]

Looks good.

-- Kevin


On 9/26/2019 3:57 AM, Andy Herrick wrote:
Revised [3] to remove the change to CreateJmods.gmk since it is not 
needed as the module will be resolved because it provides an 
implementation of ToolProvider.


[3] http://cr.openjdk.java.net/~herrick/8230649/webrev.03/

/Andy

On 9/25/2019 10:33 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


These are the code changes to make jpackage an experimental feature.

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

[2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/

/Andy

Re: RFR: JDK-8231912: Use https for URLs in help and error messages

[Kevin Rushforth]

Looks good.

-- Kevin


On 10/15/2019 9:12 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change modifies an number of urls used, mostly just changing http 
to https.


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

[2] https://cr.openjdk.java.net/~herrick/8231912/webrev.02/

/Andy

Re: fx:deploy is not available in this JDK

[Kevin Rushforth]

> Error:Java FX Packager: fx:deploy task has failed.

The 'fx:deploy' task was part of the ant plugin that was formerly 
distributed in ant-javafx.jar as part of javapackager. This ant plugin 
is not included in jpacakge, so you will need to find some other way to 
integrate jpacakge into IntelliJ.


-- Kevin


On 10/28/2019 9:18 AM, Olga Klisho wrote:

Hello,

JavaFX application successfully compiles in IntelliJ IDEA 2019.3
 with the use of:
- *Build 14-jpackage+1-35 (2019/8/29)* (http://jdk.java.net/jpackage);
- Early-Access builds for JavaFX 14 (https://gluonhq.com/products/javafx).

The same project fails to compile with* Build 14-jpackage+1-49 (2019/10/2)*
with exception attached.

JavaFX artifacts can't be built with both mentioned jpackage builds.
Please see the sample project where the issue is reproduced attached to the
issue:
https://youtrack.jetbrains.com/issue/IDEA-200721

In case of any questions or if more details are needed please feel free to
contact me. Thank you

Olga Klisho
IDEA Support Engineer
JetBrains
http://www.jetbrains.com
The Drive to Develop

Re: RFR: JDK-8233265: jpackage --add-modules cannot find additional modules with non-modular app

[Kevin Rushforth]

Looks good.

-- Kevin


On 10/31/2019 2:02 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This is a fixes a regression from the fix to JDK-8231882 
 and improves 
behavior when module listed is not in the module path.


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

[2] http://cr.openjdk.java.net/~herrick/8233265/

/Andy

Re: RFR: JDK-8233592: change --package-type option name to --type and allow -t short form

[Kevin Rushforth]

+1

On 11/5/2019 9:47 PM, Alexey Semenyuk wrote:

Looks good.

- Alexey

On 11/5/2019 3:39 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This changes the --package-type option to just --type , and adds the 
-t short form.


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

[2] https://cr.openjdk.java.net/~herrick/8233592/ 



/Andy

Re: RFR: JDK-8233594: create a new option --bind-servces to pass on to jlink

[Kevin Rushforth]

Yes, this is a good change.

+1

-- Kevin

On 11/6/2019 2:02 AM, Philip Race wrote:
I noticed this problem when creating various demo scenarios so +1 to 
the change, but


+\  Pass on --bind-seervices option to jlink (which will link 
in \n\


typo here

-phil

On 11/5/19, 3:36 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This changes adds the --bind-services option and removes the previous 
behavior of always including --bind-services in the jlink options 
except when --add-modules was specified.


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

[2] - http://cr.openjdk.java.net/~herrick/8233594/

/Andy

Re: RFR: JDK-823359: Reorder jpackage help text to focus on package

[Kevin Rushforth]

It seems fine, but it might be even better to have both the non-modular 
and modular application examples be for the default package case and 
have a single example of --app-image. Also, I presume `--package-type` 
will be replaced with `--type` (or `-t`) to match the change in command 
line option name?


-- Kevin


On 11/8/2019 6:39 PM, Alexey Semenyuk wrote:

Looks good

On 11/8/2019 11:03 AM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix reorders the sample usage in the help text.

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

[2] https://cr.openjdk.java.net/~herrick/8233591

/Andy

Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

[Kevin Rushforth]

I disagree with a couple of these changes, but they can be fixed in a 
subsequent pass, since you've already pushed the changes.


By convention, class names are camel case with leading upper-case 
letter, so ClassName expresses that better than className. Similarly, 
MyJar.jar seems better to me than myJar.jar. By convention, package 
names don't have upper-case letters at all, so package.name or `mypkg` 
might be better than packageName.


-- Kevin

On 11/9/2019 2:50 AM, Alexey Semenyuk wrote:

Looks good.

- Alexey

On 11/8/2019 4:31 PM, Andy Herrick wrote:

revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks 
like we have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, 
inputdir, MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section of 
help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

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

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy

Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

[Kevin Rushforth]

OK.

-- Kevin


On 11/9/2019 11:46 AM, Alexey Semenyuk wrote:

Kevin,

My point was to add consistency. We used to have class names in camel 
case and pascal case (className, ClassName) and this didn't look right 
for me. I agree ClassName, `mypkg` is more appropriate. I'm neutral 
about myJar.jar and MyJar.jar.


- Alexey

On 11/9/2019 12:37 AM, Kevin Rushforth wrote:
I disagree with a couple of these changes, but they can be fixed in a 
subsequent pass, since you've already pushed the changes.


By convention, class names are camel case with leading upper-case 
letter, so ClassName expresses that better than className. Similarly, 
MyJar.jar seems better to me than myJar.jar. By convention, package 
names don't have upper-case letters at all, so package.name or 
`mypkg` might be better than packageName.


-- Kevin

On 11/9/2019 2:50 AM, Alexey Semenyuk wrote:

Looks good.

- Alexey

On 11/8/2019 4:31 PM, Andy Herrick wrote:

revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks 
like we have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, 
inputdir, MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section 
of help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open 
sandbox repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

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

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Kevin Rushforth]

I took the "git diff" patch [5] that you uploaded yesterday, applied it, 
and verified that it is the same as what is in the JDK-8200758-branch 
branch of the sandbox. It builds and runs fine for me.


I ran jcheck and it found the following three files with whitespace 
errors that will need to be fixed before you push:


src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: 
Trailing whitespace
src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: 
Trailing whitespace

test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace

The second of these will go away with the fix for JDK-8234402 [6], so 
you don't need to do anything there. Once the fix for JDK-8234402 is 
pushed to sandbox, I presume you will update the webrev, right?


Pending the fix for JDK-8234402 and the needed white-space fixes, this 
is a +1 from me (although I am not a jdk Project Reviewer, so you will 
need at least one review from someone who is).


-- Kevin

[5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch
[6] https://bugs.openjdk.java.net/browse/JDK-8234402


On 11/13/2019 4:23 PM, Andy Herrick wrote:
Please review  changes for [1] which is the implementation bug for 
JEP-343.


The webrev at [2] is the total cumulative webrev of changes for the 
jpackage tool, currently in the JDK-8200758-branch branch of the open 
sandbox repository.


The webrev at [3] shows the changes since EA-06 (Build 
13-jpackage+1-49 ) up to the current point.


The latest EA (Build 14-jpackage+1-49 ) is posted at [4].

Please send feedback to core-libs-dev@openjdk.java.net


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

[2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/

[3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/

[4] http://jdk.java.net/jpackage/

Re: RFR: JDK-8234402: revert change that stopped providing JPackageToolProvider

[Kevin Rushforth]

Good point. Looks good to me once this is fixed.

-- Kevin


On 11/19/2019 6:00 PM, Alexey Semenyuk wrote:

Andy,

I guess 
http://cr.openjdk.java.net/~herrick/8234402/webrev.02/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.sdiff.html 
can be reverted to its initial state now:

---
public ToolProvider asToolProvider() {
    return ToolProvider.findFirst(name).orElse(null);
}
---

- Alexey

On 11/19/2019 7:00 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change restores JPackageToolProvider and gets rid of the 
temporary factory class.


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

[2] http://cr.openjdk.java.net/~herrick/8234402/webrev.02/

/Andy

Re: RFR: JDK-8234402: revert change that stopped providing JPackageToolProvider

[Kevin Rushforth]

Looks good.

-- Kevin


On 11/20/2019 6:36 AM, Andy Herrick wrote:

webrev revised in place at [2].

/Andy

On 11/19/2019 9:00 PM, Alexey Semenyuk wrote:

Andy,

I guess 
http://cr.openjdk.java.net/~herrick/8234402/webrev.02/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.sdiff.html 
can be reverted to its initial state now:

---
public ToolProvider asToolProvider() {
    return ToolProvider.findFirst(name).orElse(null);
}
---

- Alexey

On 11/19/2019 7:00 PM, Andy Herrick wrote:

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

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This change restores JPackageToolProvider and gets rid of the 
temporary factory class.


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

[2] http://cr.openjdk.java.net/~herrick/8234402/webrev.02/

/Andy

Re: jpackage ROOTDIR variable in a Windows bat file

[Kevin Rushforth]

$ROOTDIR is not a supported interface. You should use "$APPDIR" instead. 
Having said that, the real problem is likely your use of backslashes. I 
recommend using forward slashes and also enclosing it in single quotes, 
like this:


    -Djava.security.policy='$APPDIR/all.policy'

If you need to use backslashes for some reason, then you will need to 
use two (the first is treated as an escape character).


-- Kevin


On 11/23/2019 8:37 AM, Michael Hall wrote:

I am trying to come up with a simple Windows bat file execution of jpackage. I 
would like to use the builtin ROOTDIR variable. How should this be done?
Not very familiar with Windows.

This
-Djava.security.policy=$ROOTDIR\app\all.policy
Gets
Error: Invalid Option: [-Djava.security.policy=$ROOTDIR\app\all.policy]

This
-Djava.security.policy=%ROOTDIR%\app\all.policy
Gets this
  Error: Invalid Option: [-Djava.security.policy=\app\all.policy]

These are included in the —java-options parameter.

This thread seems to end up indicating that $ROOTDIR should work on windows
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063361.html 

Although it is for finding the runtime and not included in —java-options

Re: jpackage ROOTDIR variable in a Windows bat file

[Kevin Rushforth]

I just tried this and it worked fine for me:

jpackage --java-options '-Djava.security.policy=$APPDIR/all.policy' ...

(note that there should not be an extra "/app" since $APPDIR points to 
the app directory).


It generated this in the ApplicationName/app/ApplicationName.cfg file:

[JavaOptions]
-Djava.security.policy=$APPDIR/all.policy


-- Kevin



On 11/23/2019 9:12 AM, Michael Hall wrote:



With forward slashes

Error: Invalid Option: [-Djava.security.policy=$ROOTDIR/app/all.policy]

Sorry that should of been APPDIR. manually cutting and pasting with edit since 
I can’t currently cut and paste directly between Windows VirtualBox and OS X.

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Kevin Rushforth]

This all looks good.

+1

-- Kevin


On 11/26/2019 12:56 PM, Erik Joelsson wrote:

(adding build-dev)

Build changes look good.

/Erik

On 2019-11-20 09:37, Andy Herrick wrote:
I posted new composite webrev [7], and git patch [8] after pushing 
fix for JDK-8234402 [6].


[7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/

[8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch

/Andy

On 11/19/2019 3:13 PM, Kevin Rushforth wrote:
I took the "git diff" patch [5] that you uploaded yesterday, applied 
it, and verified that it is the same as what is in the 
JDK-8200758-branch branch of the sandbox. It builds and runs fine 
for me.


I ran jcheck and it found the following three files with whitespace 
errors that will need to be fixed before you push:


src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: 
Trailing whitespace
src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: 
Trailing whitespace
test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing 
whitespace


The second of these will go away with the fix for JDK-8234402 [6], 
so you don't need to do anything there. Once the fix for JDK-8234402 
is pushed to sandbox, I presume you will update the webrev, right?


Pending the fix for JDK-8234402 and the needed white-space fixes, 
this is a +1 from me (although I am not a jdk Project Reviewer, so 
you will need at least one review from someone who is).


-- Kevin

[5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch
[6] https://bugs.openjdk.java.net/browse/JDK-8234402


On 11/13/2019 4:23 PM, Andy Herrick wrote:
Please review  changes for [1] which is the implementation bug for 
JEP-343.


The webrev at [2] is the total cumulative webrev of changes for the 
jpackage tool, currently in the JDK-8200758-branch branch of the 
open sandbox repository.


The webrev at [3] shows the changes since EA-06 (Build 
13-jpackage+1-49 ) up to the current point.


The latest EA (Build 14-jpackage+1-49 ) is posted at [4].

Please send feedback to core-libs-dev@openjdk.java.net


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

[2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/

[3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/

[4] http://jdk.java.net/jpackage/

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Kevin Rushforth]

Updated version (with the one change mentioned in reply to Phil) looks good.

+1

-- Kevin


On 12/3/2019 11:35 AM, Andy Herrick wrote:

Please review the revised cumulative jpackage webrev at [3]

/Andy

[3] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11D/


On 11/18/2019 12:01 PM, Andy Herrick wrote:


On 11/13/2019 7:23 PM, Andy Herrick wrote:
Please review  changes for [1] which is the implementation bug for 
JEP-343.


The webrev at [2] is the total cumulative webrev of changes for the 
jpackage tool, currently in the JDK-8200758-branch branch of the 
open sandbox repository.


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

[2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/

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

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: 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: 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]

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]

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 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: 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: 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: 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: 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: 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: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 [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: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 [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: 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: 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: 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: 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: 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: 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: 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-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: 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-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-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: 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: 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: 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: 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 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: 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: 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: 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 [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