from:"Alexander Matveev"

On Fri, 31 May 2024 14:05:07 GMT, Alexey Semenyuk  wrote:

> Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app 
> launcher instead of raw Executor. This makes MainClassTest test run app 
> launchers with retries. This change addresses the primary issue.
> 
> Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide 
> API allowing to run launchers without retries. It inconsistently allowed the 
> execution of launchers with suppressed output (stdout and stderr). It 
> inconsistently executed launchers with/without PATH removed from the 
> environment.
> 
> These loopholes were eliminated:
> 
>  - stdout and stderr of app launchers is never suppressed;
>  - PATH env variable is always deleted for app launchers on Windows. It is 
> not deleted on other platforms. This change sets the correct scope of 
> [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that 
> introduced the removal of PATH env variable for app launchers;
>  - app launchers are always executed with retries unless the launcher is 
> executed with `jpackage.test.noexit` system property set to `true` indicating 
> the test app will not terminate on its own.
> 
> Other changes are due to changes in HelloApp.AppOutputVerifier class.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19502#pullrequestreview-2091922907

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option [v2]

On Fri, 31 May 2024 21:36:56 GMT, Nir Lisker  wrote:

> I see, but it doesn't say where to put license files, which are usually in 
> the root. Do you know where these belong?

No idea.

-

PR Comment: https://git.openjdk.org/jdk/pull/19377#issuecomment-2143025767

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option

On Fri, 31 May 2024 20:43:45 GMT, Nir Lisker  wrote:

> > > For your example. This almost seems like an Apple bug if you can add a 
> > > directory to the Contents directory but not a file?
> > 
> > 
> > Not sure if it is an Apple bug.
> 
> Can this be reported to Apple somehow?

I do not think that it is an Apple bug. If you look at an Apple documentation 
[1] about "Placing content in a bundle" you will see highlighted important 
message that you should not put content in the wrong location.

[1] 
https://developer.apple.com/documentation/bundleresources/placing_content_in_a_bundle?language=objc

-

PR Comment: https://git.openjdk.org/jdk/pull/19377#issuecomment-2142981609

Integrated: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option

On Fri, 24 May 2024 01:08:03 GMT, Alexander Matveev  
wrote:

> This issue is reproducible with and without `--mac-sign`. jpackage will 
> "_ad-hoc_" sign application bundle when `--mac-sign` is not specified by 
> using pseudo-identity "_-_". This is why jpackage tries to sign added files 
> and this is expected behavior by jpackage. "codesign" fails since added 
> content made application bundle structure invalid. There is nothing we can do 
> on jpackage side to sign such invalid bundles. As proposed solution we will 
> output possible reason for "codesign" failure if it fails and `--app-content` 
> was specified and possible solution. Proposed message: "One of the possible 
> reason for "codesign" failure is additional content provided via 
> "--app-content", which made application bundle structure invalid. Make sure 
> to provide additional content in a way it will not break application bundle 
> structure, otherwise add additional content as post-processing step."
> 
> Example:
> Lets assume we have "ReadMe" folder with "ReadMe.txt" file in it.
> 1) jpackage --type app-image -n Test --app-content ReadMe/ReadMe.txt ...
> "codesign" will fail with "In subcomponent: Test.app/Contents/ReadMe.txt". 
> This is expected and "ReadMe.txt" placed in "Test.app/Contents" which is also 
> expected.
> 2) jpackage --type app-image -n Test --app-content ReadMe ...
> Works and "ReadMe.txt" will be placed under "Test.app/Contents/ReadMe".
> 
> Sample output before fix:
> 
> Error: "codesign" failed with following output:
> Test.app: replacing existing signature
> Test.app: code object is not signed at all
> In subcomponent: Test.app/Contents/ReadMe.txt
> 
> 
> Sample output after fix:
> 
> "codesign" failed and additional application content was supplied via the 
> "--app-content" parameter. Probably the additional content broke the 
> integrity of the application bundle and caused the failure. Ensure content 
> supplied via the "--app-content" parameter does not break the integrity of 
> the application bundle, or add it in the post-processing step.
> Error: "codesign" failed with following output:
> Test.app: replacing existing signature
> Test.app: code object is not signed at all
> In subcomponent: Test.app/Contents/ReadMe.txt

This pull request has now been integrated.

Changeset: 9fd0e734
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/9fd0e7349ebf4a49b5c0c7a16c866b5b8e626b53
Stats: 39 lines in 6 files changed: 25 ins; 0 del; 14 mod

8332110: [macos] jpackage tries to sign added files without the --mac-sign 
option

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/19377

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option [v2]

On Thu, 30 May 2024 14:21:51 GMT, Alexey Semenyuk  wrote:

>> Alexander Matveev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8332110: jpackage tries to sign added files without the --mac-sign option 
>> [v2]
>
> test/jdk/tools/jpackage/macosx/SigningOptionsTest.java line 97:
> 
>> 95: new String[]{"--app-content", TEST_DUKE},
>> 96: null,
>> 97: "\"codesign\" failure is additional content provided 
>> via \"--app-content\""},
> 
> Why is this not a `One of the possible reason for "{0}" failure is additional 
> content provided via "--app-content"`?

It was part of old message. Latest test version has latest message: ""codesign" 
failed and additional application content was supplied via the "--app-content" 
parameter."

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19377#discussion_r1622884704

Re: RFR: 8333303: Issues with DottedVersion class

On Thu, 30 May 2024 20:10:05 GMT, Alexey Semenyuk  wrote:

> - Get rid of DottedVersion#greedy field.
>  - Add support to save the unrecognizable remainder of the version string 
> (required to handle Wix4 version string).
>  - Implement DottedVersion#equals().
>  - add DottedVersion#compareComponents(DottedVersion, DottedVersion) that 
> compares recognized components of the given DottedVersion instances.
>  - remove DottedVersion#compareTo(String)
> 
> [Edit](https://bugs.openjdk.org/secure/EditComment!default.jspa?id=5130726=14677610)
> [Delete](https://bugs.openjdk.org/secure/DeleteComment!default.jspa?id=5130726=14677610)

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19488#pullrequestreview-2089507390

Re: RFR: 8333307: Don't suppress jpackage logging in tests when it is detecting packaging tools in the system

On Thu, 30 May 2024 20:26:02 GMT, Alexey Semenyuk  wrote:

> Change jpackage tests output from:
> 
> 
> [19:16:29.586] Create: SimplePackageTest.test
> [19:16:29.587] [ RUN ] SimplePackageTest.test
> [19:16:29.663] TRACE: Bundler rpm supported
> [19:16:29.674] TRACE: Actions: [[initialize], [create], [unpack], 
> [verify-install], [finalize]]
> 
> 
> to:
> 
> 
> [19:16:29.586] Create: SimplePackageTest.test
> [19:16:29.587] [ RUN ] SimplePackageTest.test
> [19:16:29.621] Running dpkg
> [19:16:29.633] Running dpkg-deb
> [19:16:29.651] Running rpmbuild
> [19:16:29.663] TRACE: Bundler rpm supported
> [19:16:29.674] TRACE: Actions: [[initialize], [create], [unpack], 
> [verify-install], [finalize]]

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19489#pullrequestreview-2089482543

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option [v2]

On Thu, 30 May 2024 22:54:12 GMT, Alexander Matveev  
wrote:

>> This issue is reproducible with and without `--mac-sign`. jpackage will 
>> "_ad-hoc_" sign application bundle when `--mac-sign` is not specified by 
>> using pseudo-identity "_-_". This is why jpackage tries to sign added files 
>> and this is expected behavior by jpackage. "codesign" fails since added 
>> content made application bundle structure invalid. There is nothing we can 
>> do on jpackage side to sign such invalid bundles. As proposed solution we 
>> will output possible reason for "codesign" failure if it fails and 
>> `--app-content` was specified and possible solution. Proposed message: "One 
>> of the possible reason for "codesign" failure is additional content provided 
>> via "--app-content", which made application bundle structure invalid. Make 
>> sure to provide additional content in a way it will not break application 
>> bundle structure, otherwise add additional content as post-processing step."
>> 
>> Example:
>> Lets assume we have "ReadMe" folder with "ReadMe.txt" file in it.
>> 1) jpackage --type app-image -n Test --app-content ReadMe/ReadMe.txt ...
>> "codesign" will fail with "In subcomponent: Test.app/Contents/ReadMe.txt". 
>> This is expected and "ReadMe.txt" placed in "Test.app/Contents" which is 
>> also expected.
>> 2) jpackage --type app-image -n Test --app-content ReadMe ...
>> Works and "ReadMe.txt" will be placed under "Test.app/Contents/ReadMe".
>> 
>> Sample output before fix:
>> 
>> Error: "codesign" failed with following output:
>> Test.app: replacing existing signature
>> Test.app: code object is not signed at all
>> In subcomponent: Test.app/Contents/ReadMe.txt
>> 
>> 
>> Sample output after fix:
>> 
>> "codesign" failed and additional application content was supplied via the 
>> "--app-content" parameter. Probably the additional content broke the 
>> integrity of the application bundle and caused the failure. Ensure content 
>> supplied via the "--app-content" parameter does not break the integrity of 
>> the application bundle, or add it in the post-processing step.
>> Error: "codesign" failed with following output:
>> Test.app: replacing existing signature
>> Test.app: code object is not signed at all
>> In subcomponent: Test.app/Contents/ReadMe.txt
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8332110: jpackage tries to sign added files without the --mac-sign option 
> [v2]

8332110: jpackage tries to sign added files without the --mac-sign option [v2]
 - Updated error message as suggested.

-

PR Comment: https://git.openjdk.org/jdk/pull/19377#issuecomment-2140973262

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option [v2]

> This issue is reproducible with and without `--mac-sign`. jpackage will 
> "_ad-hoc_" sign application bundle when `--mac-sign` is not specified by 
> using pseudo-identity "_-_". This is why jpackage tries to sign added files 
> and this is expected behavior by jpackage. "codesign" fails since added 
> content made application bundle structure invalid. There is nothing we can do 
> on jpackage side to sign such invalid bundles. As proposed solution we will 
> output possible reason for "codesign" failure if it fails and `--app-content` 
> was specified and possible solution. Proposed message: "One of the possible 
> reason for "codesign" failure is additional content provided via 
> "--app-content", which made application bundle structure invalid. Make sure 
> to provide additional content in a way it will not break application bundle 
> structure, otherwise add additional content as post-processing step."
> 
> Example:
> Lets assume we have "ReadMe" folder with "ReadMe.txt" file in it.
> 1) jpackage --type app-image -n Test --app-content ReadMe/ReadMe.txt ...
> "codesign" will fail with "In subcomponent: Test.app/Contents/ReadMe.txt". 
> This is expected and "ReadMe.txt" placed in "Test.app/Contents" which is also 
> expected.
> 2) jpackage --type app-image -n Test --app-content ReadMe ...
> Works and "ReadMe.txt" will be placed under "Test.app/Contents/ReadMe".
> 
> Sample output before fix:
> 
> Error: "codesign" failed with following output:
> Test.app: replacing existing signature
> Test.app: code object is not signed at all
> In subcomponent: Test.app/Contents/ReadMe.txt
> 
> 
> Sample output after fix:
> 
> "codesign" failed and additional application content was supplied via the 
> "--app-content" parameter. Probably the additional content broke the 
> integrity of the application bundle and caused the failure. Ensure content 
> supplied via the "--app-content" parameter does not break the integrity of 
> the application bundle, or add it in the post-processing step.
> Error: "codesign" failed with following output:
> Test.app: replacing existing signature
> Test.app: code object is not signed at all
> In subcomponent: Test.app/Contents/ReadMe.txt

Alexander Matveev has updated the pull request incrementally with one 
additional commit since the last revision:

  8332110: jpackage tries to sign added files without the --mac-sign option [v2]

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19377/files
  - new: https://git.openjdk.org/jdk/pull/19377/files/0ad02cbb..7c1973ad

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19377=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=19377=00-01

  Stats: 9 lines in 6 files changed: 1 ins; 0 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/19377.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19377/head:pull/19377

PR: https://git.openjdk.org/jdk/pull/19377

Re: RFR: 8333307: Don't suppress jpackage logging in tests when it is detecting packaging tools in the system

On Thu, 30 May 2024 20:26:02 GMT, Alexey Semenyuk  wrote:

> Change jpackage tests output from:
> 
> 
> [19:16:29.586] Create: SimplePackageTest.test
> [19:16:29.587] [ RUN ] SimplePackageTest.test
> [19:16:29.663] TRACE: Bundler rpm supported
> [19:16:29.674] TRACE: Actions: [[initialize], [create], [unpack], 
> [verify-install], [finalize]]
> 
> 
> to:
> 
> 
> [19:16:29.586] Create: SimplePackageTest.test
> [19:16:29.587] [ RUN ] SimplePackageTest.test
> [19:16:29.621] Running dpkg
> [19:16:29.633] Running dpkg-deb
> [19:16:29.651] Running rpmbuild
> [19:16:29.663] TRACE: Bundler rpm supported
> [19:16:29.674] TRACE: Actions: [[initialize], [create], [unpack], 
> [verify-install], [finalize]]

Looks good, but I am confused why it is targeted to 24? I think you need to 
postpone integration until 24 or target it to 23.

-

PR Comment: https://git.openjdk.org/jdk/pull/19489#issuecomment-2140961328

Re: RFR: 8333116: test/jdk/tools/jpackage/share/ServiceTest.java test fails

2024-05-28 Thread Alexander Matveev

On Tue, 28 May 2024 20:13:24 GMT, Alexey Semenyuk  wrote:

> Add missing "--add-opens jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED" to 
> the test descriptor

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19436#pullrequestreview-2083892285

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option

2024-05-28 Thread Alexander Matveev

On Fri, 24 May 2024 01:08:03 GMT, Alexander Matveev  
wrote:

> This issue is reproducible with and without `--mac-sign`. jpackage will 
> "_ad-hoc_" sign application bundle when `--mac-sign` is not specified by 
> using pseudo-identity "_-_". This is why jpackage tries to sign added files 
> and this is expected behavior by jpackage. "codesign" fails since added 
> content made application bundle structure invalid. There is nothing we can do 
> on jpackage side to sign such invalid bundles. As proposed solution we will 
> output possible reason for "codesign" failure if it fails and `--app-content` 
> was specified and possible solution. Proposed message: "One of the possible 
> reason for "codesign" failure is additional content provided via 
> "--app-content", which made application bundle structure invalid. Make sure 
> to provide additional content in a way it will not break application bundle 
> structure, otherwise add additional content as post-processing step."
> 
> Example:
> Lets assume we have "ReadMe" folder with "ReadMe.txt" file in it.
> 1) jpackage --type app-image -n Test --app-content ReadMe/ReadMe.txt ...
> "codesign" will fail with "In subcomponent: Test.app/Contents/ReadMe.txt". 
> This is expected and "ReadMe.txt" placed in "Test.app/Contents" which is also 
> expected.
> 2) jpackage --type app-image -n Test --app-content ReadMe ...
> Works and "ReadMe.txt" will be placed under "Test.app/Contents/ReadMe".
> 
> Sample output before fix:
> 
> Error: "codesign" failed with following output:
> Test.app: replacing existing signature
> Test.app: code object is not signed at all
> In subcomponent: Test.app/Contents/ReadMe.txt
> 
> 
> Sample output after fix:
> 
> One of the possible reason for "codesign" failure is additional content 
> provided via "--app-content", which made application bundle structure 
> invalid. Make sure to provide additional content in a way it will not break 
> application bundle structure, otherwise add additional content as 
> post-processing step.
> Error: "codesign" failed with following output:
> Test.app: replacing existing signature
> Test.app: code object is not signed at all
> In subcomponent: Test.app/Contents/ReadMe.txt

@alexeysemenyukoracle please review

-

PR Comment: https://git.openjdk.org/jdk/pull/19377#issuecomment-2136148629

Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option

2024-05-24 Thread Alexander Matveev

Hi Michael,

> Doesn’t this still leave you with an application that isn’t validly signed? 
> And probably won’t run because of that.
Yes, it will leave you with an application that isn’t signed. I was able to run 
such application on same machine as it was generated by jpackage.

> For your example. This almost seems like an Apple bug if you can add a 
> directory to the Contents directory but not a file?
Not sure if it is an Apple bug.

> Would it also generally be a good idea to include a final codesign verify to 
> fail the build if something is wrong with the signature?
Yes, you already suggested it. See https://bugs.openjdk.org/browse/JDK-8318063 
and it was closed as won’t fix because such verification is redundant.

Thanks,
Alexander

From: Michael Hall 
Date: Friday, May 24, 2024 at 1:47 AM
To: Alexander Matveev 
Cc: core-libs-dev 
Subject: Re: RFR: 8332110: [macos] jpackage tries to sign added files without 
the --mac-sign option

On May 24, 2024, at 3:08 AM, Michael Hall  wrote:

On May 23, 2024, at 8:13 PM, Alexander Matveev 
mailto:almat...@openjdk.org>> wrote:

otherwise add additional content as post-processing step.

Doesn’t this still leave you with an application that isn’t validly signed? And 
probably won’t run because of that.

2) jpackage --type app-image -n Test --app-content ReadMe ...

For your example. This almost seems like an Apple bug if you can add a 
directory to the Contents directory but not a file?

Sorry I made my prior off-list.

Would it also generally be a good idea to include a final codesign verify to 
fail the build if something is wrong with the signature?

Something like…

echo '***'
echo 'verifying signature'
echo '***'
codesign -v --verbose=4 outputdir/HalfPipe.app

Expected output…

***
verifying signature
***
outputdir/HalfPipe.app: valid on disk
outputdir/HalfPipe.app: satisfies its Designated Requirement

I think I have suggested this before but don’t remember if I did an enhancement 
request. Maybe you do that and I’m just not aware of it if it doesn’t appear in 
the jpackage output.

RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option

2024-05-23 Thread Alexander Matveev

This issue is reproducible with and without `--mac-sign`. jpackage will 
"_ad-hoc_" sign application bundle when `--mac-sign` is not specified by using 
pseudo-identity "_-_". This is why jpackage tries to sign added files and this 
is expected behavior by jpackage. "codesign" fails since added content made 
application bundle structure invalid. There is nothing we can do on jpackage 
side to sign such invalid bundles. As proposed solution we will output possible 
reason for "codesign" failure if it fails and `--app-content` was specified and 
possible solution. Proposed message: "One of the possible reason for "codesign" 
failure is additional content provided via "--app-content", which made 
application bundle structure invalid. Make sure to provide additional content 
in a way it will not break application bundle structure, otherwise add 
additional content as post-processing step."

Example:
Lets assume we have "ReadMe" folder with "ReadMe.txt" file in it.
1) jpackage --type app-image -n Test --app-content ReadMe/ReadMe.txt ...
"codesign" will fail with "In subcomponent: Test.app/Contents/ReadMe.txt". This 
is expected and "ReadMe.txt" placed in "Test.app/Contents" which is also 
expected.
2) jpackage --type app-image -n Test --app-content ReadMe ...
Works and "ReadMe.txt" will be placed under "Test.app/Contents/ReadMe".

Sample output before fix:

Error: "codesign" failed with following output:
Test.app: replacing existing signature
Test.app: code object is not signed at all
In subcomponent: Test.app/Contents/ReadMe.txt


Sample output after fix:

One of the possible reason for "codesign" failure is additional content 
provided via "--app-content", which made application bundle structure invalid. 
Make sure to provide additional content in a way it will not break application 
bundle structure, otherwise add additional content as post-processing step.
Error: "codesign" failed with following output:
Test.app: replacing existing signature
Test.app: code object is not signed at all
In subcomponent: Test.app/Contents/ReadMe.txt

-

Commit messages:
 - 8332110: jpackage tries to sign added files without the --mac-sign option

Changes: https://git.openjdk.org/jdk/pull/19377/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19377=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332110
  Stats: 38 lines in 6 files changed: 24 ins; 0 del; 14 mod
  Patch: https://git.openjdk.org/jdk/pull/19377.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19377/head:pull/19377

PR: https://git.openjdk.org/jdk/pull/19377

Re: RFR: 8331222: Malformed text in the jpackage doc page

2024-04-30 Thread Alexander Matveev

On Tue, 30 Apr 2024 23:58:49 GMT, Alexey Semenyuk  wrote:

> Rerun pandoc on updated source man page file

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19028#pullrequestreview-2032839569

Re: RFR: 8295111: dpkg appears to have problems resolving symbolically linked native libraries

2024-04-18 Thread Alexander Matveev

On Thu, 18 Apr 2024 19:56:43 GMT, Alexey Semenyuk  wrote:

> Pass a path with resolved symlinks to `dpkg -S` command. If it fails, try the 
> original path if they differ.
> 
> Testing on Ubuntu 24.04 passed. Successfully created a .deb package from 
> SwingSet2.jar with the not-empty list of dependencies. Before the fix the 
> list of required packages was empty. In jpackage log:
> 
> 
> [19:30:48.784] Running dpkg
> [19:30:49.118] Command [PID: 244104]:
> dpkg -S /usr/lib/x86_64-linux-gnu/libm.so.6
> [19:30:49.118] Output:
> libc6:amd64: /usr/lib/x86_64-linux-gnu/libm.so.6
> [19:30:49.118] Returned: 0
> 
> [19:30:49.118] /lib/x86_64-linux-gnu/libm.so.6 is provided by [libc6]
> 
> 
> It was looking for a package providing "/lib/x86_64-linux-gnu/libm.so.6" 
> library, but the actual argument to "dpgk -S" command was 
> "/usr/lib/x86_64-linux-gnu/libm.so.6".

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18844#pullrequestreview-2009895564

Re: [External] : Re: jpackage requests permission via a dialog

2024-03-05 Thread Alexander Matveev

Hi Alan,

jpackage does not use .DS_Store file directly. Script tells “Finder” 
application to arrange DMG layout and once script is finished “Finder” will 
create .DS_Store file to store layout.
If user wants to modify it, them generated DMG needs to be converted to 
writable, mounted, layout modified by hand or any other ways like script, 
unmount and make DMG read only.

DMG might have additional content specified by jpackage users, so I do not know 
how we could re-use .DS_Store in such cases, since we do not know what content 
DMG might have.

Thanks,
Alexander

From: Alan Snyder 
Date: Tuesday, March 5, 2024 at 8:50 AM
To: Michael Hall 
Cc: Alexander Matveev , core-libs-dev@openjdk.org 

Subject: Re: [External] : Re: jpackage requests permission via a dialog
I’m wondering whether some developers might prefer to arrange the DMG layout by 
hand.

That would require jpackage to use a .DS_Store file provided by the developer 
and not running the script.

The issue is whether a .DS_Store file created using a previous DMG would work 
in a new DMG.

The documentation for install4j suggests that there is a way.

[https://www.ej-technologies.com/resources/install4j/help/doc/concepts/dmgStyling.html<https://urldefense.com/v3/__https:/www.ej-technologies.com/resources/install4j/help/doc/concepts/dmgStyling.html__;!!ACWV5N9M2RV99hQ!JI59aSFm4DwVoosoosGu5U5p_6GD8_bhXqh9tplhX6GJ_6Z7GiQ9IV3JuHHOn_an62KkkoC22UPLytOMOANY58u4rV4$>]




On Mar 5, 2024, at 12:28 AM, Michael Hall  wrote:

Hello Alexander,

I had forgotten or didn’t know that. I didn’t see it mentioned anywhere in this 
list thread previously. But there is generally no reason to delete this file. 
Unless you don’t want it included in a zip or a GitHub project. The icon 
placement for multiple files with jpackage —mac-dmg-content is determined by 
the AppleScript. You might remember I had a related bug report. I provided a 
workaround with slight changes to the code invoking the AppleScript and the 
AppleScript. You came up with your own fix. I was impressed you did it by just 
changing the AppleScript but I slightly preferred the placement of my change. 
Again, if I remember right.

Mike


On Mar 4, 2024, at 9:57 PM, Alexander Matveev  
wrote:

Hi Michael,

.DS_Store file contains information about icon position and other visual 
information on how to display folder. Once osascript configures DMG, Finder 
will store these setting in .DS_Store file. If it gets deleted from DMG, DMG 
will look like osascript was never run. So, Alan is right with “the 
configuration of the DMG appearance actually means configuring the .DS_Store 
file in the DMG”.

Re: [External] : Re: jpackage requests permission via a dialog

2024-03-04 Thread Alexander Matveev

Hi Michael,

.DS_Store file contains information about icon position and other visual 
information on how to display folder. Once osascript configures DMG, Finder 
will store these setting in .DS_Store file. If it gets deleted from DMG, DMG 
will look like osascript was never run. So, Alan is right with “the 
configuration of the DMG appearance actually means configuring the .DS_Store 
file in the DMG”.

Thanks,
Alexander

From: Michael Hall 
Date: Monday, March 4, 2024 at 7:44 PM
To: Alan Snyder 
Cc: Alexander Matveev , core-libs-dev@openjdk.org 

Subject: [External] : Re: jpackage requests permission via a dialog
I’m not following what the .DS_Store file has to do with anything, other than 
being a Mac feature that’s occasionally a nuisance when the files end up in 
zip’s or elsewhere where you don’t want them.

I’m also not sure if that was my explanation or Alexander’s you’re talking 
about. Using the osascript to run the dmg gives the dmg a look more like other 
distributed macOS applications.

If that doesn’t matter to you then you could use jpackage to create a 
standalone app image and hdiutil a dmg for it. Opening it will show the files, 
period. Or use Disk Utility to create a dmg would also work. I’ve done that 
sometimes. Then you don’t have to remember the hdiutil command line usage.

> On Mar 4, 2024, at 5:50 PM, Alan Snyder  wrote:
>
> Thank you for the explanation.
>
> It was not obvious to me that the configuration of the DMG appearance 
> actually means configuring the .DS_Store file in the DMG.
>
>   Alan
>

Re: jpackage requests permission via a dialog

2024-03-04 Thread Alexander Matveev

Hi Alan,

See below.

From: core-libs-dev  on behalf of Alan Snyder 

Date: Sunday, March 3, 2024 at 7:09 PM
To: core-libs-dev@openjdk.org 
Subject: jpackage requests permission via a dialog
I tried using jpackage on macOS to create a DMG (which I have not done before) 
and was surprised when a system dialog was displayed requesting permission for 
Terminal to control Finder.

I found this issue described in JDK-8231855, which was closed without 
explanation as “not an issue”.
From JDK-8231855: “Looks like this dialog is part of macOS Catalina Data 
Protection features. I did not found if it possible not to trigger this prompt. 
Also, this prompt is shown only once and subsequent jpackage runs will not show 
any prompts. As workaround in case when jpackage will be part of automated 
build system which needs to be run without user intervention it is recommended 
to run jpackage first on this system and confirm any prompts.”.

It seems to me that having a command line build tool try to interact with a 
user via a dialog is inappropriate, even if it happens only once per system.
So far, I did not figure out other solution for this problem.

I’m not sure why this dialog is shown, as it appears that all of the actions 
are performed by running the hdiutil program.
This dialog is shown when we running osascript 
(https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/DMGsetup.scpt)
 to post process DMG file. This script will setup DMG background and re-arrange 
icons, so it look nice.

It does seem odd, however, that jpackage mounts the DMG to modify its contents. 
Shouldn’t the contents be set up before creating the DMG?
We need to mount it to run osascript I mentioned above.

Is mounting the image the source of the dialog?
No, dialog is shown after we mount it from osascript which is run right after 
DMG is mounted.

Is there some reason this cannot be fixed?
So far I did not found any substitution to our osascript to setup background 
image and re-arrange icons. I do not think that Apple provides any other ways 
to do it.
Thanks,
Alexander

Re: RFR: 8325203: System.exit(0) kills the launched 3rd party application

2024-02-08 Thread Alexander Matveev

On Thu, 8 Feb 2024 20:57:51 GMT, Alexey Semenyuk  wrote:

> Tested with the use case from the CR.
> 
> The idea of the fix is to prevent grandchildren processes from being 
> automatically attached to the job killing all processes attached to it when 
> the job object is destroyed.
> 
> Filed a follow-up https://bugs.openjdk.org/browse/JDK-8325525 CR to track 
> adding a jtreg test for the issue.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17779#pullrequestreview-1871439784

Re: [jdk22] RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)

2024-01-17 Thread Alexander Matveev

On Wed, 17 Jan 2024 18:33:27 GMT, Alexey Semenyuk  wrote:

> Clean backport

Looks good. I verified that it is a clean backport.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk22/pull/88#pullrequestreview-1828180526

Re: RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)

2024-01-11 Thread Alexander Matveev

On Thu, 11 Jan 2024 18:19:15 GMT, Alexey Semenyuk  wrote:

> Change the full path of a unit file passed to `systemctl enable` command to 
> the unit file name. This prevents the command from creating a symlink in 
> `/etc/systemd/system/` directory

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/17376#pullrequestreview-1816945852

Re: RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)

2024-01-11 Thread Alexander Matveev

On Thu, 11 Jan 2024 18:19:15 GMT, Alexey Semenyuk  wrote:

> Change the full path of a unit file passed to `systemctl enable` command to 
> the unit file name. This prevents the command from creating a symlink in 
> `/etc/systemd/system/` directory

Fix looks good. I will approve once "Integration blocker" issue is resolved.

-

PR Comment: https://git.openjdk.org/jdk/pull/17376#issuecomment-1887841042

Re: RFR: 8322041: JDK 22 RDP1 L10n resource files update

2023-12-13 Thread Alexander Matveev

On Wed, 13 Dec 2023 22:12:48 GMT, Alisen Chung  wrote:

> Translation drop for JDK22 RDP1

jpackage part looks fine.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17096#pullrequestreview-1780705685

Re: RFR: 8294699: Launcher causes lingering busy cursor

2023-12-04 Thread Alexander Matveev

On Mon, 4 Dec 2023 20:08:15 GMT, Alexey Semenyuk  wrote:

> 8294699: Launcher causes lingering busy cursor

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16958#pullrequestreview-1763517307

Re: RFR: 8227529: With malformed --app-image the error messages are awful

2023-11-30 Thread Alexander Matveev

On Thu, 30 Nov 2023 14:14:56 GMT, Alexey Semenyuk  wrote:

> Add tests to make sure jpackage emits meaningful error messages when used 
> with invalid app images.
> 
> Added `JPackageCommand.nullableOutputBundle()` function that returns an empty 
> `Optional` instance if it fails to deduce a path to the output bundle from 
> jpackage command line. This is the case when jpackage command is used with 
> `--app-image` cli option and referenced app image directory is missing 
> `.jpackage.xml` file.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16901#pullrequestreview-1758569694

Integrated: 8320681: [macos] Test tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java timed out on macOS

2023-11-28 Thread Alexander Matveev

On Tue, 28 Nov 2023 02:24:20 GMT, Alexander Matveev  
wrote:

> Looks like the test timed out due to slow/loaded host. Failure log similar to 
> [JDK-8296489](https://bugs.openjdk.org/browse/JDK-8296489) and 
> [JDK-8298735](https://bugs.openjdk.org/browse/JDK-8298735). Fixed by 
> increasing test timeout.

This pull request has now been integrated.

Changeset: a657aa38
Author:    Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/a657aa38a56056211a9d2773b30e8fe1a89c128e
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod

8320681: [macos] Test tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java 
timed out on macOS

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/16838

Re: RFR: 8320858: Move jpackage tests to tier3

2023-11-28 Thread Alexander Matveev

On Tue, 28 Nov 2023 08:51:17 GMT, Alan Bateman  wrote:

> Update the jtreg test group configuration in the jdk test tree so that the 
> jpackage tests run in tier3 rather than tier2.
> 
> At this time, the jpackage tests run in jdk:tier2, more specifically 
> jdk:tier2_part2 as part of the core_tools test group. The  jpackage tests 
> take a significant amount of time on some platforms (on macOS and Windows in 
> particular). The changes here remove tools/jpackage from core_tools (it's not 
> a core tool anyway) and adds the tests to tier3. In the future then maybe 
> tier3 can be split up if needed.

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16841#pullrequestreview-1753956164

RFR: 8320681: [macos] Test tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java timed out on macOS

Looks like the test timed out due to slow/loaded host. Failure log similar to 
[JDK-8296489](https://bugs.openjdk.org/browse/JDK-8296489) and 
[JDK-8298735](https://bugs.openjdk.org/browse/JDK-8298735). Fixed by increasing 
test timeout.

-

Commit messages:
 - 8320681: [macos] Test tools/jpackage/macosx/MacAppStoreJlinkOptionsTest.java 
timed out on macOS

Changes: https://git.openjdk.org/jdk/pull/16838/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=16838=00
  Issue: https://bugs.openjdk.org/browse/JDK-8320681
  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/16838.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16838/head:pull/16838

PR: https://git.openjdk.org/jdk/pull/16838

Re: RFR: 8301856: Generated .spec file for RPM installers uninstalls desktop launcher on update

On Thu, 23 Nov 2023 16:36:28 GMT, Alexey Semenyuk  wrote:

> Fix uninstallation scripts in Linux installers to take action on desktop 
> files only if these desktop files are not owned by another package. This 
> prevents `xdg-mime uninstall`, `xdg-desktop-menu uninstall` commands from 
> execution in case of upgrades.
> 
> Borrowed helper shell script function from "launchers as services" for the 
> fix. Now on Linux there is shared code between "launchers as services" and 
> "desktop integration" subsystems - "common_utils.sh". Added 
> `ServiceAndDesktopTest.java` to test how they align.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16800#pullrequestreview-1751505209

Re: RFR: 8320706: RuntimePackageTest.testUsrInstallDir test fails on Linux

On Fri, 24 Nov 2023 16:33:13 GMT, Alexey Semenyuk  wrote:

> 8320706: RuntimePackageTest.testUsrInstallDir test fails on Linux

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16810#pullrequestreview-1751469146

Re: RFR: 8320249: tools/jpackage/share/AddLauncherTest.java#id1 fails intermittently on Windows in verifyDescription [v2]

On Fri, 24 Nov 2023 22:31:18 GMT, Alexey Semenyuk  wrote:

>> 8320249: tools/jpackage/share/AddLauncherTest.java#id1 fails intermittently 
>> on Windows in verifyDescription
>
> Alexey Semenyuk has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   fix utf8 representation

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16814#pullrequestreview-1751393743

Re: RFR: 8319338: tools/jpackage/share/RuntimeImageTest.java fails with -XX:+UseZGC

2023-11-06 Thread Alexander Matveev

On Tue, 7 Nov 2023 03:20:45 GMT, Alexey Semenyuk  wrote:

> Remove `-Xmx512m` from the jtreg `@run` command as @AlanBateman suggested

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16535#pullrequestreview-1716751895

Integrated: 8310933: Copying from runtime image to application image should not follow symlinks

2023-10-30 Thread Alexander Matveev

On Mon, 30 Oct 2023 19:43:32 GMT, Alexander Matveev  
wrote:

> - Added LinkOption.NOFOLLOW_LINKS when copying pre-define runtime image.
> - Added test to cover this change.
> - Cleanup RuntimeImageTest.java by removing unused imports and removed 
> incubator from jpackage module name.

This pull request has now been integrated.

Changeset: 1ca2cfaf
Author:    Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/1ca2cfafdd906851b923be69852ccf9a3bb4db35
Stats: 128 lines in 4 files changed: 114 ins; 9 del; 5 mod

8310933: Copying from runtime image to application image should not follow 
symlinks

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/16426

RFR: 8310933: Copying from runtime image to application image should not follow symlinks

2023-10-30 Thread Alexander Matveev

- Added LinkOption.NOFOLLOW_LINKS when copying pre-define runtime image.
- Added test to cover this change.
- Cleanup RuntimeImageTest.java by removing unused imports and removed 
incubator from jpackage module name.

-

Commit messages:
 - 8310933: Copying from runtime image to application image should not follow 
symlinks

Changes: https://git.openjdk.org/jdk/pull/16426/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=16426=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310933
  Stats: 128 lines in 4 files changed: 114 ins; 9 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/16426.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16426/head:pull/16426

PR: https://git.openjdk.org/jdk/pull/16426

Integrated: 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild

2023-10-24 Thread Alexander Matveev

On Fri, 6 Oct 2023 22:15:00 GMT, Alexander Matveev  wrote:

> - Added `--mac-app-image-sign-identity` and `--mac-installer-sign-identity` 
> CLI options to jpackage to provide signing identity directly to `codesign` 
> and `productbuild` tools as per CSR 
> [JDK-8316631](https://bugs.openjdk.org/browse/JDK-8316631).
> - If `codesign` or `productbuild` fails, then output of these tools will be 
> printed to stdout to help user diagnose issues with signing using new 
> options. Examples with sign identity set to "test" which does not exist on 
> system:
>> Error: "codesign" failed with following output:
>> test: no identity found
> 
>> Error: "productbuild" failed with following output:
>> productbuild: error: Cannot write product to "/Users/SOMEDIR/Test-1.0.pkg". 
>> (Could not find appropriate signing identity for “test”.)
> - Added error handling not to allow invalid combinations of signing options.
> - Updated signing tests to test new changes.

This pull request has now been integrated.

Changeset: f1dfdc1a
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/f1dfdc1a79f3a16eae58d15d1945541a08f7e145
Stats: 555 lines in 24 files changed: 450 ins; 24 del; 81 mod

8311877: [macos] Add CLI options to provide signing identity directly to 
codesign and productbuild

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/16085

Re: RFR: 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild [v3]

2023-10-20 Thread Alexander Matveev

On Fri, 20 Oct 2023 04:23:22 GMT, Alexander Matveev  
wrote:

>> - Added `--mac-app-image-sign-identity` and `--mac-installer-sign-identity` 
>> CLI options to jpackage to provide signing identity directly to `codesign` 
>> and `productbuild` tools as per CSR 
>> [JDK-8316631](https://bugs.openjdk.org/browse/JDK-8316631).
>> - If `codesign` or `productbuild` fails, then output of these tools will be 
>> printed to stdout to help user diagnose issues with signing using new 
>> options. Examples with sign identity set to "test" which does not exist on 
>> system:
>>> Error: "codesign" failed with following output:
>>> test: no identity found
>> 
>>> Error: "productbuild" failed with following output:
>>> productbuild: error: Cannot write product to "/Users/SOMEDIR/Test-1.0.pkg". 
>>> (Could not find appropriate signing identity for “test”.)
>> - Added error handling not to allow invalid combinations of signing options.
>> - Updated signing tests to test new changes.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8311877: [macos] Add CLI options to provide signing identity directly to 
> codesign and productbuild [v2]

Michael filed https://bugs.openjdk.org/browse/JDK-8318063 for codesign 
verification, so I would prefer to move discussion under this JBS issue on this 
topic.

For now I do not know if we need to add it. From my experience I never found 
case when codesign did not fail during signing, but signature verification 
failed. So, for now I think it is redundant and we will never get to signature 
verification step if something wrong since codesign will fail during signing. 
Only case I see is when we did not run codesign at all, but it means our entire 
signing is broken.

Michael, do you have any examples when you was able to sign application bundle 
via jpackage or manually, but codesign verification gave error?

-

PR Comment: https://git.openjdk.org/jdk/pull/16085#issuecomment-1773366403

Re: RFR: 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild [v3]

2023-10-19 Thread Alexander Matveev

On Fri, 20 Oct 2023 04:23:22 GMT, Alexander Matveev  
wrote:

>> - Added `--mac-app-image-sign-identity` and `--mac-installer-sign-identity` 
>> CLI options to jpackage to provide signing identity directly to `codesign` 
>> and `productbuild` tools as per CSR 
>> [JDK-8316631](https://bugs.openjdk.org/browse/JDK-8316631).
>> - If `codesign` or `productbuild` fails, then output of these tools will be 
>> printed to stdout to help user diagnose issues with signing using new 
>> options. Examples with sign identity set to "test" which does not exist on 
>> system:
>>> Error: "codesign" failed with following output:
>>> test: no identity found
>> 
>>> Error: "productbuild" failed with following output:
>>> productbuild: error: Cannot write product to "/Users/SOMEDIR/Test-1.0.pkg". 
>>> (Could not find appropriate signing identity for “test”.)
>> - Added error handling not to allow invalid combinations of signing options.
>> - Updated signing tests to test new changes.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8311877: [macos] Add CLI options to provide signing identity directly to 
> codesign and productbuild [v2]

Yes, you got it right. I agree it does not make sense to consider error if 
`--mac-sign` is not specified. I fixed it as you suggested. 
`--mac-app-image-sign-identity` will be ignored and no signing is done if 
`--mac-sign` is not specified.

Also, updated `SigningPackageTest.java` to include tests for cases when only 
app-image is being signed or when only pkg is being signed using sign identity 
options.

-

PR Comment: https://git.openjdk.org/jdk/pull/16085#issuecomment-1772057635

Re: RFR: 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild [v3]

2023-10-19 Thread Alexander Matveev

> - Added `--mac-app-image-sign-identity` and `--mac-installer-sign-identity` 
> CLI options to jpackage to provide signing identity directly to `codesign` 
> and `productbuild` tools as per CSR 
> [JDK-8316631](https://bugs.openjdk.org/browse/JDK-8316631).
> - If `codesign` or `productbuild` fails, then output of these tools will be 
> printed to stdout to help user diagnose issues with signing using new 
> options. Examples with sign identity set to "test" which does not exist on 
> system:
>> Error: "codesign" failed with following output:
>> test: no identity found
> 
>> Error: "productbuild" failed with following output:
>> productbuild: error: Cannot write product to "/Users/SOMEDIR/Test-1.0.pkg". 
>> (Could not find appropriate signing identity for “test”.)
> - Added error handling not to allow invalid combinations of signing options.
> - Updated signing tests to test new changes.

Alexander Matveev has updated the pull request incrementally with one 
additional commit since the last revision:

  8311877: [macos] Add CLI options to provide signing identity directly to 
codesign and productbuild [v2]

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16085/files
  - new: https://git.openjdk.org/jdk/pull/16085/files/4cd56e25..c3112444

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16085=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=16085=01-02

  Stats: 159 lines in 9 files changed: 59 ins; 73 del; 27 mod
  Patch: https://git.openjdk.org/jdk/pull/16085.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16085/head:pull/16085

PR: https://git.openjdk.org/jdk/pull/16085

Re: RFR: 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild [v2]

2023-10-18 Thread Alexander Matveev

> - Added `--mac-app-image-sign-identity` and `--mac-installer-sign-identity` 
> CLI options to jpackage to provide signing identity directly to `codesign` 
> and `productbuild` tools as per CSR 
> [JDK-8316631](https://bugs.openjdk.org/browse/JDK-8316631).
> - If `codesign` or `productbuild` fails, then output of these tools will be 
> printed to stdout to help user diagnose issues with signing using new 
> options. Examples with sign identity set to "test" which does not exist on 
> system:
>> Error: "codesign" failed with following output:
>> test: no identity found
> 
>> Error: "productbuild" failed with following output:
>> productbuild: error: Cannot write product to "/Users/SOMEDIR/Test-1.0.pkg". 
>> (Could not find appropriate signing identity for “test”.)
> - Added error handling not to allow invalid combinations of signing options.
> - Updated signing tests to test new changes.

Alexander Matveev 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 two additional 
commits since the last revision:

 - Merge remote-tracking branch 'upstream/master' into JDK-8311877
 - 8311877: [macos] Add CLI options to provide signing identity directly to 
codesign and productbuild

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16085/files
  - new: https://git.openjdk.org/jdk/pull/16085/files/57abfbad..4cd56e25

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16085=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=16085=00-01

  Stats: 42104 lines in 1446 files changed: 26275 ins; 7631 del; 8198 mod
  Patch: https://git.openjdk.org/jdk/pull/16085.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16085/head:pull/16085

PR: https://git.openjdk.org/jdk/pull/16085

Re: RFR: 8316563: test tools/jpackage/linux/LinuxResourceTest.java fails on CentOS Linux release 8.5.2111 and Fedora 27 [v2]

2023-10-17 Thread Alexander Matveev

On Tue, 17 Oct 2023 07:04:03 GMT, yaqsun  wrote:

>> on some RHEL Linux 8.X and Fedora 27 machines , we run into errors in test 
>> tools/jpackage/linux/LinuxResourceTest.java.
>> It's the same problem as https://bugs.openjdk.org/browse/JDK-8314121. 
>> But after 8314121, tools/jpackage/linux/LinuxResourceTest.java still 
>> reappears. tools/jpackage/linux/LinuxResourceTest.java does not use file 
>> src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/template.spec.
>
> yaqsun has updated the pull request incrementally with two additional commits 
> since the last revision:
> 
>  - fixed Whitespace error: del tab
>  - 8316563: test tools/jpackage/linux/LinuxResourceTest.java fails on RHEL8 
> and Fedora 27

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15832#pullrequestreview-1683474813

RFR: 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild

2023-10-06 Thread Alexander Matveev

- Added `--mac-app-image-sign-identity` and `--mac-installer-sign-identity` CLI 
options to jpackage to provide signing identity directly to `codesign` and 
`productbuild` tools as per CSR 
[JDK-8316631](https://bugs.openjdk.org/browse/JDK-8316631).
- If `codesign` or `productbuild` fails, then output of these tools will be 
printed to stdout to help user diagnose issues with signing using new options. 
Examples with sign identity set to "test" which does not exist on system:
> Error: "codesign" failed with following output:
> test: no identity found

> Error: "productbuild" failed with following output:
> productbuild: error: Cannot write product to "/Users/SOMEDIR/Test-1.0.pkg". 
> (Could not find appropriate signing identity for “test”.)
- Added error handling not to allow invalid combinations of signing options.
- Updated signing tests to test new changes.

-

Commit messages:
 - 8311877: [macos] Add CLI options to provide signing identity directly to 
codesign and productbuild

Changes: https://git.openjdk.org/jdk/pull/16085/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=16085=00
  Issue: https://bugs.openjdk.org/browse/JDK-8311877
  Stats: 550 lines in 24 files changed: 463 ins; 23 del; 64 mod
  Patch: https://git.openjdk.org/jdk/pull/16085.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16085/head:pull/16085

PR: https://git.openjdk.org/jdk/pull/16085

Re: RFR: 8317283: jpackage tests run osx-specific checks on windows and linux

2023-09-28 Thread Alexander Matveev

On Thu, 28 Sep 2023 23:38:51 GMT, Alexey Semenyuk  wrote:

> - Don't run osx specific checks on Linux and Windows
>  - Rework checks output from 
> 
> [17:31:52.845] TRACE: assertTrue(): Unexptected value in app image file for 
> 
> [17:31:52.860] TRACE: assertTrue(): Unexptected value in app image file for 
> 
> 
> to
> 
> [22:07:46.519] TRACE: assertEquals(false): Check for unexptected value in app 
> image file for 
> [22:07:46.519] TRACE: assertEquals(false): Check for unexptected value in app 
> image file for 

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15975#pullrequestreview-1649891012

Re: RFR: 8303959: tools/jpackage/share/RuntimePackageTest.java fails with java.lang.AssertionError missing files

2023-09-28 Thread Alexander Matveev

On Thu, 28 Sep 2023 20:58:30 GMT, Alexey Semenyuk  wrote:

> Don't use JDK image from `$JAVA_HOME` as a value of `--runtime-image` 
> jpackage cli option. Use jlink to create a runtime in test work dir if the 
> default runtime is not specified and pass the location of jlink output.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15971#pullrequestreview-1649720770

Re: RFR: JDK-8316897: tools/jpackage/junit tests fail on AIX after JDK-8316547 [v3]

2023-09-26 Thread Alexander Matveev

On Tue, 26 Sep 2023 12:20:55 GMT, Matthias Baesken  wrote:

>> AIX currently does not have the jdk.jpackage system module. We have to take 
>> this into account for these jpackage tests.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   remove added blank lines

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15916#pullrequestreview-1645006874

Re: RFR: 8316547: Use JUnit.dir jtreg property with jpackage JUnit tests

2023-09-19 Thread Alexander Matveev

On Tue, 19 Sep 2023 20:15:20 GMT, Alexey Semenyuk  wrote:

> Strip
> 
> /*
>  * @test
>  * @modules jdk.jpackage
>  * @compile --patch-module jdk.jpackage=${test.src} --add-reads 
> jdk.jpackage=ALL-UNNAMED --add-exports 
> jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED AppImageFileTest.java
>  * @run junit/othervm --patch-module jdk.jpackage=${test.classes} --add-reads 
> jdk.jpackage=ALL-UNNAMED --add-exports 
> jdk.jpackage/jdk.jpackage.internal=ALL-UNNAMED 
> jdk.jpackage.internal.AppImageFileTest
>  */
> 
> comment from jpackage JUnit tests and instruct jtreg to run them through 
> `JUnit.dirs` property.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15826#pullrequestreview-1634191968

Re: RFR: 8301247: JPackage app-image exe launches multiple exe's in JDK 17+ [v2]

2023-09-15 Thread Alexander Matveev

On Fri, 15 Sep 2023 13:32:39 GMT, Alexey Semenyuk  wrote:

>> On Windows, restart app launcher in a way that when the parent app launcher 
>> process terminates, the child app launcher process is automatically 
>> terminated.
>
> Alexey Semenyuk has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Misplaced canRunLauncher() check.
>  - Added a test to cover the test case

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15690#pullrequestreview-1629860059

Re: RFR: 8314909: tools/jpackage/windows/Win8282351Test.java fails with java.lang.AssertionError: Expected [0]. Actual [1618]:

2023-09-15 Thread Alexander Matveev

On Fri, 15 Sep 2023 15:40:25 GMT, Alexey Semenyuk  wrote:

> Run jpackage jtreg tests on Windows sequentially. Asynch execution of 
> `msiexec.exe` that unpacks msi files doesn't work well.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15766#pullrequestreview-1629741284

Re: [External] : Re: RFR: 8308042: [macos] Developer ID Application Certificate not picked up by jpackage if it contains UNICODE characters

2023-08-23 Thread Alexander Matveev

Hi Michael,

On Aug 22, 2023, at 11:33 PM, Michael Hall 
mailto:mik3h...@gmail.com>> wrote:



On Aug 22, 2023, at 9:40 PM, Alan Snyder 
mailto:javali...@cbfiddle.com>> wrote:



On Aug 22, 2023, at 4:42 PM, Alexander Matveev 
mailto:alexander.matv...@oracle.com>> wrote:

Hi Alan,

On Aug 22, 2023, at 3:35 PM, Alan Snyder 
mailto:javali...@cbfiddle.com>> wrote:

I’m confused by this.

The issue is marked as macOS, but on macOS you don’t need to “find” the 
certificate, codesign finds it using the text supplied by the user. jpackage 
does not need to understand this text.

This is done for error handling. If signing is requested jpackage tries to find 
the certificate to make sure it is exist and is not expired and will exit with 
error if it does not exist or expired. In case if we just pass user provided 
information to codesign, then jpackage will fail during signing and after app 
image was generated. jpackage will fail faster if user mistyped certificate 
name in case when jpackage checks for it first.


The problem with this solution is that it introduces bugs. This bug and 
JDK-8311877 both result from jpackage trying to perform its own certificate 
search instead of letting codesign do the search.

The claimed advantage of failing “faster" is negligible (it is a small 
difference and only happens when the user has made a mistake) and not worth the 
(proven) risk of introducing bugs.

If you think you can do a better job of diagnosing the failure to find a 
certificate than codesign, then you should post-process the failure of 
codesign. But I don’t see this as a worthwhile investment.


Second reason is that both jpackage and codesign will find certificate if it 
contains provided key name/identity. codesign will fail if it finds two or more 
certificates, but jpackage will use first one with warning message.


Surely codesign can handle certificates with unicode names, can’t it?

Yes it can, but problem was is that our certificate validation code was not 
able to handle certificates with Unicode names.


Exactly my point. By doing your own certificate validation you risk doing it 
wrong.


Assuming code sign will catch the errors discussed or even not.

Would it make sense to do post validation of the entire app after completion?

Good point, but I am not sure if we should be doing this.

Thanks,
Alexander


echo '***'
echo 'verifying signature'
echo '***'
codesign -v --verbose=4 outputdir/HalfPipe.app
echo ''
echo 'spctl assess install'
echo ''
#spctl --assess --type install --verbose=4 outputdir/HalfPipe.app
echo ''
echo 'spctl assess execute'
echo ''
#spctl --assess --type execute --verbose=4 outputdir/HalfPipe.app

I don’t remember why I killed the spctl checks. Maybe they were flagging errors 
that weren’t?

Re: [External] : Re: RFR: 8308042: [macos] Developer ID Application Certificate not picked up by jpackage if it contains UNICODE characters

2023-08-23 Thread Alexander Matveev

Hi Alan,

On Aug 22, 2023, at 7:40 PM, Alan Snyder
mailto:javali...@cbfiddle.com>> wrote:

On Aug 22, 2023, at 4:42 PM, Alexander Matveev
mailto:alexander.matv...@oracle.com>> wrote:

Hi Alan,

On Aug 22, 2023, at 3:35 PM, Alan Snyder
mailto:javali...@cbfiddle.com>> wrote:

I’m confused by this.

The issue is marked as macOS, but on macOS you don’t need to “find” the
certificate, codesign finds it using the text supplied by the user. jpackage
does not need to understand this text.

This is done for error handling. If signing is requested jpackage tries to find
the certificate to make sure it is exist and is not expired and will exit with
error if it does not exist or expired. In case if we just pass user provided
information to codesign, then jpackage will fail during signing and after app
image was generated. jpackage will fail faster if user mistyped certificate
name in case when jpackage checks for it first.

The problem with this solution is that it introduces bugs. This bug and
JDK-8311877 both result from jpackage trying to perform its own certificate
search instead of letting codesign do the search.

The claimed advantage of failing “faster" is negligible (it is a small
difference and only happens when the user has made a mistake) and not worth the
(proven) risk of introducing bugs.

If you think you can do a better job of diagnosing the failure to find a
certificate than codesign, then you should post-process the failure of
codesign. But I don’t see this as a worthwhile investment.

JDK-8308042 is about fixing bug in current approach used by jpackage. You
suggestion is to passthrough value from “--mac-signing-key-user-name” to
codesign and use it as value for "--sign” argument in codesign will change
behavior of “--mac-signing-key-user-name” and I do not think that we should be
doing this as part of this bug fix.

From documentation:
https://docs.oracle.com/en/java/javase/20/jpackage/support-application-features.html#GUID-8D9F0607-91F4-4070-8823-02FCAB12238D

If you want to distribute your application outside the Mac App Store, then
you'll need the certificates "Developer ID Application: "
and "Developer ID Installer: ".

If you want to deploy your application through the Mac App Store, then you'll
need the certificates "3rd Party Mac Developer Application: " and "3rd Party Mac Developer Installer: ”.

If --mac-app-store is set we will look for "3rd Party Mac Developer
Application: ” and if we cannot find it we will fallback to
"Developer ID Application: ” and if it is not found we will
fail. If --mac-app-store is not set we will look for "Developer ID Application:
”. is value provided via
“--mac-signing-key-user-name”.

I think you suggestion is to add additional argument to jpackage which will be
used as passthrough value for "--sign” argument in codesign. I am still not
sure if we should be doing this, but I am currently considering it as a
solution for https://bugs.openjdk.org/browse/JDK-8311877 instead of adding
"Apple Development” as fallback if we cannot find "Developer ID Application”.

Thanks,
Alexander

Second reason is that both jpackage and codesign will find certificate if it
contains provided key name/identity. codesign will fail if it finds two or more
certificates, but jpackage will use first one with warning message.

Surely codesign can handle certificates with unicode names, can’t it?

Yes it can, but problem was is that our certificate validation code was not
able to handle certificates with Unicode names.

Exactly my point. By doing your own certificate validation you risk doing it
wrong.

Thanks,
Alexander

On Aug 22, 2023, at 3:15 PM, Alexander Matveev
mailto:almat...@openjdk.org>> wrote:

- Added support for certificates with UNICODE characters.
- Added new approach to find certificate using "security" and "openssl"
commands. Just "security" does not works, since it can truncate certificates
name. "security" is used to dump certificate in PEM format and then "openssl"
to get subject form PEM.
- New approach works with UNICODE and ASCII, but I left old approach to avoid
regressions. If old approach fails to find certificate (UNICODE or very long
subject case) we will fallback to new approach.
- Added tests for UNICODE to SigningAppImageTest and SigningPackageTest.java. I
do not think that we need to cover other signing cases for UNICODE.

Commit messages:
- 8308042: [macos] Developer ID Application Certificate not picked up by
jpackage if it contains UNICODE characters

Changes: https://git.openjdk.org/jdk/pull/15394/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=15394=00
Issue: https://bugs.openjdk.org/browse/JDK-8308042
Stats: 444 lines in 11 files changed: 248 ins; 60 del; 136 mod
Patch: https://git.openjdk.org/jdk/pull/15394.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/15394/head:pull/15394

PR: https://git.openjdk.org/jdk/pull/15394

Integrated: 8308042: [macos] Developer ID Application Certificate not picked up by jpackage if it contains UNICODE characters

2023-08-23 Thread Alexander Matveev

On Tue, 22 Aug 2023 21:59:50 GMT, Alexander Matveev  
wrote:

> - Added support for certificates with UNICODE characters.
> - Added new approach to find certificate using "security" and "openssl" 
> commands. Just "security" does not works, since it can truncate certificates 
> name. "security" is used to dump certificate in PEM format and then "openssl" 
> to get subject form PEM.
> - New approach works with UNICODE and ASCII, but I left old approach to avoid 
> regressions. If old approach fails to find certificate (UNICODE or very long 
> subject case) we will fallback to new approach.
> - Added tests for UNICODE to SigningAppImageTest and SigningPackageTest.java. 
> I do not think that we need to cover other signing cases for UNICODE.

This pull request has now been integrated.

Changeset: 57a322da
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/57a322da9bf6aac98e834516728fb6da1c18e7aa
Stats: 444 lines in 11 files changed: 248 ins; 60 del; 136 mod

8308042: [macos] Developer ID Application Certificate not picked up by jpackage 
if it contains UNICODE characters

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/15394

Re: RFR: 8308042: [macos] Developer ID Application Certificate not picked up by jpackage if it contains UNICODE characters

2023-08-22 Thread Alexander Matveev

Hi Alan,

> On Aug 22, 2023, at 3:35 PM, Alan Snyder  wrote:
> 
> I’m confused by this.
> 
> The issue is marked as macOS, but on macOS you don’t need to “find” the 
> certificate, codesign finds it using the text supplied by the user. jpackage 
> does not need to understand this text.

This is done for error handling. If signing is requested jpackage tries to find 
the certificate to make sure it is exist and is not expired and will exit with 
error if it does not exist or expired. In case if we just pass user provided 
information to codesign, then jpackage will fail during signing and after app 
image was generated. jpackage will fail faster if user mistyped certificate 
name in case when jpackage checks for it first.

Second reason is that both jpackage and codesign will find certificate if it 
contains provided key name/identity. codesign will fail if it finds two or more 
certificates, but jpackage will use first one with warning message. 

> 
> Surely codesign can handle certificates with unicode names, can’t it?

Yes it can, but problem was is that our certificate validation code was not 
able to handle certificates with Unicode names.

Thanks,
Alexander

> 
>> On Aug 22, 2023, at 3:15 PM, Alexander Matveev  wrote:
>> 
>> - Added support for certificates with UNICODE characters.
>> - Added new approach to find certificate using "security" and "openssl" 
>> commands. Just "security" does not works, since it can truncate certificates 
>> name. "security" is used to dump certificate in PEM format and then 
>> "openssl" to get subject form PEM.
>> - New approach works with UNICODE and ASCII, but I left old approach to 
>> avoid regressions. If old approach fails to find certificate (UNICODE or 
>> very long subject case) we will fallback to new approach.
>> - Added tests for UNICODE to SigningAppImageTest and 
>> SigningPackageTest.java. I do not think that we need to cover other signing 
>> cases for UNICODE.
>> 
>> -
>> 
>> Commit messages:
>> - 8308042: [macos] Developer ID Application Certificate not picked up by 
>> jpackage if it contains UNICODE characters
>> 
>> Changes: https://git.openjdk.org/jdk/pull/15394/files
>> Webrev: https://webrevs.openjdk.org/?repo=jdk=15394=00
>> Issue: https://bugs.openjdk.org/browse/JDK-8308042
>> Stats: 444 lines in 11 files changed: 248 ins; 60 del; 136 mod
>> Patch: https://git.openjdk.org/jdk/pull/15394.diff
>> Fetch: git fetch https://git.openjdk.org/jdk.git pull/15394/head:pull/15394
>> 
>> PR: https://git.openjdk.org/jdk/pull/15394
>> 
>

RFR: 8308042: [macos] Developer ID Application Certificate not picked up by jpackage if it contains UNICODE characters

2023-08-22 Thread Alexander Matveev

- Added support for certificates with UNICODE characters.
- Added new approach to find certificate using "security" and "openssl" 
commands. Just "security" does not works, since it can truncate certificates 
name. "security" is used to dump certificate in PEM format and then "openssl" 
to get subject form PEM.
- New approach works with UNICODE and ASCII, but I left old approach to avoid 
regressions. If old approach fails to find certificate (UNICODE or very long 
subject case) we will fallback to new approach.
- Added tests for UNICODE to SigningAppImageTest and SigningPackageTest.java. I 
do not think that we need to cover other signing cases for UNICODE.

-

Commit messages:
 - 8308042: [macos] Developer ID Application Certificate not picked up by 
jpackage if it contains UNICODE characters

Changes: https://git.openjdk.org/jdk/pull/15394/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15394=00
  Issue: https://bugs.openjdk.org/browse/JDK-8308042
  Stats: 444 lines in 11 files changed: 248 ins; 60 del; 136 mod
  Patch: https://git.openjdk.org/jdk/pull/15394.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15394/head:pull/15394

PR: https://git.openjdk.org/jdk/pull/15394

Integrated: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-11 Thread Alexander Matveev

On Thu, 10 Aug 2023 22:58:18 GMT, Alexander Matveev  
wrote:

> - This is regression from 
> [JDK-8298488](https://bugs.openjdk.org/browse/JDK-8298488).
> - Since JDK-8298488 unsigned app bundles are ad-hoc signed and `codesign` 
> will report that app bundle is signed and thus our tests failed.
> - Fixed tests by checking that all app bundles are signed and by checking how 
> they signed ad-hoc vs actual certificate.
> - Unsigned post process image will be ad-hoc re-sign when generating DMG or 
> PKG, since we adding `.package` file which makes ad-hoc signature invalid. 
> This is similar to [JDK-8293462](https://bugs.openjdk.org/browse/JDK-8293462).

This pull request has now been integrated.

Changeset: ec0cc630
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/ec0cc6300a02dd92b25d9072b8b3859dab583bbd
Stats: 94 lines in 8 files changed: 58 ins; 0 del; 36 mod

8313904: [macos] All signing tests which verifies unsigned app images are 
failing

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/15235

Re: [External] : Re: RFR: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-11 Thread Alexander Matveev

Hi Michael,

On Aug 10, 2023, at 10:48 PM, Michael Hall 
mailto:mik3h...@gmail.com>> wrote:



I assume done with jpackage by indicating something like —mac-sign only? If 
wrong feel free to correct.

No, it is always done if —mac-sign is NOT specified and we doing ad-hoc signing 
on app bundle only. PKG will not be ad-hoc signed.
If —mac-sign is provided we will use certificate provided with —mac-sign to 
sign app image and PKG as before.

Thanks,
Alexander

I always do dmg or app-image for quick testing. I haven’t done any pkg yet.

So if I understand correctly now for app bundle types - dmg or app-image, some 
signing is always done. Certificate if signing is indicated and it is provided 
or adhoc if it isn’t indicated as a default. I didn’t know this.

Yes, but ad-hoc signing was introduced back in JDK 20 with 
https://bugs.openjdk.org/browse/JDK-8298488 for macOS x64 and in JDK 19 with 
https://bugs.openjdk.org/browse/JDK-8277493 for macOS aarch64. We do ad-hoc 
signing for app images when generating PKG as well, so it is for all targets 
and not only for app-image or dmg.

Thanks,
Alexander

Re: RFR: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-10 Thread Alexander Matveev

Hi Michael,

On Aug 10, 2023, at 6:11 PM, Michael Hall 
mailto:mik3h...@gmail.com>> wrote:



On Aug 10, 2023, at 6:35 PM, Michael Hall 
mailto:mik3h...@gmail.com>> wrote:



On Aug 10, 2023, at 6:21 PM, Alexander Matveev 
mailto:almat...@openjdk.org>> wrote:

- Fixed tests by checking that all app bundles are signed and by checking how 
they signed ad-hoc vs actual certificate.

How is ad-hoc signing done?



If it’s this, I guess I got it
https://developer.apple.com/documentation/security/seccodesignatureflags/1397793-adhoc

Yes, this is how it is done.


I assume done with jpackage by indicating something like —mac-sign only? If 
wrong feel free to correct.

No, it is always done if —mac-sign is NOT specified and we doing ad-hoc signing 
on app bundle only. PKG will not be ad-hoc signed.
If —mac-sign is provided we will use certificate provided with —mac-sign to 
sign app image and PKG as before.

Thanks,
Alexander

RFR: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-10 Thread Alexander Matveev

- This is regression from 
[JDK-8298488](https://bugs.openjdk.org/browse/JDK-8298488).
- Since JDK-8298488 unsigned app bundles are ad-hoc signed and `codesign` will 
report that app bundle is signed and thus our tests failed.
- Fixed tests by checking that all app bundles are signed and by checking how 
they signed ad-hoc vs actual certificate.
- Unsigned post process image will be ad-hoc re-sign when generating DMG or 
PKG, since we adding `.package` file which makes ad-hoc signature invalid. This 
is similar to [JDK-8293462](https://bugs.openjdk.org/browse/JDK-8293462).

-

Commit messages:
 - 8313904: [macos] All signing tests which verifies unsigned app images are 
failing

Changes: https://git.openjdk.org/jdk/pull/15235/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15235=00
  Issue: https://bugs.openjdk.org/browse/JDK-8313904
  Stats: 94 lines in 8 files changed: 58 ins; 0 del; 36 mod
  Patch: https://git.openjdk.org/jdk/pull/15235.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15235/head:pull/15235

PR: https://git.openjdk.org/jdk/pull/15235

Re: RFR: 8311104: dangling-gsl warning in libwixhelper.cpp

2023-07-26 Thread Alexander Matveev

On Wed, 26 Jul 2023 18:42:47 GMT, Alexey Semenyuk  wrote:

> 8311104: dangling-gsl warning in libwixhelper.cpp

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15044#pullrequestreview-1548666131

Re: RFR: 8311104: dangling-gsl warning in libwixhelper.cpp

2023-07-26 Thread Alexander Matveev

On Wed, 26 Jul 2023 18:42:47 GMT, Alexey Semenyuk  wrote:

> 8311104: dangling-gsl warning in libwixhelper.cpp

src/jdk.jpackage/windows/native/libwixhelper/libwixhelper.cpp line 75:

> 73: void findInstalledProducts(const Guid& upgradeCode,
> 74: std::vector& 
> products) {
> 75: const tstring upgradeCodeStr = upgradeCode.toMsiString();

I am not familiar with this warning. Does it mean that in original `const 
LPCTSTR upgradeCodeStr = upgradeCode.toMsiString().c_str();`, `tstring` retuned 
from `toMsiString()` will get destroyed right after` c_str()` completed? Which 
will make `upgradeCodeStr` invalid?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15044#discussion_r1275432462

Re: RFR: 8312488: tools/jpackage/share/AppLauncherEnvTest.java fails with dynamically linked libstdc++ [v2]

2023-07-24 Thread Alexander Matveev

On Sat, 22 Jul 2023 21:41:06 GMT, Vladimir Petko  wrote:

>> Use the  same approach as Logger::defaultLogger() to ensure that the 
>> appender is not destroyed before shared object destructor function is 
>> executed.
>> 
>> The alternative is to delete dcon() function, but we might run into a 
>> similiar tear-down issue later (e.g. logging from some object's destructor).
>> 
>> 
>> ==
>> Test summary
>> ==
>>TEST  TOTAL  PASS  FAIL ERROR 
>>   
>>jtreg:test/jdk/tools215   215 0 0 
>>   
>> ==
>> TEST SUCCESS
>
> Vladimir Petko has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   copyright typo

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14971#pullrequestreview-1543908185

Re: RFR: 8312488: tools/jpackage/share/AppLauncherEnvTest.java fails with dynamically linked libstdc++

2023-07-21 Thread Alexander Matveev

On Fri, 21 Jul 2023 10:55:13 GMT, Vladimir Petko  wrote:

> Use the  same approach as Logger::defaultLogger() to ensure that the appender 
> is not destroyed before shared object destructor function is executed.
> 
> The alternative is to delete dcon() function, but we might run into a 
> similiar tear-down issue later (e.g. logging from some object's destructor).
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk/tools215   215 0 0  
>  
> ==
> TEST SUCCESS

src/jdk.jpackage/share/native/common/app.cpp line 2:

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

Missing "," after 2023. Should be: "Copyright (c) 2020, 2023, Oracle and/or its 
affiliates.".

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14971#discussion_r1271102509

Re: RFR: JDK-8309032: jpackage does not work for module projects unless --module-path is specified

2023-07-19 Thread Alexander Matveev

On Thu, 20 Jul 2023 02:04:53 GMT, Alexey Semenyuk  wrote:

>> [JDK-8309032](https://bugs.openjdk.org/browse/JDK-8309032), 
>> [JDK-8306488](https://bugs.openjdk.org/browse/JDK-8306488)
>
> @sashamatveev would you like to review the change before I sponsor it?

@alexeysemenyukoracle Looks good.

-

PR Comment: https://git.openjdk.org/jdk/pull/14840#issuecomment-1643003305

Re: RFR: JDK-8309032: jpackage does not work for module projects unless --module-path is specified

2023-07-19 Thread Alexander Matveev

On Tue, 11 Jul 2023 23:47:30 GMT, airsquared  wrote:

> [JDK-8309032](https://bugs.openjdk.org/browse/JDK-8309032), 
> [JDK-8306488](https://bugs.openjdk.org/browse/JDK-8306488)

Marked as reviewed by almatvee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14840#pullrequestreview-1538311227

Re: RFR: 8304914: Use OperatingSystem, Architecture, and Version in jpackage [v3]

2023-05-18 Thread Alexander Matveev

On Mon, 15 May 2023 22:51:52 GMT, Roger Riggs  wrote:

>> Refactor the Platform class in jdk.jpackage to use the internal 
>> OperatingSystem, Architecture, and Version classes.
>> The OperatingSystem.isXXX() and Architecture.isYYY() methods replace 
>> comparisons in the Platform class.
>> The checks of the os.version are replaced but may not be needed if OpenJDK 
>> no longer supports them.
>> 
>> It is recommended to remove os version checks that apply only to Mac 
>> versions before 10.15.
>> Mac OS X 10.15 is the oldest version supported.
>
> Roger Riggs has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 34 commits:
> 
>  - Merge branch 'master' into 8304914-os-jpackage
>  - The OperatingSystem enum treats AIX separately from Linux. The original 
> Platform used the same enum for both.
>Whereever OperatingSystem.isLinux is used in jpackage it should include 
> isAix as well.
>  - Minor source code style cleanup
>  - Merge branch 'master' into 8304914-os-jpackage
>  - Merge branch '8306678-os-version' into 8304914-os-jpackage
>  - Merge branch '8304915-arch-enum' into 8306678-os-version
>  - Correct comment on isPPC64() and refer to isLittleEndian() instead of 
> mentioning PPC64LE
>  - Simplify initialization in ClassLoaderHelper and fix VersionTest.
>  - Revert changes to MacOsX sun.nio.fs.BsdFileStore; the version check is 
> being removed in another PR.
>  - Review comment updates
>  - ... and 24 more: https://git.openjdk.org/jdk/compare/7b0b9b57...64ab7126

Based on jpackage Platform.java we defined Linux as `os.indexOf("nix") >= 0 || 
os.indexOf("nux") >= 0`, I might be wrong but I think it will not detect AIX in 
original implementation. Do you know why it is added to jpackage? Also, did we 
test jpackage on AIX system?

-

PR Comment: https://git.openjdk.org/jdk/pull/13586#issuecomment-1553665336

Re: RFR: 8304914: Use OperatingSystem, Architecture, and Version in jpackage [v2]

2023-05-18 Thread Alexander Matveev

On Mon, 15 May 2023 20:10:38 GMT, Roger Riggs  wrote:

>> src/jdk.jpackage/share/classes/jdk/jpackage/internal/CLIHelp.java line 51:
>> 
>>> 49: Log.info(I18N.getString("MSG_Help_no_args"));
>>> 50: } else {
>>> 51: OperatingSystem platform = OperatingSystem.current();
>> 
>> Should we add if (Log.isVerbose) back? Otherwise default case in switch 
>> statement never executed.
>
> I was puzzled by that check early on and its still a puzzle. It seems a bit 
> of a hack overloaded on the platform dispatch.
> As it turns out, the only time that `Log.isVerbose()` is true when 
> `showHelp()` is called is when the environment variable `JPACKAGE_DEBUG` is 
> used for debugging.
> `showHelp()` is only called before argument parsing is done and it doesn't 
> look for --verbose. See `Main.execute()` 
> So checking for isVerbose has no practical function.

Makes sense. Looks good.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/13586#discussion_r1198309008

Re: RFR: 8304914: Use OperatingSystem, Architecture, and Version in jpackage [v2]

2023-05-12 Thread Alexander Matveev

On Fri, 5 May 2023 14:12:21 GMT, Roger Riggs  wrote:

>> Refactor the Platform class in jdk.jpackage to use the internal 
>> OperatingSystem, Architecture, and Version classes.
>> The OperatingSystem.isXXX() and Architecture.isYYY() methods replace 
>> comparisons in the Platform class.
>> The checks of the os.version are replaced but may not be needed if OpenJDK 
>> no longer supports them.
>> 
>> It is recommended to remove os version checks that apply only to Mac 
>> versions before 10.15.
>> Mac OS X 10.15 is the oldest version supported.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Minor source code style cleanup

src/jdk.jpackage/share/classes/jdk/jpackage/internal/CLIHelp.java line 51:

> 49: Log.info(I18N.getString("MSG_Help_no_args"));
> 50: } else {
> 51: OperatingSystem platform = OperatingSystem.current();

Should we add if (Log.isVerbose) back? Otherwise default case in switch 
statement never executed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/13586#discussion_r1192826773

Re: RFR: 8294806: jpackaged-app ignores splash screen from jar file

2023-04-10 Thread Alexander Matveev

On Mon, 10 Apr 2023 21:16:19 GMT, Alexey Semenyuk  wrote:

> When `--main-jar` is specified and the name of the main class is picked from 
> the jar's manifest, set `app.jarfile` property in launcher cfg file instead 
> of `app.mainclass` and `app.classpath` properties to make app launcher run 
> the app from the jar and not as a class from the classpath.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13413#pullrequestreview-1378179729

Integrated: 8298995: tools/jpackage/share/AddLauncherTest.java#id1 failed "AddLauncherTest.test; checks=53"

2023-03-17 Thread Alexander Matveev

On Mon, 13 Mar 2023 16:50:51 GMT, Alexander Matveev  
wrote:

> This is similar to many other issues we had with `hdiutil` failures. Fixed in 
> similar way by repeating `hdiutil` command. Cannot confirm that it will 
> resolve this issue, but similar issues got resolved by repeating command. 
> Also, switched writing output from `hdiutil` to file. It is known issue that 
> execution of `hdiutil` can hang if output is not written to file.

This pull request has now been integrated.

Changeset: c56f011b
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/c56f011baa4ce7f44661f6ade8e313f812730c02
Stats: 49 lines in 2 files changed: 31 ins; 0 del; 18 mod

8298995: tools/jpackage/share/AddLauncherTest.java#id1 failed 
"AddLauncherTest.test; checks=53"

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/13002

RFR: 8298995: tools/jpackage/share/AddLauncherTest.java#id1 failed "AddLauncherTest.test; checks=53"

2023-03-16 Thread Alexander Matveev

This is similar to many other issues we had with `hdiutil` failures. Fixed in 
similar way by repeating `hdiutil` command. Cannot confirm that it will resolve 
this issue, but similar issues got resolved by repeating command. Also, 
switched writing output from `hdiutil` to file. It is known issue that 
execution of `hdiutil` can hang if output is not written to file.

-

Commit messages:
 - 8298995: tools/jpackage/share/AddLauncherTest.java#id1 failed 
"AddLauncherTest.test; checks=53"

Changes: https://git.openjdk.org/jdk/pull/13002/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=13002=00
  Issue: https://bugs.openjdk.org/browse/JDK-8298995
  Stats: 49 lines in 2 files changed: 31 ins; 0 del; 18 mod
  Patch: https://git.openjdk.org/jdk/pull/13002.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/13002/head:pull/13002

PR: https://git.openjdk.org/jdk/pull/13002

Re: RFR: JDK-8304063: tools/jpackage/share/AppLauncherEnvTest.java fails when checking LD_LIBRARY_PATH [v2]

2023-03-16 Thread Alexander Matveev

On Thu, 16 Mar 2023 09:08:08 GMT, Matthias Baesken  wrote:

>> The test fails on Alpine Linux 3.17, when checking the environment variable 
>> LD_LIBRARY_PATH; looks like the actual env variable is much longer than the 
>> test expects. It turned out that at least on Linux (probably also on other 
>> OS like AIX)  the actual env variable has the expected string at it's end, 
>> but might contain more path entries ( LD_LIBRARY_PATH can be adjusted by jvm 
>> - 
>> https://github.com/openjdk/jdk/blob/master/src/java.base/unix/native/libjli/java_md.c
>>   ).
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   redo check

Marked as reviewed by almatvee (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/13041

Integrated: 8299779: Test tools/jpackage/share/jdk/jpackage/tests/MainClassTest.java timed out

2023-03-10 Thread Alexander Matveev

On Tue, 7 Mar 2023 00:56:02 GMT, Alexander Matveev  wrote:

> - Fixed by increasing test timeout. Fix verified on host which reproduced 
> issue.

This pull request has now been integrated.

Changeset: 0a4d54f7
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/0a4d54f7ce2ac906a8012ed92c84ed8303cb4b90
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod

8299779: Test tools/jpackage/share/jdk/jpackage/tests/MainClassTest.java timed 
out

Reviewed-by: asemenyuk

-

PR: https://git.openjdk.org/jdk/pull/12896

RFR: 8299779: Test tools/jpackage/share/jdk/jpackage/tests/MainClassTest.java timed out

2023-03-06 Thread Alexander Matveev

- Fixed by increasing test timeout. Fix verified on host which reproduced issue.

-

Commit messages:
 - 8299779: Test tools/jpackage/share/jdk/jpackage/tests/MainClassTest.java 
timed out

Changes: https://git.openjdk.org/jdk/pull/12896/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=12896=00
  Issue: https://bugs.openjdk.org/browse/JDK-8299779
  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/12896.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12896/head:pull/12896

PR: https://git.openjdk.org/jdk/pull/12896

Re: RFR: 8298735: Some tools/jpackage/windows/* tests fails with jtreg test timeout

2023-01-09 Thread Alexander Matveev

On Mon, 9 Jan 2023 22:53:18 GMT, Alexey Semenyuk  wrote:

> Increase failed test timeouts.
> Simple tests got an x1.5 increase and parametrized tests got an x2 increase

Marked as reviewed by almatvee (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/11914

Re: RFR: 8299278: tools/jpackage/share/AddLauncherTest.java#id1 failed AddLauncherTest.bug8230933

2023-01-09 Thread Alexander Matveev

On Thu, 5 Jan 2023 20:10:45 GMT, Alexey Semenyuk  wrote:

> 8299278: tools/jpackage/share/AddLauncherTest.java#id1 failed 
> AddLauncherTest.bug8230933

Marked as reviewed by almatvee (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/11868

Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v5]

On Fri, 16 Dec 2022 03:38:54 GMT, Damon Nguyen  wrote:

>> Open l10n drop
>> All tests passed
>
> Damon Nguyen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Revert old translation. Fix lang codes

jpackage changes looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR: https://git.openjdk.org/jdk20/pull/35

Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]

On Fri, 16 Dec 2022 00:24:26 GMT, Damon Nguyen  wrote:

>> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_ja.properties
>>  line 37:
>> 
>>> 35: 
>>> resource.post-app-image-script=\u30A2\u30D7\u30EA\u30B1\u30FC\u30B7\u30E7\u30F3\u30FB\u30A4\u30E1\u30FC\u30B8\u3092\u79FB\u5165\u3057\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8
>>> 36: 
>>> resource.post-msi-script=exe\u30A4\u30F3\u30B9\u30C8\u30FC\u30E9\u306Emsi\u30D5\u30A1\u30A4\u30EB\u304C\u4F5C\u6210\u3055\u308C\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8
>>> 37: resource.wxl-file-name=MsiInstallerStrings_en.wxl
>> 
>> Why this was changed?
>
> Also not sure as it is what I received from the latest translation drop. If 
> this is incorrect, I can manually correct this here and for the other locales 
> for this file.

Yes it needs to be correct. It was changed to _ja, etc with 
[commit](https://github.com/openjdk/jdk20/commit/543163a03b5f1af7a7e7af317a26eb8c5aa81c38#diff-afb8ea68d8b3e9be4bba1c417c82d2162eea9333a3157f05ef57c026af94eb3b).
 Which was done on Aug 10, 2022, so it seems message drop is based on old code.

-

PR: https://git.openjdk.org/jdk20/pull/35

Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]

On Fri, 16 Dec 2022 00:15:51 GMT, Damon Nguyen  wrote:

>> src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties
>>  line 82:
>> 
>>> 80: 
>>> 81: error.foreign-app-image=Fehler: .jpackage.xml-Datei fehlt in 
>>> app-image-Verzeichnis ({0})
>>> 82: error.invalid-app-image=Fehler: app-image-Verzeichnis ({0}) wurde von 
>>> einer anderen jpackage-Version generiert, oder .jpackage.xml ist nicht 
>>> wohlgeformt
>> 
>> error.invalid-app-image looks like old translation and missing "{1}". Also, 
>> why () is used instead of ""?
>
> I'm not sure for the decisions of change in punctuation and for the missing 
> {1} as I didn't do the translation myself. Would this be better to be 
> reverted?

Looks like translation is done on files before JDK-8293462 was integrated 
(before Sep 26). See 
[commit](https://github.com/openjdk/jdk20/commit/1e222bccd3807c1be0d1d824e0ff9745751d8375#diff-8bfeb61c827c2bc073c65cbf4137dbf8f7baa5f4f88e5d59785e985d9510577c)
 when I changed these two strings. For now it is better to revert them to 
original English version or figure out why translation is old.

-

PR: https://git.openjdk.org/jdk20/pull/35

Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v4]

On Thu, 15 Dec 2022 23:01:35 GMT, Damon Nguyen  wrote:

>> Open l10n drop
>> All tests passed
>
> Damon Nguyen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Revert double quote as well

src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/MsiInstallerStrings_ja.wxl
 line 15:

> 13:Id="ShortcutPromptDlgStartMenuShortcutControlLabel">スタート・メニューのショートカットの作成
> 14:   [ProductName]セットアップ
> 15:   Open with [ProductName]

Why it was removed in all translations, except English?

src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_de.properties
 line 51:

> 49: error.msi-product-version-minor-out-of-range=Nebenversion muss im Bereich 
> [0, 255] liegen
> 50: error.version-swap=Versionsinformationen f\u00FCr {0} konnten nicht 
> aktualisiert werden
> 51: error.icon-swap=Failed to update icon for {0}

Why this was removed?

-

PR: https://git.openjdk.org/jdk20/pull/35

Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]