date:20220512

On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

This pull request has now been integrated.

Changeset: c3bade2e
Author:Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/c3bade2e08f865bf1e65d48e6d27bff9c022d35f
Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod

8286623: Bundle zlib by default with JDK on macos aarch64

Reviewed-by: lancea, ihse, erikj

-

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

Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

Thank you everyone for the reviews.

-

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

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




> On May 12, 2022, at 3:13 PM, Magnus Ihse Bursie  wrote:
> 
> On Thu, 12 May 2022 04:15:50 GMT, Alexander Matveev  
> wrote:
> 
>>> - It is not possible to support native JDK commands such as "java" inside 
>>> Mac App Store bundles due to embedded info.plist. Workarounds suggested in 
>>> JDK-8286122 does not seems to be visible.
>>> - With proposed fix we will enforce "--strip-native-commands" option for 
>>> jlink, so native JDK commands are not included when generating Mac App 
>>> Store bundles.
>>> - Custom runtime provided via --runtime-image should not contain native 
>>> commands as well, otherwise jpackage will throw error.
>>> - Added two tests to validate fix.
>> 
>> Alexander Matveev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>  8286122: [macos]: App bundle cannot upload to Mac App Store due to 
>> info.plist embedded in java exe [v2]
> 
> We have the `NSMicrophoneUsageDescription` permission on the `java` launcher 
> in the JDK, since otherwise no Java program can access the mike, even though 
> most won't care. I agree that the situation is different for a jpackaged app, 
> where the developer knows if that permission is needed or not.

I’d have to agree with Apple DTS that this is an interesting exception. 

> 
> Yes, plistbuddy is an official Apple program.
> 
> My understanding of the PR was that native commands are removed by jlink if 
> the user is packaging on a mac for the App Store. I thought this was a 
> workaround that solved the immediate problem of not being able to submit the 
> app to App Store. (However, I don't know how the app is supposed to be 
> started without a launcher…)
> 

I thought that if the app was indicated as intended for the App Store and 
native commands were also indicated an error would be thrown and the app not 
built. It doesn’t allow apps that will fail to attempt the App Store but does 
nothing for getting them there. Switching from an error to a warning and 
forcing the native commands to be stripped would allow the app into the app 
store but with unknown functionality probably not working. My understanding.

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

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

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

We have the `NSMicrophoneUsageDescription` permission on the `java` launcher in 
the JDK, since otherwise no Java program can access the mike, even though most 
won't care. I agree that the situation is different for a jpackaged app, where 
the developer knows if that permission is needed or not.

Yes, plistbuddy is an official Apple program.

My understanding of the PR was that native commands are removed by jlink if the 
user is packaging on a mac for the App Store. I thought this was a workaround 
that solved the immediate problem of not being able to submit the app to App 
Store. (However, I don't know how the app is supposed to be started without a 
launcher...)

-

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v4]

2022-05-12 Thread Phil Race

On Thu, 12 May 2022 01:27:30 GMT, Yasumasa Suenaga  wrote:

>> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 
>> on Fedora 36.
>> As you can see, the warnings spreads several areas. Let me know if I should 
>> separate them by area.
>> 
>> * -Wstringop-overflow
>> * src/hotspot/share/oops/array.hpp
>> * 
>> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>> 
>> In member function 'void Array::at_put(int, const T&) [with T = unsigned 
>> char]',
>> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
>> inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
>> inlined from 'ConstantPool* 
>> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:
>
> Yasumasa Suenaga has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Calculate char offset before realloc()

I will see what upstream thinks about the harfbuzz warning but in the mean time 
we can just disable it.

-

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

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



> What you say sounds good, although I feel I only partially understand it. :-)

If it’s correct that normal applications don’t need the executable included 
Info.plist and jpackage can somehow get versions that don’t include it that 
seems the easiest and most hack free solution.
Apple DTS did suggest including it might avoid some TCC check but I don’t know 
what that might be about.
I don’t know how many apps try to include native commands but I have at least 
one non-App Store one that does. 
An attempt at a solution seems worthwhile rather than just throwing errors when 
a developer tries to get theirs into the App Store.
 

> I assume the important point here is that the app get the 
> NSMicrophoneUsageDescription property, and afaict this can be provided by the 
> Info.plist file for the entire application, as you say. And possible the 
> problem here is that we embed metadata in the java executable at the same 
> time.


Uh, this is something I again don’t understand. It seems questionable that most 
java applications would need this Info.plist key granting access to the 
microphone.

https://developer.apple.com/documentation/bundleresources/information_property_list/nsmicrophoneusagedescription

It should be up to the developer to include this in their application 
Info.plist if their application requires it. That is where I thought my earlier 
suggestion of allowing application post-processing before doing separate 
standalone signing would make sense. The current jpackage solution of allowing 
the developer to include a separate complete alternate Info.plist in a separate 
directory I found somewhat awkward.
With post-processing you could include a tool like PlistBuddy

https://www.unix.com/man-page/osx/8/PLISTBUDDY/

To script the Info.plist changes. I believe this is used by native OS/X 
applications for this purpose including from XCode.
I think it is actually an Apple tool. With a funner name than usual for them.

> There seems to be a lot of guessing here. :-) I assume we either need to read 
> up on how this works (which might be difficult if this is seem as a badly 
> documented corner case even by Apple tech support), or test some 
> alternatives, or perhaps both.
> 
> That solution make take some time to get correct, so the jpackage team needs 
> to decide if they want to go with the workaround in this PR in the meantime.

OK, one last possible misunderstanding on my part. The PR simply throws an 
error if commands are included. It doesn’t involve a workaround does it?

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




> 
> What you say sounds good, although I feel I only partially understand it. :-)

If it’s correct that normal applications don’t need the executable included 
Info.plist and jpackage can somehow get versions that don’t include it that 
seems the easiest and most hack free solution.
Apple DTS did suggest including it might avoid some TCC check but I don’t know 
what that might be about.
I don’t know how many apps try to include native commands but I have at least 
one non-App Store one that does. 
An attempt at a solution seems worthwhile rather just throwing errors when a 
developer tries to get theirs into the App Store.
 
> 
> I assume the important point here is that the app get the 
> NSMicrophoneUsageDescription property, and afaict this can be provided by the 
> Info.plist file for the entire application, as you say. And possible the 
> problem here is that we embed metadata in the java executable at the same 
> time.
> 

Uh, this is something I again don’t understand. It seems questionable that most 
java applications would need this Info.plist key granting access to the 
microphone.
https://developer.apple.com/documentation/bundleresources/information_property_list/nsmicrophoneusagedescription
 


It should be up to the developer to include this in their application 
Info.plist if their application requires it. That is where I thought my earlier 
suggestion of allowing application post-processing before standalone signing 
would make sense. The current jpackage solution of allowing the developer to 
include a separate complete alternate Info.plist in a separate directory I 
found somewhat awkward.
With post-processing you could include a tool like PlistBuddy
https://www.unix.com/man-page/osx/8/PLISTBUDDY/ 


To script the Info.plist changes. I believe this is used by native OS/X 
applications for this purpose including from XCode.
I think it is actually an Apple tool. With a funner name than usual for them.

> There seems to be a lot of guessing here. :-) I assume we either need to read 
> up on how this works (which might be difficult if this is seem as a badly 
> documented corner case even by Apple tech support), or test some 
> alternatives, or perhaps both.
> 
> That solution make take some time to get correct, so the jpackage team needs 
> to decide if they want to go with the workaround in this PR in the meantime.
> 

OK, one last possible misunderstanding on my part. The PR simply throws an 
error if commands are included. It doesn’t involve a workaround does it?

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v4]

2022-05-12 Thread Kim Barrett

On Thu, 12 May 2022 01:29:13 GMT, Yasumasa Suenaga  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Calculate char offset before realloc()
>
> Thanks for all to review this PR! I think we should separate this issue as 
> following:
> 
> * Suppress warnings
> * make/modules/java.desktop/lib/Awt2dLibraries.gmk
> * src/hotspot/share/classfile/bytecodeAssembler.cpp
> * src/hotspot/share/classfile/classFileParser.cpp
> * 
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
> * src/hotspot/share/opto/memnode.cpp
> * src/hotspot/share/opto/type.cpp
> * src/hotspot/share/utilities/compilerWarnings.hpp
> * src/hotspot/share/utilities/compilerWarnings_gcc.hpp
> * src/java.base/unix/native/libjli/java_md_common.c
> * Bug fixes
> * src/java.base/share/native/libjli/java.c
> * src/java.base/share/native/libjli/parse_manifest.c
> * src/jdk.jpackage/linux/native/applauncher/LinuxPackage.c
> 
> I want to include the change of Awt2dLibraries.gmk (HarfBuzz) in this PR 
> because it is 3rd party library.
> 
> I will separate in above if I do not hear any objections, and this issue (PR) 
> handles "suppress warnings" only.

> @YaSuenag From my PoV this sounds like a good suggestion.

+1

-

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

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




Aren't we embedding this bundle ID in every launcher, so you would 
need a .template for each possible launcher that could be 
included in an app?
I naively assumed that only the java launcher was needed, since this is 
about packaging a Java app, so all you'd need is an entry to your main 
class. Is this not the case?




What I think we need to do is first evaluate if we actually need to 
embed this bundle ID in the executables at all, or rather, what would 
the consequences be if we didn't. My understanding is that we only do 
this to be able to run them outside of a bundle directory structure, 
but this would need some pretty thorough testing of course. If we are 
to generate a special set of launchers for jpackage, maybe all we need 
to do is not embed any bundle ID in them, as they are meant to only be 
used within a jpackaged app, so they should be covered by Info.plist 
files anyway.


What you say sounds good, although I feel I only partially understand 
it. :-)


I assume the important point here is that the app get the 
NSMicrophoneUsageDescription property, and afaict this can be provided 
by the Info.plist file for the entire application, as you say. And 
possible the problem here is that we embed metadata in the java 
executable at the same time.


There seems to be a lot of guessing here. :-) I assume we either need to 
read up on how this works (which might be difficult if this is seem as a 
badly documented corner case even by Apple tech support), or test some 
alternatives, or perhaps both.


That solution make take some time to get correct, so the jpackage team 
needs to decide if they want to go with the workaround in this PR in the 
meantime.


/Magnus


/Erik

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

2022-05-12 Thread Alexey Semenyuk

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

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

Marked as reviewed by asemenyuk (Reviewer).

-

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

Integrated: 8282191: Implementation of Foreign Function & Memory API (Preview)

2022-05-12 Thread Maurizio Cimadamore

On Mon, 21 Mar 2022 10:45:27 GMT, Maurizio Cimadamore  
wrote:

> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

This pull request has now been integrated.

Changeset: 2c5d1362
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.java.net/jdk/commit/2c5d136260fa717afa374db8b923b7c886d069b7
Stats: 65711 lines in 373 files changed: 43720 ins; 19432 del; 2559 mod

8282191: Implementation of Foreign Function & Memory API (Preview)

Reviewed-by: erikj, jvernee, psandoz, dholmes, mchung

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v34]

2022-05-12 Thread Maurizio Cimadamore

On Fri, 29 Apr 2022 08:29:52 GMT, Guoxiong Li  wrote:

>>> Remind: please use the command `/jep JEP-424` [1] to mark this PR.
>>> 
>>> [1] 
>>> https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/jep
>> 
>> Question: I'm willing to try it out. If something goes wrong - would a `jep 
>> unneeded` be enough to "unstuck" this PR?
>
>> would a jep unneeded be enough to "unstuck" this PR?
> 
> Yes if no bug. Conceptually, the `/jep unneeded` will behave as no jep 
> command.

@lgxbslgx - JEP has been targeted, but the JEP action seems to be blocking this 
- what should we do?

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]

2022-05-12 Thread Maurizio Cimadamore

> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

Maurizio Cimadamore has updated the pull request with a new target base due to 
a merge or a rebase. The pull request now contains 65 commits:

 - Merge branch 'master' into foreign-preview
 - Merge branch 'master' into foreign-preview
 - Fix crashes in heap segment benchmarks due to misaligned access
 - Merge branch 'master' into foreign-preview
 - Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java
   
   Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
 - Add tests for loaderLookup/restricted method corner cases
 - * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI
   * Tweak restricted method check to work when there's no caller class
 - Tweak examples in SymbolLookup javadoc
 - Merge branch 'master' into foreign-preview
 - Update 
src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
   
   Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
 - ... and 55 more: https://git.openjdk.java.net/jdk/compare/82aa0455...f170415b

-

Changes: https://git.openjdk.java.net/jdk/pull/7888/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7888=44
  Stats: 65711 lines in 373 files changed: 43720 ins; 19432 del; 2559 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7888.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888

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

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

2022-05-12 Thread Scott Palmer




> On May 12, 2022, at 8:10 AM, Michael Hall  wrote:
> 
> 
> 
>> 
>> My primary suggestion would to be to use an UUID for the unique ID. They are 
>> of fixed length, are for all intents and purposes unique and you can conjure 
>> them up from your hat. (An alternative is that the user needs to specify a 
>> unique ID, but that is probably a less ideal solution.) Presumably, we can 
>> have some kind of prefix like "org.openjdk.jpackage." before the UUID to 
>> make them a bit understandable, if someone where to inspect the package 
>> metadata.e 
> 
> I was thinking jpackage would change the XXX to app name but a fixed size 
> unique field would probably be better.
>> 
>> This seems like a fully workable solution to me. However, I'd really like to 
>> understand option #1 better, which was: "Package the `java` executable in a 
>> standard bundle, replacing `runtime/Contents/Home/bin/java` with a symlink 
>> to that."
>> 
>> I don't know what a "standard bundle" is, or how you would go around to 
>> package the java executable in one. But on the surface, it sounds much nicer 
>> than binary editing.
>> 
>> I also don't understand if that means that the standard bundle needs to be 
>> created at jpackage time, so it gives us the chance to set a proper ID, or 
>> if the standard bundle can be created at build time, and then the existence 
>> of this bundle just makes Apple avoid the bundle ID collision checks. Or if 
>> I'm just misunderstanding it all...
>> 
>> /Magnus
> 
> I may again not understanding but I was thinking they were talking about 
> something like symlinks to a machine installed JDK and this seemed to me to 
> defeat some of the purpose of embedding the jdk. But he could be thinking 
> else. Something external to the application anyhow it seemed.
> 

I thought they meant something like the embedded JDK would be like a framework 
bundle. Since the framework is expected to be the same in multiple apps it 
would be excluded from the duplicate id check. (I think that is related to the 
older bug that the Apple guy thought might have come back.)

Scott

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

2022-05-12 Thread erik . joelsson




On 2022-05-12 04:58, Magnus Ihse Bursie wrote:

On 2022-05-12 13:17, Michael Hall wrote:
A solution like including a bundle identifier something like 
net.java.openjdk.MYAPP.java would be possible at packaging time but 
not at build time.
To fix this at build time you would need to generate a name unique to 
each installed jdk. Including release 
net.java.openjdk.JDK_RELEASE.java might avoid some conflicts but 
would still be open to conflict for two applications at the same 
release. So it can’t be addressed ‘before the fact’ either. The only 
thing I am currently thinking of that might work would be include a 
replaceable part in the identifier. So something like 
net.java.openjdk.java.XX
Where jpackage could include something to change the X…. to a 
unique application name. If you don’t change the string size you 
could probably avoid some of the resizing issues Apple DTS mentions. 
Whether there is a standard Apple tool to do this I don’t know.


As you say, we're a bit in a bind since the java executable needs to 
be created when the JDK is built, but the bundle ID needs to be 
determined when jpackage is run (and a suitable, per-application ID 
can be created), and there is no standard tooling to update the bundle 
ID after build time.


I believe what you are describing is exactly solution #2 suggested by 
the Apple engineer. I don't think that would be horribly difficult to 
achieve, though. Sure, it's a bit of a hack, but not the ugliest I've 
seen in my days. If we go down this route, I suppose we do something 
like this:


1) When building the JDK, we create an additional copy of the java 
executable, and store it with the jdk.jpackage resources. Let's call 
it java.template, for the sake of it. This is identical to the real 
bin/java except for the fact that it contains a different bundle ID, 
with a large enough padding field, like X...  This way, we don't 
have to mess around with the real java executable for the JDK.


Aren't we embedding this bundle ID in every launcher, so you would need 
a .template for each possible launcher that could be included 
in an app?


What I think we need to do is first evaluate if we actually need to 
embed this bundle ID in the executables at all, or rather, what would 
the consequences be if we didn't. My understanding is that we only do 
this to be able to run them outside of a bundle directory structure, but 
this would need some pretty thorough testing of course. If we are to 
generate a special set of launchers for jpackage, maybe all we need to 
do is not embed any bundle ID in them, as they are meant to only be used 
within a jpackaged app, so they should be covered by Info.plist files 
anyway.


/Erik

Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

2022-05-12 Thread Erik Joelsson

On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

Marked as reviewed by erikj (Reviewer).

-

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

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




> 
> My primary suggestion would to be to use an UUID for the unique ID. They are 
> of fixed length, are for all intents and purposes unique and you can conjure 
> them up from your hat. (An alternative is that the user needs to specify a 
> unique ID, but that is probably a less ideal solution.) Presumably, we can 
> have some kind of prefix like "org.openjdk.jpackage." before the UUID to make 
> them a bit understandable, if someone where to inspect the package metadata.e 

I was thinking jpackage would change the XXX to app name but a fixed size 
unique field would probably be better.
> 
> This seems like a fully workable solution to me. However, I'd really like to 
> understand option #1 better, which was: "Package the `java` executable in a 
> standard bundle, replacing `runtime/Contents/Home/bin/java` with a symlink to 
> that."
> 
> I don't know what a "standard bundle" is, or how you would go around to 
> package the java executable in one. But on the surface, it sounds much nicer 
> than binary editing.
> 
> I also don't understand if that means that the standard bundle needs to be 
> created at jpackage time, so it gives us the chance to set a proper ID, or if 
> the standard bundle can be created at build time, and then the existence of 
> this bundle just makes Apple avoid the bundle ID collision checks. Or if I'm 
> just misunderstanding it all...
> 
> /Magnus

I may again not understanding but I was thinking they were talking about 
something like symlinks to a machine installed JDK and this seemed to me to 
defeat some of the purpose of embedding the jdk. But he could be thinking else. 
Something external to the application anyhow it seemed.

Re: RFR: 8286601: Mac Aarch: Excessive warnings to be ignored for build jdk

2022-05-12 Thread Adam Farley

On Wed, 11 May 2022 20:55:29 GMT, Adam Farley  wrote:

> These warnings are ignored while building the build+full jdks with gcc,
> but only ignored while building the full JDK if using clang. This
> produces tons of warnings we normally ignore, e.g. unused parameter.
> 
> I suggest that if we're ignoring this type of error while using gcc,
> we should also ignore them while using clang.
> 
> Signed-off-by: Adam Farley 

Thanks Erik and Magnus. :)

-

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

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


On 2022-05-12 13:17, Michael Hall wrote:
I had read this but possibly didn’t grok the issue myself. If I 
understand correctly now the conflict isn’t within the application but 
across applications to some other application that has already been 
added to the Mac App Store that included the commands with the given 
CFBundleIdentifier. 


Yes, that is indeed how I also interpret this. Presumably, the very 
first developer to submit a jpackaged application to App Store (from 
what I can tell now OpenJDK itself is not present there, which I 
mistakenly believed before) got everything working without troubles, but 
blocked all other developers from submitting their apps.


A solution like including a bundle identifier something like 
net.java.openjdk.MYAPP.java would be possible at packaging time but 
not at build time.
To fix this at build time you would need to generate a name unique to 
each installed jdk. Including release 
net.java.openjdk.JDK_RELEASE.java might avoid some conflicts but would 
still be open to conflict for two applications at the same release. So 
it can’t be addressed ‘before the fact’ either. The only thing I am 
currently thinking of that might work would be include a replaceable 
part in the identifier. So something like 
net.java.openjdk.java.XX
Where jpackage could include something to change the X…. to a 
unique application name. If you don’t change the string size you could 
probably avoid some of the resizing issues Apple DTS mentions. Whether 
there is a standard Apple tool to do this I don’t know.


As you say, we're a bit in a bind since the java executable needs to be 
created when the JDK is built, but the bundle ID needs to be determined 
when jpackage is run (and a suitable, per-application ID can be 
created), and there is no standard tooling to update the bundle ID after 
build time.


I believe what you are describing is exactly solution #2 suggested by 
the Apple engineer. I don't think that would be horribly difficult to 
achieve, though. Sure, it's a bit of a hack, but not the ugliest I've 
seen in my days. If we go down this route, I suppose we do something 
like this:


1) When building the JDK, we create an additional copy of the java 
executable, and store it with the jdk.jpackage resources. Let's call it 
java.template, for the sake of it. This is identical to the real 
bin/java except for the fact that it contains a different bundle ID, 
with a large enough padding field, like X...  This way, we don't 
have to mess around with the real java executable for the JDK.


2) At jpackage time, this java.template file is installed instead of 
bin/java, and the padding field is replaced by a unique value. The java 
executable is small (33kB on macOS, currently) so a simple search 
through the binary field for the pattern is likely to work alright. As 
long as there are no checksums being broken, this should be straightforward.


My primary suggestion would to be to use an UUID for the unique ID. They 
are of fixed length, are for all intents and purposes unique and you can 
conjure them up from your hat. (An alternative is that the user needs to 
specify a unique ID, but that is probably a less ideal solution.) 
Presumably, we can have some kind of prefix like "org.openjdk.jpackage." 
before the UUID to make them a bit understandable, if someone where to 
inspect the package metadata.


This seems like a fully workable solution to me. However, I'd really 
like to understand option #1 better, which was: "Package the `java` 
executable in a standard bundle, replacing 
`runtime/Contents/Home/bin/java` with a symlink to that."


I don't know what a "standard bundle" is, or how you would go around to 
package the java executable in one. But on the surface, it sounds much 
nicer than binary editing.


I also don't understand if that means that the standard bundle needs to 
be created at jpackage time, so it gives us the chance to set a proper 
ID, or if the standard bundle can be created at build time, and then the 
existence of this bundle just makes Apple avoid the bundle ID collision 
checks. Or if I'm just misunderstanding it all...


/Magnus

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

> On May 12, 2022, at 4:42 AM, Magnus Ihse Bursie 
>  wrote:
> 
>> Some further spelunking reveals the issue. Consider this: 
>> 
>> % otool -s __TEXT __info_plist -v 
>> APP_NAME.app/Contents/runtime/Contents/Home/bin/java 
>> … 
>>  
>> CFBundleIdentifier 
>> net.java.openjdk.java 
>> … 
>>  
>>  
>> 
>> This is an obscure corner case in the macOS bundle story: A standalone tool, 
>> like `java`, which isn’t in a bundle structure, and thus can’t have a 
>> standalone `Info.plist` file, can put its information property list in a 
>> `__TEXT` / `__info_plist` section within its executable. And it seems that 
>> the folks who created your Java runtime did just that. 
>> 
>> Given that, the Mac App Store error is valid: You are trying to submit an 
>> executable whose bundle ID matches some existing app. 
>> 
>> To get around this check you’ll need to give `java` a new bundle ID. That’s 
>> not easy to do. This `__TEXT` / `__info_plist` section is set up by a linker 
>> option (see `-sectcreate` in  
>> and there’s no good way to modify it after the fact [1]. 

I had read this but possibly didn’t grok the issue myself. If I understand 
correctly now the conflict isn’t within the application but across applications 
to some other application that has already been added to the Mac App Store that 
included the commands with the given CFBundleIdentifier. A solution like 
including a bundle identifier something like net.java.openjdk.MYAPP.java would 
be possible at packaging time but not at build time. 
To fix this at build time you would need to generate a name unique to each 
installed jdk. Including release net.java.openjdk.JDK_RELEASE.java might avoid 
some conflicts but would still be open to conflict for two applications at the 
same release. So it can’t be addressed ‘before the fact’ either. The only thing 
I am currently thinking of that might work would be include a replaceable part 
in the identifier. So something like 
net.java.openjdk.java.XX
Where jpackage could include something to change the X…. to a unique 
application name. If you don’t change the string size you could probably avoid 
some of the resizing issues Apple DTS mentions. Whether there is a standard 
Apple tool to do this I don’t know.

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v4]

On Thu, 12 May 2022 01:29:13 GMT, Yasumasa Suenaga  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Calculate char offset before realloc()
>
> Thanks for all to review this PR! I think we should separate this issue as 
> following:
> 
> * Suppress warnings
> * make/modules/java.desktop/lib/Awt2dLibraries.gmk
> * src/hotspot/share/classfile/bytecodeAssembler.cpp
> * src/hotspot/share/classfile/classFileParser.cpp
> * 
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
> * src/hotspot/share/opto/memnode.cpp
> * src/hotspot/share/opto/type.cpp
> * src/hotspot/share/utilities/compilerWarnings.hpp
> * src/hotspot/share/utilities/compilerWarnings_gcc.hpp
> * src/java.base/unix/native/libjli/java_md_common.c
> * Bug fixes
> * src/java.base/share/native/libjli/java.c
> * src/java.base/share/native/libjli/parse_manifest.c
> * src/jdk.jpackage/linux/native/applauncher/LinuxPackage.c
> 
> I want to include the change of Awt2dLibraries.gmk (HarfBuzz) in this PR 
> because it is 3rd party library.
> 
> I will separate in above if I do not hear any objections, and this issue (PR) 
> handles "suppress warnings" only.

@YaSuenag From my PoV this sounds like a good suggestion.

-

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

Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]

2022-05-12 Thread Maxim Kartashev

On Thu, 12 May 2022 04:25:24 GMT, David Holmes  wrote:

>> This change seem to have made this group of declarations obsolete: 
>> `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow 
>> the [link](
>> https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)).
>>  Does anyone have plans to drop those? Have any bugs been filed for that? If 
>> not, I'll attend to that myself.
>
> @mkartashev  all references to WINVER in the AWT code seem obsolete. Possibly 
> most of the IS_WINxxx usages could also be replaced.

@dholmes-ora @MBaesken Thank you! Filed 
[JDK-8286634](https://bugs.openjdk.java.net/browse/JDK-8286634).

-

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

Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

LGTM

-

Marked as reviewed by ihse (Reviewer).

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

Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

2022-05-12 Thread Lance Andersen

On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

Hi Jai,

Thank you for your efforts with testing and reaching out to Apple.

Given we do not see the issue with the bundled zlib, this is our best path 
forward for stability on macOS aarch64.

Once we can determine the cause with the help of Apple, we can switch back to 
the zlib that comes with macOS on this platform.

-

Marked as reviewed by lancea (Reviewer).

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

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

(cc:ing build-dev.)

On 2022-05-12 00:17, Michael Hall wrote:

Is this restricted somehow to Mac App Store applications?

Is a warning issued as stripping native commands may break application
functionality?

Is it not possible for the user to provide their own Info.plist with a
different bundle identifier that doesn’t collide?

I’m not real familiar with the build process. But would it be possible for the
user to build their own jdk that substitutes something else for the colliding
identifier that gets embedded?
This problem seem to come about as a clash between the the OpenJDK
packages themselves being submitted to Apple, as well as a
developer-specific jpackage containing JDK binaries, such as java.

The well-written response from Apple tech support in the original web
bug is very instructive. I'll quote it in its entirety:

I’ve dealt with this message before, and it has a long and interesting
history. The Mac App Store prevents two developers from submitting
executables with the same bundle identifier. This is an important
security check: We don’t want app A impersonating app B.

This check applies to all executables, not just the app’s main
executable. Historically the Mac App Store ingestion process had bugs
that caused it to apply to other types of code, like frameworks and
bundles. When I first saw this incident I was worried that these bugs
had come back.

However, that’s not the case. Let’s look at the main executables in
your app:

% find APP_NAME.app -type f -print0 | xargs -0 file | grep
"Mach-O.*executable"

APP_NAME.app/Contents/MacOS/APP_NAME: Mach-O 64-bit executable x86_64
APP_NAME.app/Contents/runtime/Contents/Home/bin/java: Mach-O 64-bit
executable x86_64
APP_NAME.app/Contents/runtime/Contents/Home/bin/keytool: Mach-O 64-bit
executable x86_64
APP_NAME.app/Contents/runtime/Contents/Home/lib/jspawnhelper: Mach-O
64-bit executable x86_64

Based on the error message it’s obvious we need to focus on the `java`
executable. However, it’s placed in a location that doesn’t have a
corresponding `Info.plist` file:

% find APP_NAME.app -name "Info.plist"
APP_NAME.app/Contents/Info.plist
APP_NAME.app/Contents/runtime/Contents/Info.plist

The first file here applies to your entire app and the second file
applies to the Java runtime package as a whole. Moreover, neither of
these have a bundle ID that matches the error message:

% plutil -extract CFBundleIdentifier raw -o -
"APP_NAME.app/Contents/Info.plist"

UNIQUE.BUNDLE.ID
% plutil -extract CFBundleIdentifier raw -o -
"APP_NAME.app/Contents/runtime/Contents/Info.plist"

com.oracle.java.com.UNIQUE.BUNDLE.ID

So where is this bundle ID coming from?

* * *

Some further spelunking reveals the issue. Consider this:

% otool -s __TEXT __info_plist -v
APP_NAME.app/Contents/runtime/Contents/Home/bin/java

…

CFBundleIdentifier
net.java.openjdk.java
…

This is an obscure corner case in the macOS bundle story: A standalone
tool, like `java`, which isn’t in a bundle structure, and thus can’t
have a standalone `Info.plist` file, can put its information property
list in a `__TEXT` / `__info_plist` section within its executable. And
it seems that the folks who created your Java runtime did just that.

Given that, the Mac App Store error is valid: You are trying to submit
an executable whose bundle ID matches some existing app.

To get around this check you’ll need to give `java` a new bundle ID.
That’s not easy to do. This `__TEXT` / `__info_plist` section is set
up by a linker option (see `-sectcreate` in )> and there’s no good way to modify it after the
fact [1].

At this point my advice is that you escalate this with your Java
runtime vendor. I suspect that they’ve added this information property
list to get around a TCC check — the only other interesting property
in there is `NSMicrophoneUsageDescription` — and they probably didn’t
notice this Mac App Store submission issue. There’s a couple of ways
they could resolve this [2] but it’s really their issue to resolve.

[1] And by “good” I mean “Using a standard tool supplied by Apple.”
The Mach-O file format is reasonably well documented and thus you
could build a custom tool to do that, but even that’s not easy. There
are a couple of problems:

* The new information property list may be larger than the previous one.

* The `__info_plist` section can appear anywhere in the `__TEXT` segment.

If you increase the size of the section then subsequent sections need
to move up to accommodate it and, depending on which sections you have
to move, that might require other cross-section links to be fixed up.
Yergh!

[2] The ones that spring to mind are:

* Package the `java` executable in a standard bundle, replacing
`runtime/Contents/Home/bin/java` with a symlink to that.

* Add an `__info_plist` section with a bunch of padding and then build
a tool to update the bundle ID in that section, taking advantage of
that padding to avoid the need to

Integrated: 8286601: Mac Aarch: Excessive warnings to be ignored for build jdk

2022-05-12 Thread Adam Farley

On Wed, 11 May 2022 20:55:29 GMT, Adam Farley  wrote:

> These warnings are ignored while building the build+full jdks with gcc,
> but only ignored while building the full JDK if using clang. This
> produces tons of warnings we normally ignore, e.g. unused parameter.
> 
> I suggest that if we're ignoring this type of error while using gcc,
> we should also ignore them while using clang.
> 
> Signed-off-by: Adam Farley 

This pull request has now been integrated.

Changeset: 40f43c6b
Author:Adam Farley 
Committer: Magnus Ihse Bursie 
URL:   
https://git.openjdk.java.net/jdk/commit/40f43c6b1ffc88d55dd3223f5d0259ae73cf0356
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod

8286601: Mac Aarch: Excessive warnings to be ignored for build jdk

Reviewed-by: erikj

-

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

Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

Dummy comment to initiate a mail to core-libs mailing list too.

-

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

RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

Can I please get a review of this change to the build system which now makes 
bundled zlib as the default for macos aarch64 systems? 

We have been running into various intermittent failures on macos aarch64 
systems for several months now. After investigation we have been able to 
ascertain that the root cause of the issue lies within the zlib that's shipped 
on macos aarch64 systems. The complete details about that issue is available at 
https://bugs.openjdk.java.net/browse/JDK-8282954. 
We have filed a bug with Apple for their inputs on what we can do to fix it in 
that shipped library. Given the nature of that issue, we don't have a timeline 
on when/if the solution for that will be available. Until that time, at least, 
the proposal is to use bundled zlib (which doesn't reproduce those failures) by 
default on macos aarch64.

-

Commit messages:
 - jib profile - don't explicitly set system zlib for macosx-aarch64
 - default to bundled zlib on macos aarch64

Changes: https://git.openjdk.java.net/jdk/pull/8675/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8675=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286623
  Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8675.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8675/head:pull/8675

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

Integrated: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled