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

[Erik Joelsson]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

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

Makefile change looks ok.

-

Marked as reviewed by erikj (Reviewer).

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

Re: RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current

[Erik Joelsson]

On Mon, 17 May 2021 16:55:35 GMT, Naoto Sato  wrote:

> Please review the changes to the subject issue. java.util.Locale class has a 
> long-standing issue for those obsolete ISO 639 languages where its 
> normalization ends up in the obsolete codes. This change intends to flip the 
> normalization towards the current codes, providing a system property for 
> compatibility behavior. ResourceBundle class is also modified to load either 
> obsolete/current bundles. For more detail, take a look at the CSR.

I see no relevant build changes to comment on as the build label was only added 
because the buildtool java source was touched, so this looks ok from a build 
perspective.

-

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

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module

[erik . joelsson]

On 2021-01-29 02:56, Magnus Ihse Bursie wrote:

On Fri, 29 Jan 2021 00:30:21 GMT, Phil Race  wrote:


This completes the desktop module JNF removal

* remove  -framework JavaNativeFoundation from make files

* remove  #import  from all source 
files. If needed add import of JNIUtilities.h to get jni.h definitions - better 
anyway since then it gets the current JDK ones not the ones from the O/S

* replace JNFNSToJavaString with NSStringToJavaString and  JNFJavaToNSString 
with JavaStringToNSString

* replace JNFNormalizedNSStringForPath with 
NormalizedPathNSStringFromJavaString and JNFNormalizedJavaStringForPath with 
NormalizedPathJavaStringFromNSString

* replace JNFGet/ReleaseStringUTF16UniChars with direct calls to JNI

* Map all JNFRunLoop perform* calls to the ThreadUtilities versions (the vast 
majority already did this)

* Redo the ThreadUtilities calls to JNFRunLoop to directly invoke NSObject 
perform* methods.

* define new javaRunLoopMode in ThreadUtilities to replace the JNF one and use 
where needed.

* Remove the single usage of JNFPerformEnvBlock

* replace JNFJavaToNSNumber in single A11Y file with local replacement

* replace single usage of JNFNSTimeIntervalToJavaMillis in ScreenMenu.m with 
local replacement

* remove un-needed JNFRunLoopDidStartNotification from NSApplicationAWT.m

* misc. remaining cleanup (eg missed JNF_CHECK_AND_RETHROW_EXCEPTION)

I mostly have questions about what is missing from this PR. :-) (If this is 
supposed to remove the final remnants of JNF)

- There is a disabled warning in `make/autoconf/flags-cflags.m4`, line 173, 
referring to JavaNativeFoundation. It can presumably be removed. If it triggers 
individually instead, the warning should be disabled on a per-library basis.

- In `make/modules/java.base/Lib.gmk`, line 99 & 113, are references to 
JavaNativeFoundation. It seems that `libosxsecurity` needs to be cleaned from JNF 
as well. Also, the comments indicate that the exception for STATIC_BUILD can be 
removed. (You can verify this with `configure --enable-static-build`)


libosxsecurity is being worked on separately, see 
https://github.com/openjdk/jdk/pull/1845


That said, we may need a separate bug for build to remove the any 
lingering global stuff after each component has been fixed.


/Erik


- In `make/modules/java.desktop/Lib.gmk`, line 129, and 
`make/modules/java.desktop/lib/Awt2dLibraries.gmk`, line 866 & 094, it seems 
like `libosx`, `libawt_lwawt`, and `liboxui` also has JNF that needs to be removed. 
If these are fixed in any of the other issues for the umbrella JDK-8257852, I 
apologize. I could not figure that out.

- There is also a test dependency that I have seen being addressed, in 
`make/test/JtregNativeJdk.gmk` line 82, for `libTestMainKeyWindow`.

-

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

Re: RFR: 8259343: [macOS] Update JNI error handling in Cocoa code. [v3]

[Erik Joelsson]

On Mon, 11 Jan 2021 19:27:12 GMT, Phil Race  wrote:

>> Proposed updates to JNI error handling.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8259343: [macOS] Update JNI error handling in Cocoa code.

Build changes look good.

-

Marked as reviewed by erikj (Reviewer).

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

Re: RFR: 8259343: [macOS] Update JNI error handling in Cocoa code. [v2]

[Erik Joelsson]

On Mon, 11 Jan 2021 17:52:19 GMT, Phil Race  wrote:

>> Proposed updates to JNI error handling.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8259343: [macOS] Update JNI error handling in Cocoa code.

make/modules/java.desktop/Lib.gmk line 96:

> 94:   $(call SET_SHARED_LIBRARY_ORIGIN), \
> 95:   LIBS := \
> 96:   -ljava \

There should be a dependency declaration added too. Something like this right 
after the SetupJdkLibrary macro call:

$(BUILD_LIBOSXAPP): $(call FindLib, java.base, java)

-

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

Re: RFR: 8251854: [macosx] Java forces the use of discrete GPU

[Erik Joelsson]

On Tue, 10 Nov 2020 08:19:13 GMT, Sergey Bylokhov  wrote:

> This is a review request for the bug particularly fixed some time ago:
> https://mail.openjdk.java.net/pipermail/2d-dev/2015-May/005425.html
> 
> In that review request it was found that the old fix does not work well in 
> all cases, see:
> https://mail.openjdk.java.net/pipermail/2d-dev/2015-August/005611.html
> 
> The current fix updates an embedded plist.info, so the java will not require
> discrete graphics by default, same as for any other applications. 
> 
> The discrete card will be used:
>  1. If macOS decided to enable it for some reason
>  2. If the java app sets/uses a full-screen window
>  3. If the user disable "automatic graphics switching" in the system 
> preferences
>  4. If an external monitor is connected to the laptop
> 
> In other cases, the integrated graphics will be used, which should be fine in 
> most cases since this graphic is used/tested in the mbp 13/etc. This is not 
> only about rendering performance but also about startup performance, on my 
> current new laptop mbp 16 + Cataline 10.15.7 the switching 
> discrete/integrated causes unexpected delays up to 10 seconds.
> 
> 
> Note that the new "metal" pipeline also does not require discrete graphics.
> 
> The documentation for NSSupportsAutomaticGraphicsSwitching:
> https://developer.apple.com/documentation/bundleresources/information_property_list/nssupportsautomaticgraphicsswitching
> 
> I'll create a release note after approval.
> 
> Performance numbers:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010979.html
> Old review request:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010973.html

Change looks ok from a build point of view, but I can't comment on the validity 
and implications of using this key.

-

Marked as reviewed by erikj (Reviewer).

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

Re: RFR: 8255798: Remove dead headless code in CompileJavaModules.gmk

[Erik Joelsson]

On Tue, 3 Nov 2020 08:21:22 GMT, Magnus Ihse Bursie  wrote:

> The variable BUILD_HEADLESS_ONLY is no longer set. And sun/applet does not 
> exist anymore.

Marked as reviewed by erikj (Reviewer).

-

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

Re: RFR(S) : 8240904 : Screen flashes on test failures when running tests from make

[Erik Joelsson]

Thanks, that looks much better to me.

/Erik

On 2020-04-16 10:02, Igor Ignatyev wrote:

Hi Erik,

sure, I've actually replaced one long statement w/ multiple shorter 
ones, which made the comment section redundant -- 
http://cr.openjdk.java.net/~iignatyev//8240904/webrev.01


Thanks,
-- Igor

On Apr 16, 2020, at 6:17 AM, Erik Joelsson <mailto:erik.joels...@oracle.com>> wrote:


Looks ok to me. Would it be possible to break up the long lines a bit 
to improve readability? Backslash escape for newlines should work in 
properties files.


/Erik

On 2020-04-15 22:22, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8240904/webrev.00

35 lines changed: 26 ins; 0 del; 10 mod


Hi all,

8233827[1] which added screenshots to so-called failure handler had 
an unexpected side-effect on linux, where users might observer 
flashes each time a screenshot is taken, which, to put it mildly, is 
annoying. the patch replaces gnome-screenshot app w/ calling 
java.awt API to make a screenshot. the patch also uses the same 
solution to make screenshots on windows and solaris (which 
previously didn't save screenshots on failures). we still use native 
app on mac as using java.awt API requires accessibility permissions, 
which might be as annoying as flashes on linux.


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

JBS: https://bugs.openjdk.java.net/browse/JDK-8240904
webrev: http://cr.openjdk.java.net/~iignatyev//8240904/webrev.00
testing: verified that screenshots are successfully generated on 
headful systems, and headless systems ignore errors 
('java.awt.AWTException: headless environment' from j.a.Robot::).


Thanks,
-- Igor

Re: RFR(S) : 8240904 : Screen flashes on test failures when running tests from make

[Erik Joelsson]

Looks ok to me. Would it be possible to break up the long lines a bit to 
improve readability? Backslash escape for newlines should work in 
properties files.


/Erik

On 2020-04-15 22:22, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8240904/webrev.00

35 lines changed: 26 ins; 0 del; 10 mod


Hi all,

8233827[1] which added screenshots to so-called failure handler had an 
unexpected side-effect on linux, where users might observer flashes each time a 
screenshot is taken, which, to put it mildly, is annoying. the patch replaces 
gnome-screenshot app w/ calling java.awt API to make a screenshot. the patch 
also uses the same solution to make screenshots on windows and solaris (which 
previously didn't save screenshots on failures). we still use native app on mac 
as using java.awt API requires accessibility permissions, which might be as 
annoying as flashes on linux.

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

JBS: https://bugs.openjdk.java.net/browse/JDK-8240904
webrev: http://cr.openjdk.java.net/~iignatyev//8240904/webrev.00
testing: verified that screenshots are successfully generated on headful systems, and 
headless systems ignore errors ('java.awt.AWTException: headless environment' from 
j.a.Robot::).

Thanks,
-- Igor

Re: RFR: JDK-8241463 Move build tools to respective modules

[Erik Joelsson]

Looks good.

/Erik

On 2020-03-23 12:03, Magnus Ihse Bursie wrote:
The build tools (small java tools that are run during the build to 
generate source code, or data, needed in the JDK) have historically 
been placed in the "make" directory. This maybe made sense long time 
ago, but does not do so anymore.


Instead, the build tools source code should move the the module that 
needs them. For instance, compilefontconfig should move to 
java.desktop, etc.


There are multiple reasons for this:

* Currently we build *all* build tools at once, which mean that we 
cannot compile java.base until e.g. the compilefontconfig tool is 
compiled, even though it is not needed.


* If a build tool, e.g. compilefontconfig is modified, all build tools 
are recompiled, which triggers a rebuild of more or less the entire 
JDK. This makes development of the build tools unnecessary tedious.


* When the build tools are modified, the group owning the 
corresponding module is the proper review instance, not the build 
team. But since they reside under "make", the review mails often 
include build-dev, but this is mostly noise for us. With this move, 
the ownership is made clear.


In this patch, I have not modified how and when the build tools are 
compiled, but this shuffle is the prerequisite for continuing with 
that in a follow-up patch.


I have also moved the build tools to the org.openjdk.buildtools.* 
package name space (inspired by Skara), instead of the strangely named 
build.tools.* name space.


A few build tools are not moved in this patch. Two of them, 
charsetmapping and cldrconverter, are shared between two modules. (I 
think they should move to modules nevertheless, but they need some 
more thought to make sure I do this right.) The rest are tools that 
are needed for the build in general, like linking or javadoc support. 
I'll move this to a better location too, but in a separate patch.


Bug: https://bugs.openjdk.java.net/browse/JDK-8241463
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8241463-move-build-tools-to-modules/webrev.01


/Magnus

Re: [15] Review Request: 8235975 Update copyright year to match last edit in jdk repository for 2014/15/16/17/18

[Erik Joelsson]

Build files look good.

/Erik

On 2019-12-24 19:22, Sergey Bylokhov wrote:

Hello.

Here is an updated version:
  Bug: https://bugs.openjdk.java.net/browse/JDK-8235975
  Patch (2 Mb): 
http://cr.openjdk.java.net/~serb/8235975/webrev.03/open.patch

  Fix: http://cr.openjdk.java.net/~serb/8235975/webrev.03/

 - "jdk.internal.vm.compiler" is removed from the patch.
 - "Aes128CtsHmacSha2EType.java" is updated to "Copyright (c) 2018"

On 12/22/19 11:24 pm, Sergey Bylokhov wrote:

Hello.
Please review the fix for JDK 15.

Bug: https://bugs.openjdk.java.net/browse/JDK-8235975
Patch (2 Mb): 
http://cr.openjdk.java.net/~serb/8235975/webrev.02/open.patch

Fix: http://cr.openjdk.java.net/~serb/8235975/webrev.02

I have updated the source code copyrights by the 
"update_copyright_year.sh"

script for 2014/15/16/18/19 years, unfortunately, cannot run it for 2017
because of: "JDK-8187443: Forest Consolidation: Move files to unified 
layout"

which touched all files.

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Erik Joelsson]

On 2019-10-25 08:42, Alexey Ivanov wrote:

Hi Dmitry,

Apple uses macOS to refer to its operating system, so should we also 
use it instead of former OS X?


For HTML version, I would suggest using the following markup 
Ctrl +F1 for the keyboard shortcut. [1]



The HTML is generated from the markup and should not be edited by hand.

/Erik

I think using  elements for instructions on how to disable 
the shortcut is incorrect. In fact, it does not require any special 
markup.



What about this text?
Some client UI tests devoted to tooltips may fail on macOS 
platform. The tests use Ctrl +F1 key 
sequence to show/hide a tooltip. However, this key combination is 
reserved by the operating system. To run these tests correctly, the 
default global shortcut should be disabled: choose Apple menu  
System Preferences, click Keyboard, then click Shortcuts; deselect 
"Turn keyboard access on/of" property.


I copied the way Apple uses in its documentation to refer to commands.

On 25/10/2019 11:27, Dmitry Markov wrote:

Hello,

Could you review the fix for jdk14, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8232880
webrev: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/

Some Client UI tests use the following key sequence to show/hide 
tooltip message: “CTRL” + “F1". However this key combination is 
reserved by operating system on OS X platform. As a results the tests 
fail.

Test documentation should be updated with some notes regarding this.

Thanks,
Dmitry



[1] https://www.w3.org/TR/html52/textlevel-semantics.html#the-kbd-element

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Erik Joelsson]

No need for new webrev.

/Erik

On 2019-10-25 06:45, Dmitry Markov wrote:

Thank you for the review, Erik!
I will update the fix as you suggested before push if you do not mind.

Thanks,
Dmitry


On 25 Oct 2019, at 14:00, Erik Joelsson  wrote:

Looks good. I would put in a "the" before "operating system".

/Erik

On 2019-10-25 03:27, Dmitry Markov wrote:

Hello,

Could you review the fix for jdk14, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8232880 
<https://bugs.openjdk.java.net/browse/JDK-8232880>
webrev: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/ 
<http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/>

Some Client UI tests use the following key sequence to show/hide tooltip message: 
“CTRL” + “F1". However this key combination is reserved by operating system on 
OS X platform. As a results the tests fail.
Test documentation should be updated with some notes regarding this.

Thanks,
Dmitry

Re: [12] Review Request: 8212680 (JDK12b14/Solaris-sparc) SplashScreen::getSplashScreen call fails with ULE: "libsplashscreen.so: ld.so.1: java: fatal: libz.so.1: open failed: No such file o

[Erik Joelsson]

Looks ok to me if we are fine with making changes to libpng source. I 
thought this was usually not something we wanted to do with upstream 
sources.


/Erik

On 2018-11-28 15:11, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8212680
Webrev: http://cr.openjdk.java.net/~serb/8212680/webrev.00

On Solaris we faced the bug which was fixed in macOS already:
  https://bugs.openjdk.java.net/browse/JDK-8196803

The problem is that there is a call to "inflateValidate" function in 
pngrutil.c[1], guarded by a preprocessor check of ZLIB_VERNUM being 
high enough and by the "PNG_IGNORE_ADLER32". If we compile this call 
in and link to the newer version of zlib, we will get link errors if 
the code is executed on an older Mac/Solaris/ with an older version of 
zlib.


The bug can be reproduced on "old" Solaris 11.3, which was not updated 
for a while(since 2015).


We can fix it by requiring some "OS Patches and Package Updates", but 
since it was
reproduced on macOS, and potentially can occur on other platforms, I 
have decided
to fix it in the code. The new property is introduced to the libpng 
"PNG_ADLER32_SUPPORTED",
which control the usage of "PNG_IGNORE_ADLER32" and as a result 
control the call to "inflateValidate"[1].
This new property is set in the makefile when we build "bundled" 
versions of libpng+zlib only.


This was reported upstream, and the future version of libpng may have 
some similar solution.


[1] 
http://hg.openjdk.java.net/jdk/jdk/file/396dfb0e8ba5/src/java.desktop/share/native/libsplashscreen/libpng/pngrutil.c#l457

Re: RFR(XXS): 8214007: Fix sun.awt.nativedebug on X11 platforms

[Erik Joelsson]

Looks ok to me.

/Erik

On 2018-11-16 10:08, Volker Simonis wrote:

Hi,

can I please have a review for the following trivial fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214007/
https://bugs.openjdk.java.net/browse/JDK-8214007

AWT supports some kind of native logging which can be enabled with
"-Dsun.awt.nativedebug=true -Dawtdebug.ctrace=true".

Unfortunately this doesn't work on X platforms any more because both,
libawt and libawt_xawt end up with a copy of debug_trace.o. Among
others, debug_trace.o contains the static variable
GlobalTracingEnabled which denotes the tracing state. This obviously
can't work if the final executable contains several instances of this
variable.

The fix is trivial. Remove "common/awt/debug" from the set of sources
for libawt_xawt. libawt_xawt is linked against libawt (which already
contains debug_trace.o) anyway, so this is no problem.

Thank you and best regards,
Volker

Re: RFR: JDK-8210705 Stop exporting all symbols on macosx

[Erik Joelsson]

Looks even better.

/Erik


On 2018-09-24 10:33, Magnus Ihse Bursie wrote:


On 2018-09-24 17:43, Erik Joelsson wrote:

Looks good.
Thanks. I realized there's one more part where we still export all 
symbols on macosx, and that's the launchers. Here's an updated webrev 
that handles them too:


http://cr.openjdk.java.net/~ihse/JDK-8210705-stop-exporting-all-symbols-on-macosx/webrev.02 



/Magnus


/Erik


On 2018-09-24 02:00, Magnus Ihse Bursie wrote:
For historical reasons, we have exported all symbols on macosx. This 
was an issue that probably arise with the very first macosx port, 
and the complication that we then used map files with a format that 
Xcode did not support.


Now we don't use mapfiles anymore, and there's no reason to keep 
exporting all symbols on macosx. Instead, we should only export the 
public symbols, just like for all other platforms.


All platform-independent code is already properly annotated with 
JNIEXPORT, but there was a few files in libosxapp that needed some 
additional JNIEXPORTs.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210705
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210705-stop-exporting-all-symbols-on-macosx/webrev.01


/Magnus

Re: RFR: JDK-8210705 Stop exporting all symbols on macosx

[Erik Joelsson]

Looks good.

/Erik


On 2018-09-24 02:00, Magnus Ihse Bursie wrote:
For historical reasons, we have exported all symbols on macosx. This 
was an issue that probably arise with the very first macosx port, and 
the complication that we then used map files with a format that Xcode 
did not support.


Now we don't use mapfiles anymore, and there's no reason to keep 
exporting all symbols on macosx. Instead, we should only export the 
public symbols, just like for all other platforms.


All platform-independent code is already properly annotated with 
JNIEXPORT, but there was a few files in libosxapp that needed some 
additional JNIEXPORTs.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210705
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210705-stop-exporting-all-symbols-on-macosx/webrev.01


/Magnus

Re: RFR: JDK-8211029 Have a common set of enabled warnings for all native libraries

[Erik Joelsson]

Looks good.

/Erik


On 2018-09-24 01:18, Magnus Ihse Bursie wrote:
With JDK-8210988, the foundation is in place for a more systematic way 
of handling warnings across all native libraries (hotspot and the JDK 
libraries alike).


With this patch, make sure we enable all warnings equally for all 
libaries. If an individual library triggers a specific warning, 
disable it in that library.


There was a single warning from clang in awt_Font.c (due to a very 
broken cast) that was not possible to turn off (unless -Wextra was 
turned off entirely), so I fixed the code instead.


I have tested that this compiles without warnings on all standard 
Oracle build platforms/toolchains. On top of that, I've also tested a 
variety of gcc's: 4.8.5, 5.5.0, 6.4.0 and 7.3.0 (the minor versions 
for each major versions shipped by Ubuntu). I've also tested clang 5.0 
on linux, XCode 9.2 on macosx and Solaris Studio 12.6 on solaris. Even 
with such extensive testing, the nature of this fix which is to add 
the flags that turn on a broad range of suitable warnings (like 
-Wall), and then selectively disable individual warnings on individual 
libraries, there's a certain risk that some platform/toolchain 
combinations that used to compile without warnings, now start 
exhibiting warnings. If this should happen, the short-term workaround 
is to use "configure --disable-warnings-as-errors". The medium term 
fix is to disable the problematic warning in the library in question, 
and the long-term solution is (hopefully) to fix the code.


Bug: https://bugs.openjdk.java.net/browse/JDK-8211029
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8211029-common-set-of-warnings/webrev.01


/Magnus

Re: RFR: JDK-8210944 Stop replacing -MD with -MT in libwindowsaccessbridge

[Erik Joelsson]

Looks good.

/Erik


On 2018-09-20 00:05, Magnus Ihse Bursie wrote:
Currently, we are filtering out -MD and replacing it with -MT when 
building libwindowsaccessbridge. This has just been a way to replicate 
the behavior of old build system, and there's no point in doing so.


In fact, it is recommended *not* to mix -MT and -MD in dlls and 
executable, as that might lead to problems (as seen in e.g. JDK-8207139).


Bug: https://bugs.openjdk.java.net/browse/JDK-8210944
Patch inline:
diff --git a/make/lib/Lib-jdk.accessibility.gmk 
b/make/lib/Lib-jdk.accessibility.gmk

--- a/make/lib/Lib-jdk.accessibility.gmk
+++ b/make/lib/Lib-jdk.accessibility.gmk
@@ -69,7 +69,7 @@
 EXTRA_SRC := common, \
 OPTIMIZATION := LOW, \
 DISABLED_WARNINGS_microsoft := 4311 4302 4312, \
-    CFLAGS := $(filter-out -MD, $(CFLAGS_JDKLIB)) -MT \
+    CFLAGS := $(CFLAGS_JDKLIB) \
 -DACCESSBRIDGE_ARCH_$2, \
 EXTRA_HEADER_DIRS := \
 include/bridge \

/Magnus

Re: 8209520: Build fails when native code coverage is enabled

[Erik Joelsson]

On 2018-08-31 01:20, Magnus Ihse Bursie wrote:


These are two different arguments for turning off warnings for code 
coverage:

1) gcc is producing incorrect warnings
2) the warnings might be correct, but we are going to treat such bugs 
as low priority



I understand and accept 1, but I do not accept 2 as a valid reason for 
turning off warnings. I tried to dig through history in the 
(Oracle-internal) bug Igor posted, but in the end I could find no 
changes was made to any compiler flags..?


I have googled around but found no discussions at all that indicate 
that gcov should emit invalid warnings.


As for argument 2: we're using code coverage as a way to increase code 
quality, after all. If we get additional warnings, that indicate ways 
to improve the code, then we should use this to improve the code, not 
hide it away. And there's no reason not to treat such bugs as severe 
as any other compile errors.



I agree and it seems I was assuming too much when I claimed 1.
Let's get more concrete: In this case, we have three warnings. To 
solve this, Leonid fixed:


* In macroAssembler_x86.cpp, he added ShouldNotReachHere(), which 
definitely improves code quality.


* In genCollectedHeap.cpp, he he initalized two local variables to 
NULL. After studying the code, I see that this is strictly not needed, 
and gcc is apparently losing some information here that could have 
been used to determine that the code is correct. However, my initial 
reaction when reading the code was that it was indeed broken, and it 
took me some time to prove that it was not. This is not good code 
readability. It's fragile, and some future code change might end up 
with a non-initialized local variable. So I think this is good, 
defensive programming, no matter what.


* In splashscreen_png.c, I'll honestly say that I don't know wether 
this fix is correct or not. But it does seem reasonable. The 
SplashDecodePng() function uses setjmp() *shudder* and for me, that's 
a big Here Be Dragons! sign. I also note that the success variable is 
read in the done: label, just below access to row_pointers and 
image_data -- which are both, already, declared volatile. So I'm 
guessing that declaring success volatile is in fact the correct thing 
to do. (I challenge anyone with a greater understanding of setjmp to 
prove me wrong...)


So I say we should be happy we got those warnings, and fix the code.

I don't see this as an argument that gcc emits invalid warnings. If 
that happens in the future, then we can discuss eliminating those 
warning. But if we do, we should eliminate them selectively, only the 
incorrect warnings and only on the files/libraries where they occur 
erroneously. Not disable all warnings in a single stroke.


Thank you for the detailed analysis. With this I definitely agree that 
it's better to fix the code.


/Erik

/Magnus

Re: 8209520: Build fails when native code coverage is enabled

[Erik Joelsson]

Hello,

On 2018-08-30 02:22, Magnus Ihse Bursie wrote:



On 2018-08-24 00:44, Igor Ignatev wrote:

Hi Leonid,

We have never supported native code coverage builds with warnings 
enabled as errors. There are bugs in gcc which cause false positive 
warnings, so it was decided to ignore all warnings from instrumented 
builds. It’d be much better and reliable to fix makefiles to always 
use ‘disable-warning-as-errors’ when ‘enable-native-coverage’ is 
used. It should be pretty straightforward to do.

I disagree.

While it is simple to change the build to disable warnings as error 
when building with code coverage, I think hard-coding this into the 
build system is a step backwards :-( and not something I want to support.


I shared your opinion at first while discussing this offline with 
Leonid. What changed my mind was the claim that the warnings cannot be 
truly trusted when GCC is generating code coverage data. If that is 
true, then having warnings as errors turned on then serves no purpose. 
The majority of builds will be without code coverage enabled, so we will 
still get ample protection against warnings in the code.
The code changes suggested by Leonid seems trivial and benign, and 
helps readability, even apart from fixing compiler warnings.


If this is deemed unacceptable, it's better to disable those few 
warnings specifically, for the files/libraries they occur in.


If the claim on the warnings produced by GCC while generating code 
coverage is false, then I certainly agree that the warnings should be 
fixed instead.


/Erik

/Magnus




cc’ing build alias.

Cheers,
— Igor

On Aug 23, 2018, at 2:37 PM, Vladimir Kozlov 
 wrote:


macroassembler changes are good.

Thanks,
Vladimir


On 8/23/18 1:51 PM, Leonid Mesnik wrote:
Hi
Could you please review following fix which fix code so gcc doesn't 
complain when JDK is build with enabled native code coverage.
webrev: http://cr.openjdk.java.net/~lmesnik/8209520/webrev.00/ 


bug: https://bugs.openjdk.java.net/browse/JDK-8209520
These warning appeared because of change optimization settings used 
for getting code coverage.
1) src/hotspot/cpu/x86/macroAssembler_x86.cpp, 
src/hotspot/share/gc/shared/genCollectedHeap.cpp

gcc complained about uninitialized variables, like
* For target hotspot_variant-server_libjvm_objs_macroAssembler_x86.o:
/home/lmesnik/ws/jdk-8209520/open/src/hotspot/cpu/x86/macroAssembler_x86.cpp: 
In member function 'void ControlWord::print() const':
/home/lmesnik/ws/jdk-8209520/open/src/hotspot/cpu/x86/macroAssembler_x86.cpp:5769:11: 
error: 'pc' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]

  printf("%04x  masks = %s, %s, %s", _value & 0x, f, rc, pc);
~~^~~~
/home/lmesnik/ws/jdk-8209520/open/src/hotspot/cpu/x86/macroAssembler_x86.cpp:5769:11: 
error: 'rc' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
So I just fixed codepath to show more explicitly that variables are 
initialized before usage.

2) src/java.desktop/share/native/libsplashscreen/splashscreen_png.c:
The changes to prevent waning about clobbering in 
splashscreen_png.c are similar to fix in:

1. JDK-8080695 
    splashscreen_png.c compile error with gcc 4.9.2
The another approach would be to fix build to ignore these warnings 
for code coverage build. While I think it makes build system even 
more complicated.

Leonid

Re: RFR: 8205494: Convert or remove all AWT applet demos

[Erik Joelsson]

Looks good.

/Erik


On 2018-06-21 15:09, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8205494
Webrev : http://cr.openjdk.java.net/~prr/8205494/

This removes all the old 1990's vintage applet demos which can't be 
run anymore.

If any are interesting enough to turn into main programs then they can be
retrieved from the mercurial history or the arched OpenJDK10 forest.

-phil

Re: RFR (XL): JDK-8204572 SetupJdkLibrary should setup SRC and -I flags automatically

[Erik Joelsson]

Looks good.

/Erik


On 2018-06-08 01:50, Magnus Ihse Bursie wrote:

On 2018-06-07 23:20, Erik Joelsson wrote:

Hello Magnus,

Very nice refactoring!

Thanks!



JdkNativeCompilation.gmk
line 126-127 looks a bit long. There is an extra space on 126. Also, 
why not addprefix for adding -I instead of clunky foreach? Not that I 
care greatly, but I usually prefer that construct.


Yeah, I agree, addprefix is better. I just forgot about it; foreach is 
a nice allround tool.


Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8204572-autodetect-SRC-and-headers-dirs/webrev.02/

(Only changes is in JdkNativeCompilation.gmk, as per above comments).

/Magnus



Otherwise looks good.

/Erik


On 2018-06-07 13:22, Magnus Ihse Bursie wrote:

This change needs some background information.

I've been working on simplifying and streamlining the compilation of 
native libraries of the JDK. Previously, I introduced the 
SetupJdkLibrary function, and started to get a better control of 
compiler flags. This patch continues on both paths.


The original intent of this change, which I naively thought was 
going to be much simpler than it turned out :-) was to separate the 
-I flags (location of header files) from other flags, and to 
generate these automatically, wherever possible. Since we have a 
standard way of locating native code, and most libraries just want 
to have an -I path to their own source base and the generated Java 
header, we should be able to provide this in SetupJdkLibrary.


This turned out to be closely related to SetupJdkLibrary being able 
to discover the location of the SRC directories itself, using 
"convention over configuration" and assuming that the library 
"libfoo" for "java.module" would be located in 
java.module/*/native/libfoo.


While this sounds simple in theory, when actually trying to 
implement this I of course ran into all the places where some 
special handling was indeed needed. So even if like 90% of all 
libraries were simple to get to build using automated discovery of 
source and header directories, the 10% that did not caused me much 
more headaches than I had anticipated. On the other hand, now that 
I've sorted out all those places, the few remaining odd solutions is 
clearly documented and not just something that "just happens" due to 
strange configurations.


One file deserves mentioning specifically: Awt2dLibraries.gmk. The 
java.desktop libraries are unfortunately quite entangled with each 
other, and do not readily follow the patterns that are used 
elsewhere in the code base. So it might just look like the file has 
just gone from one state of messiness, to another, which would 
hardly be an improvement. :-( I would still argue that the new 
messiness is better: It is now much clearer in what ways the 
libraries diverge from our standard assumption, and what course of 
action needs to be taken to minimize these differences. (Which is 
something I believe should be done -- these issues are not just 
cosmetic but is the root of most of the issues we always see for 
these libraries, when upgrading compilers, etc.)


During this change, I noticed that not all native libraries include 
the proper generated header file. This is a dangerous coding 
practice, since a change in the Java part of the interface might not 
get picked up properly in the native part. I've added the missing 
includes that I've detected, and due to these changes, I'm also 
including the component teams in what is really only a build change. 
As can be seen for jdk.crypto.mscapi; there had indeed been changes 
that needed correcting.


Since this is (basically) a pure build change, my gold standard here 
has been the build compare script. In essence, the build output 
prior to my change and with this change are 100% identical. In 
truth, this is a bit too strong a claim. A few changes has occurred, 
but none of them should matter. Here's a breakdown of the compare.sh 
results:


* Windows-x64:

No differences at all.

* Solaris:

Two libraries are reported to differ: libsaproc.so and 
libfontmanager.so, both with a disass diff on ~700 bytes. Analyzing 
this, I found that the object files used to link these two libraries 
has no disass differences. They have a slight binary difference and 
a difference in size, due to the include paths being different (and 
this is stored in the .o file, which makes it different). Somehow 
this apparently triggers the linker to generate a slightly different 
code in a few places, using a different register or so. (Weird...)


* MacOS:

Two libraries are reported to differ: libjava.dylib and 
libmlib_image.dylib, both of them just reported as a binary diff (no 
symbol, disass or fulldump differences). This is not really 
unsuspected, but I analyzed it anyway.


I found that for libjava.dylib, a single .o file was different. This 
one was actually picked up from closed sources, and are not really 
relevant for OpenJDK. Anyway, the

Re: RFR (XL): JDK-8204572 SetupJdkLibrary should setup SRC and -I flags automatically

[Erik Joelsson]

Hello Magnus,

Very nice refactoring!

JdkNativeCompilation.gmk
line 126-127 looks a bit long. There is an extra space on 126. Also, why 
not addprefix for adding -I instead of clunky foreach? Not that I care 
greatly, but I usually prefer that construct.


Otherwise looks good.

/Erik


On 2018-06-07 13:22, Magnus Ihse Bursie wrote:

This change needs some background information.

I've been working on simplifying and streamlining the compilation of 
native libraries of the JDK. Previously, I introduced the 
SetupJdkLibrary function, and started to get a better control of 
compiler flags. This patch continues on both paths.


The original intent of this change, which I naively thought was going 
to be much simpler than it turned out :-) was to separate the -I flags 
(location of header files) from other flags, and to generate these 
automatically, wherever possible. Since we have a standard way of 
locating native code, and most libraries just want to have an -I path 
to their own source base and the generated Java header, we should be 
able to provide this in SetupJdkLibrary.


This turned out to be closely related to SetupJdkLibrary being able to 
discover the location of the SRC directories itself, using "convention 
over configuration" and assuming that the library "libfoo" for 
"java.module" would be located in java.module/*/native/libfoo.


While this sounds simple in theory, when actually trying to implement 
this I of course ran into all the places where some special handling 
was indeed needed. So even if like 90% of all libraries were simple to 
get to build using automated discovery of source and header 
directories, the 10% that did not caused me much more headaches than I 
had anticipated. On the other hand, now that I've sorted out all those 
places, the few remaining odd solutions is clearly documented and not 
just something that "just happens" due to strange configurations.


One file deserves mentioning specifically: Awt2dLibraries.gmk. The 
java.desktop libraries are unfortunately quite entangled with each 
other, and do not readily follow the patterns that are used elsewhere 
in the code base. So it might just look like the file has just gone 
from one state of messiness, to another, which would hardly be an 
improvement. :-( I would still argue that the new messiness is better: 
It is now much clearer in what ways the libraries diverge from our 
standard assumption, and what course of action needs to be taken to 
minimize these differences. (Which is something I believe should be 
done -- these issues are not just cosmetic but is the root of most of 
the issues we always see for these libraries, when upgrading 
compilers, etc.)


During this change, I noticed that not all native libraries include 
the proper generated header file. This is a dangerous coding practice, 
since a change in the Java part of the interface might not get picked 
up properly in the native part. I've added the missing includes that 
I've detected, and due to these changes, I'm also including the 
component teams in what is really only a build change. As can be seen 
for jdk.crypto.mscapi; there had indeed been changes that needed 
correcting.


Since this is (basically) a pure build change, my gold standard here 
has been the build compare script. In essence, the build output prior 
to my change and with this change are 100% identical. In truth, this 
is a bit too strong a claim. A few changes has occurred, but none of 
them should matter. Here's a breakdown of the compare.sh results:


* Windows-x64:

No differences at all.

* Solaris:

Two libraries are reported to differ: libsaproc.so and 
libfontmanager.so, both with a disass diff on ~700 bytes. Analyzing 
this, I found that the object files used to link these two libraries 
has no disass differences. They have a slight binary difference and a 
difference in size, due to the include paths being different (and this 
is stored in the .o file, which makes it different). Somehow this 
apparently triggers the linker to generate a slightly different code 
in a few places, using a different register or so. (Weird...)


* MacOS:

Two libraries are reported to differ: libjava.dylib and 
libmlib_image.dylib, both of them just reported as a binary diff (no 
symbol, disass or fulldump differences). This is not really 
unsuspected, but I analyzed it anyway.


I found that for libjava.dylib, a single .o file was different. This 
one was actually picked up from closed sources, and are not really 
relevant for OpenJDK. Anyway, the reason for the difference was the 
same as for the Solaris libs; the include paths had changes, which 
caused a binary diff.


For libmlib_image.dylib, the link order had changed causing the noted 
binary difference.


* Linux:

On linux, the compare script noted differences for libextnet, libjava, 
libmlib_image, libprefs, libsaproc, libsplashscreen and libsunec.


The differences for libextnet, libprefs, libsplashscreen and libsunec 
turned 

Re: RFR: JDK-8204211: windows : handle potential C++ exception in GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using SAFE_SIZE_ARRAY_ALLOC / safe_Malloc

[Erik Joelsson]

Build change looks ok, but the validity of disabling the warning needs 
review from someone else.


/Erik


On 2018-06-05 00:47, Baesken, Matthias wrote:

Hi Christoph, thank's for the  review .
Could I have a second one  for example from the  awt or build-dev  reviewers ?

Best Regards, Matthias



-Original Message-
From: Langer, Christoph
Sent: Montag, 4. Juni 2018 16:49
To: Baesken, Matthias ; Thomas Stüfe
; 'build-...@openjdk.java.net' ; awt-dev@openjdk.java.net
Cc: 2d-dev <2d-...@openjdk.java.net>
Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in
GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using
SAFE_SIZE_ARRAY_ALLOC / safe_Malloc

Hi Matthias,

looks good to me.

Don't forget the Copyright years.

Best regards
Christoph


-Original Message-
From: Baesken, Matthias
Sent: Montag, 4. Juni 2018 16:20
To: Thomas Stüfe ; 'build-
d...@openjdk.java.net' ; awt-
d...@openjdk.java.net
Cc: 2d-dev <2d-...@openjdk.java.net>; Langer, Christoph

Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception in
GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using
SAFE_SIZE_ARRAY_ALLOC / safe_Malloc

Hello,  I prepared a second webrev.

- use now  const-reference  in the catch-statements as suggested by

Thomas

- reenabled the cl warning  showing the exception issues  in
make/lib/Awt2dLibraries.gmk
- found a second place  in  WPrinterJob.cpp   with similar issues  after
reenabling the warnings

Please review :

http://cr.openjdk.java.net/~mbaesken/webrevs/8204211.1/

(cc-ing  build-dev   because of  the makefile change,  and
src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp
because of the awt change )


Thanks, Matthias




-Original Message-
From: Baesken, Matthias
Sent: Montag, 4. Juni 2018 09:24
To: 'Thomas Stüfe' 
Cc: '2d-dev' <2d-...@openjdk.java.net>; Langer, Christoph

Subject: RE: RFR: JDK-8204211: windows : handle potential C++ exception

in

GDIRenderer -was : RE: [OpenJDK 2D-Dev] java2d coding using
SAFE_SIZE_ARRAY_ALLOC / safe_Malloc

A small update  -  I found a second  place  in WPrinterJob.cpp   where  the
exception handling is missing. After fixing both places I can reenable
warning 4297  in
Awt2dLibraries.gmk  which has been  disabled before , probably

because

of

the warnings generated when the exceptions where not handled .
Should I update the change with the other file + makefile change ?

Best regards, Matthias



hg diff

diff -r 12fe57c319e1 make/lib/Awt2dLibraries.gmk
--- a/make/lib/Awt2dLibraries.gmk  Tue Apr 10 11:02:09 2018 +0800
+++ b/make/lib/Awt2dLibraries.gmk  Mon Jun 04 09:18:03 2018 +0200
@@ -213,6 +213,7 @@
LIBAWT_CFLAGS += -fgcse-after-reload
  endif


  $(eval $(call SetupJdkLibrary, BUILD_LIBAWT, \
  NAME := awt, \
  SRC := $(LIBAWT_DIRS), \
@@ -224,7 +225,7 @@
  format-nonliteral parentheses, \
  DISABLED_WARNINGS_clang := logical-op-parentheses extern-

initializer,

\

  DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE, \
-DISABLED_WARNINGS_microsoft := 4297 4244 4267 4996, \
+DISABLED_WARNINGS_microsoft := 4244 4267 4996, \
  ASFLAGS := $(LIBAWT_ASFLAGS), \
  LDFLAGS := $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN),

\

  LDFLAGS_macosx := -L$(INSTALL_LIBRARIES_HERE), \
diff -r 12fe57c319e1


src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.cpp

---


a/src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.c

ppTue Apr 10 11:02:09 2018 +0800
+++


b/src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.c

ppMon Jun 04 09:18:03 2018 +0200
@@ -85,7 +85,13 @@
  *pNpoints = outpoints;
  }
  if (outpoints > POLYTEMPSIZE) {
-pPoints = (POINT *) SAFE_SIZE_ARRAY_ALLOC(safe_Malloc,
sizeof(POINT), outpoints);
+try {
+pPoints = (POINT *) SAFE_SIZE_ARRAY_ALLOC(safe_Malloc,
sizeof(POINT), outpoints);
+} catch (const std::bad_alloc&) {
+return NULL;
+}
  }
  BOOL isempty = fixend;
  for (int i = 0; i < npoints; i++) {
diff -r 12fe57c319e1
src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp
---

a/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp

Tue Apr 10 11:02:09 2018 +0800
+++

b/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp

Mon Jun 04 09:18:03 2018 +0200
@@ -873,7 +873,12 @@
int numSizes = ::DeviceCapabilities(printerName, printerPort,
DC_PAPERS, NULL, NULL);
if (numSizes > 0) {
-  LPTSTR papers = (LPTSTR)SAFE_SIZE_ARRAY_ALLOC(safe_Malloc,
numSizes, sizeof(WORD));
+  LPTSTR papers;
+  try {
+  papers = (LPTSTR)SAFE_SIZE_ARRAY_ALLOC(safe_Malloc,

numSizes,

sizeof(WORD));
+  } catch (const std::bad_alloc&) {
+  papers = NULL;
+  }
if (papers != NULL &&
::DeviceCapabilities(printerName, printerPort,
 

Re: [11] Review Request: 8203308 Remove the appletviewer classes

[Erik Joelsson]

Build changes look good.

/Erik


On 2018-05-21 19:39, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk11.

Bug: https://bugs.openjdk.java.net/browse/JDK-8203308
Webrev: http://cr.openjdk.java.net/~serb/8203308/webrev.00

Description:
 - Implementation of the AppletViewer was removed
 - Tests in sun/applet were removed and TEST.ROOT/TEST.groups were 
updated

 - Small update in make file was done

We still have a few classes in sun.applet package which are related to 
the client security, sound. I think that we can drop it as well at 
some point.

Re: RFR: JDK-8194327 [macos] AWT windows have incorrect main/key window behaviors

[Erik Joelsson]

Build changes look good.

/Erik


On 2018-05-16 09:18, Sergey Bylokhov wrote:

Looks fine.
cc build-dev to review changes in the make file.

On 14/05/2018 14:01, Alan Snyder wrote:

http://cr.openjdk.java.net/~serb/alans/8194327/webrev.02

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

Re: RFR: 8201429: Support AIX Input Method Editor (IME) for AWT Input Method Framework (IMF)

[Erik Joelsson]

Build change looks good.

/Erik


On 2018-05-08 02:54, Langer, Christoph wrote:

Hi Eric,

thanks for that excellent suggestion. I already thought there should be some 
means to do that but was not aware of how that could be accomplished. I updated 
the webrev in place.

Thanks
Christoph


-Original Message-
From: Erik Joelsson [mailto:erik.joels...@oracle.com]
Sent: Freitag, 4. Mai 2018 17:45
To: Langer, Christoph <christoph.lan...@sap.com>; awt-
d...@openjdk.java.net
Cc: build-...@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
Subject: Re: RFR: 8201429: Support AIX Input Method Editor (IME) for AWT
Input Method Framework (IMF)

Hello,

It looks like what you are trying to achieve is to override
awt_InputMethod.c with an OS specific version of that file. We have a
construct for this in SetupNativeCompilation that should handle it
automatically, if you just list the source dirs in priority order. I
would suggest leveraging this by making this change instead:

First in the list of LIBAWT_XAWT_DIRS (line 272), prepend a line like this:

$(wildcard
$(TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS)/native/libawt_xawt)
\

/Erik


On 2018-05-04 07:07, Langer, Christoph wrote:

Hi,

please help reviewing the contribution of the support for the AIX Input

Method Editor (IME) in AWT's Input Method Framework.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201429.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8201429

I took Ichiroh's initial proposal [1] and tried to integrate it better with

existing code. I have split
src/java.desktop/unix/classes/sun/awt/X11InputMethod.java into

a) a base class containing the common code:

src/java.desktop/unix/classes/sun/awt/X11InputMethodBase.java

b) a specific class for the common Linux/Unixes:

src/java.desktop/unix/classes/sun/awt/X11InputMethod.java and

c) a specific class for AIX:

src/java.desktop/aix/classes/sun/awt/X11InputMethod.java

The AIX specific additions to the native code of

src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c were so
much that I decided to add a specific implementation file for AIX:
src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod_aix.c. The
changes to the former file are some cleanups.

I'm still in the process of testing the changes - but maybe you can run

further tests, especially on non-AIX unixes to make sure we didn't break
something.

Thanks & Best regards
Christoph

[1]: http://mail.openjdk.java.net/pipermail/awt-dev/2018-

April/013869.html

Re: RFR: 8201429: Support AIX Input Method Editor (IME) for AWT Input Method Framework (IMF)

[Erik Joelsson]

Hello,

It looks like what you are trying to achieve is to override 
awt_InputMethod.c with an OS specific version of that file. We have a 
construct for this in SetupNativeCompilation that should handle it 
automatically, if you just list the source dirs in priority order. I 
would suggest leveraging this by making this change instead:


First in the list of LIBAWT_XAWT_DIRS (line 272), prepend a line like this:

$(wildcard 
$(TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS)/native/libawt_xawt) \


/Erik


On 2018-05-04 07:07, Langer, Christoph wrote:

Hi,

please help reviewing the contribution of the support for the AIX Input Method 
Editor (IME) in AWT's Input Method Framework.

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201429.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8201429

I took Ichiroh's initial proposal [1] and tried to integrate it better with 
existing code. I have split 
src/java.desktop/unix/classes/sun/awt/X11InputMethod.java into
a) a base class containing the common code: 
src/java.desktop/unix/classes/sun/awt/X11InputMethodBase.java
b) a specific class for the common Linux/Unixes: 
src/java.desktop/unix/classes/sun/awt/X11InputMethod.java and
c) a specific class for AIX: 
src/java.desktop/aix/classes/sun/awt/X11InputMethod.java

The AIX specific additions to the native code of 
src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c were so much 
that I decided to add a specific implementation file for AIX: 
src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod_aix.c. The changes 
to the former file are some cleanups.

I'm still in the process of testing the changes - but maybe you can run further 
tests, especially on non-AIX unixes to make sure we didn't break something.

Thanks & Best regards
Christoph

[1]: http://mail.openjdk.java.net/pipermail/awt-dev/2018-April/013869.html

Re: [8u] RFR: 8196516: libfontmanager must be built with LDFLAGS allowing unresolved symbols

[Erik Joelsson]

Looks good.

/Erik


On 2018-05-03 04:33, Severin Gehwolf wrote:

Hi,

Could I please get a review of this change for JDK 8u? We are seing
build failures due to unresolved symbols when building
libfontmanager.so. The build flag to trigger this is to configure with:

--with-extra-ldflags="-Wl,-z,defs"

Attempting a build of JDK 8 with this fails. This has been fixed in JDK
11 and hasn't shown any issues so far. Due to the change in the build
system the JDK 11 patch does not apply cleanly after path unshuffeling
(different context it seems). Hence, asking here for a review before
asking for JDK 8 backport approval.

Bug: https://bugs.openjdk.java.net/browse/JDK-8196516
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webrev.jdk8/

Review thread for JDK 11:
http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021575.html

Thanks,
Severin

Re: RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

[Erik Joelsson]

On 2018-04-13 12:44, Volker Simonis wrote:


Phil Race <philip.r...@oracle.com <mailto:philip.r...@oracle.com>> 
schrieb am Fr. 13. Apr. 2018 um 19:21:



I suppose this potentially helps the concurrency of the build ?
I can't think of why this would be a problem now there is no
compile-time linking
involved and it seems Linux was already fine without this,
but a jdk-submit would be prudent ..


I did start Solaris and AIX builds before I left the office. I can 
certainly also submit a job to JDK-submit, but at least hs-submit 
wasn’t working at all (i.e. didn’t return any results).



hs-submit is disabled since the merge of jdk/hs to jdk/jdk.

/Erik



-phil.

On 04/13/2018 09:22 AM, Volker Simonis wrote:
> Hi Erik,
>
> thanks for looking at the patch and good catch! You're right
that the
> dependency can now be removed. Here's the new webrev:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524.v1
<http://cr.openjdk.java.net/%7Esimonis/webrevs/2018/8201524.v1>
>
> Regards,
> Volker
>
> On Fri, Apr 13, 2018 at 6:00 PM, Erik Joelsson
<erik.joels...@oracle.com <mailto:erik.joels...@oracle.com>> wrote:
>> Hello Volker,
>>
>> The change looks good, but now that we no longer link against
>> libawt_headless, we should also remove the make dependency a
few lines down.
>> (Should have been done already for Solaris.)
>>
>> /Erik
>>
>>
>>
>> On 2018-04-13 06:28, Volker Simonis wrote:
>>> Hi,
>>>
>>> can I please have a review for this tiny AIX cleanup:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
<http://cr.openjdk.java.net/%7Esimonis/webrevs/2018/8201524/>
>>> https://bugs.openjdk.java.net/browse/JDK-8201524
>>>
>>> This is a follow up change of JDK-8196516 which discovered
that on AIX
>>> libfontmanager is always linked against libawt_headless at
build time.
>>> If we are running in a headfull environment, libfontmanager will
>>> dynamically load libawt_xawt which is not good because
libawt_headless
>>> and libawt_xawt define some common symbols. If we're running in a
>>> headless environment, libawt_headless may be loaded a second
time (at
>>> least on Linux/Solaris) which isn't good either.
>>>
>>> Both of these scenarios haven't caused any problems on AIX
yet, but I
>>> think it's good to cleanup the AIX implementation as well and
don't
>>> link libfontmanager against libawt_headless anymore. In order to
>>> achieve this, we have to allow unresolved symbols during the
linking
>>> of libfontmanager. This can be easily achieved by adding the
additions
>>> linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This
works fine
>>> for AIX because options which come later on the command line take
>>> precedence
>>> over earlier ones.
>>>
>>> Thank you and best regards,
>>> Volker
>>

Re: RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

[Erik Joelsson]

Yes, we don't want unneeded dependencies declared as it both potentially 
slows down the build (though this removal will not have any measurable 
impact) as well as confuses humans trying to make sense of the makefiles.


This removal should be fine as we don't link to libawt_xawt on any 
platform anymore. Linking is the only reason we have those dependencies.


/Erik


On 2018-04-13 10:20, Phil Race wrote:


I suppose this potentially helps the concurrency of the build ?
I can't think of why this would be a problem now there is no 
compile-time linking

involved and it seems Linux was already fine without this,
but a jdk-submit would be prudent ..

-phil.

On 04/13/2018 09:22 AM, Volker Simonis wrote:

Hi Erik,

thanks for looking at the patch and good catch! You're right that the
dependency can now be removed. Here's the new webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524.v1

Regards,
Volker

On Fri, Apr 13, 2018 at 6:00 PM, Erik Joelsson 
<erik.joels...@oracle.com> wrote:

Hello Volker,

The change looks good, but now that we no longer link against
libawt_headless, we should also remove the make dependency a few 
lines down.

(Should have been done already for Solaris.)

/Erik



On 2018-04-13 06:28, Volker Simonis wrote:

Hi,

can I please have a review for this tiny AIX cleanup:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
https://bugs.openjdk.java.net/browse/JDK-8201524

This is a follow up change of JDK-8196516 which discovered that on AIX
libfontmanager is always linked against libawt_headless at build time.
If we are running in a headfull environment, libfontmanager will
dynamically load libawt_xawt which is not good because libawt_headless
and libawt_xawt define some common symbols. If we're running in a
headless environment, libawt_headless may be loaded a second time (at
least on Linux/Solaris) which isn't good either.

Both of these scenarios haven't caused any problems on AIX yet, but I
think it's good to cleanup the AIX implementation as well and don't
link libfontmanager against libawt_headless anymore. In order to
achieve this, we have to allow unresolved symbols during the linking
of libfontmanager. This can be easily achieved by adding the additions
linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This works fine
for AIX because options which come later on the command line take
precedence
over earlier ones.

Thank you and best regards,
Volker

Re: RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

[Erik Joelsson]

Looks good, thanks!

/Erik


On 2018-04-13 09:22, Volker Simonis wrote:

Hi Erik,

thanks for looking at the patch and good catch! You're right that the
dependency can now be removed. Here's the new webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524.v1

Regards,
Volker

On Fri, Apr 13, 2018 at 6:00 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:

Hello Volker,

The change looks good, but now that we no longer link against
libawt_headless, we should also remove the make dependency a few lines down.
(Should have been done already for Solaris.)

/Erik



On 2018-04-13 06:28, Volker Simonis wrote:

Hi,

can I please have a review for this tiny AIX cleanup:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
https://bugs.openjdk.java.net/browse/JDK-8201524

This is a follow up change of JDK-8196516 which discovered that on AIX
libfontmanager is always linked against libawt_headless at build time.
If we are running in a headfull environment, libfontmanager will
dynamically load libawt_xawt which is not good because libawt_headless
and libawt_xawt define some common symbols. If we're running in a
headless environment, libawt_headless may be loaded a second time (at
least on Linux/Solaris) which isn't good either.

Both of these scenarios haven't caused any problems on AIX yet, but I
think it's good to cleanup the AIX implementation as well and don't
link libfontmanager against libawt_headless anymore. In order to
achieve this, we have to allow unresolved symbols during the linking
of libfontmanager. This can be easily achieved by adding the additions
linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This works fine
for AIX because options which come later on the command line take
precedence
over earlier ones.

Thank you and best regards,
Volker

Re: RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

[Erik Joelsson]

Hello Volker,

The change looks good, but now that we no longer link against 
libawt_headless, we should also remove the make dependency a few lines 
down. (Should have been done already for Solaris.)


/Erik


On 2018-04-13 06:28, Volker Simonis wrote:

Hi,

can I please have a review for this tiny AIX cleanup:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
https://bugs.openjdk.java.net/browse/JDK-8201524

This is a follow up change of JDK-8196516 which discovered that on AIX
libfontmanager is always linked against libawt_headless at build time.
If we are running in a headfull environment, libfontmanager will
dynamically load libawt_xawt which is not good because libawt_headless
and libawt_xawt define some common symbols. If we're running in a
headless environment, libawt_headless may be loaded a second time (at
least on Linux/Solaris) which isn't good either.

Both of these scenarios haven't caused any problems on AIX yet, but I
think it's good to cleanup the AIX implementation as well and don't
link libfontmanager against libawt_headless anymore. In order to
achieve this, we have to allow unresolved symbols during the linking
of libfontmanager. This can be easily achieved by adding the additions
linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This works fine
for AIX because options which come later on the command line take
precedence
over earlier ones.

Thank you and best regards,
Volker

Re: RFR: 8196516: libfontmanager must be built with LDFLAGS allowing unresolved symbols

[Erik Joelsson]

On 2018-04-10 04:30, Severin Gehwolf wrote:

On Mon, 2018-04-09 at 15:39 +0200, Severin Gehwolf wrote:

Hi,

Could somebody please review this build fix for libfontmanager.so. The
issue for us is that with some LDFLAGS the build breaks as described in
bug JDK-8196218. However, we cannot link to a providing library at
build-time since we don't know which one it should be: libawt_headless
or libawt_xawt. That has to happen at runtime. The proposed fix filters
out relevant linker flags when libfontmanager is being built. More
details are in the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8196516

Latest webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webrev.02/


Testing: I've run this through submit[1] and got the following results.
SwingSet2 works fine for me on F27. I'm currently running some more
tests on RHEL 7.

I've finished testing on RHEL 7 by manually verifying that not both
libawt_xawt and libawt_headless get loaded when running SwingSet.

Could somebody tell me what this failure below is about?
This was an internal infrastructure failure. I would say it's safe to 
ignore.


/Erik

Thanks,
Severin


-
Mach5 mach5-one-sgehwolf-JDK-8196516-20180409-1036-17877: Builds PASSED. 
Testing FAILURE.

0 Failed Tests

Mach5 Tasks Results Summary

NA: 0
UNABLE_TO_RUN: 0
EXECUTED_WITH_FAILURE: 0
KILLED: 0
PASSED: 82
FAILED: 1
Test

1 Failed

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_2-windows-x64-
debug-31 SetupFailedException in setup...profile run-test-prebuilt' ,
return value: 10


Not sure what this test failure means. Could somebody at Oracle shed
some light on this?

Thanks,
Severin

Re: RFR: 8196516: libfontmanager must be built with LDFLAGS allowing unresolved symbols

[Erik Joelsson]

Looks good. Thanks!

/Erik


On 2018-04-10 04:25, Severin Gehwolf wrote:

Hi Erik,

On Mon, 2018-04-09 at 09:20 -0700, Erik Joelsson wrote:

Hello Severin,

I'm ok with this solution for now.

Thanks for the review!


Could you please reduce the indentation on line 652. In the build system
we like 4 spaces for continuation indent [1]

Done. New webrev at:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webrev.02

Could someone from awt-dev have a look at this too? Thanks!

Cheers,
Severin


/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-04-09 06:39, Severin Gehwolf wrote:

Hi,

Could somebody please review this build fix for libfontmanager.so. The
issue for us is that with some LDFLAGS the build breaks as described in
bug JDK-8196218. However, we cannot link to a providing library at
build-time since we don't know which one it should be: libawt_headless
or libawt_xawt. That has to happen at runtime. The proposed fix filters
out relevant linker flags when libfontmanager is being built. More
details are in the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8196516
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webrev.01/

Testing: I've run this through submit[1] and got the following results.
SwingSet2 works fine for me on F27. I'm currently running some more
tests on RHEL 7.

-
Mach5 mach5-one-sgehwolf-JDK-8196516-20180409-1036-17877: Builds PASSED. 
Testing FAILURE.

0 Failed Tests

Mach5 Tasks Results Summary

NA: 0
UNABLE_TO_RUN: 0
EXECUTED_WITH_FAILURE: 0
KILLED: 0
PASSED: 82
FAILED: 1
Test

1 Failed

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_2-windows-x64-
debug-31 SetupFailedException in setup...profile run-test-prebuilt' ,
return value: 10


Not sure what this test failure means. Could somebody at Oracle shed
some light on this?

Thanks,
Severin

Re: RFR: 8196516: libfontmanager must be built with LDFLAGS allowing unresolved symbols

[Erik Joelsson]

Hello Severin,

I'm ok with this solution for now.

Could you please reduce the indentation on line 652. In the build system 
we like 4 spaces for continuation indent [1]


/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-04-09 06:39, Severin Gehwolf wrote:

Hi,

Could somebody please review this build fix for libfontmanager.so. The
issue for us is that with some LDFLAGS the build breaks as described in
bug JDK-8196218. However, we cannot link to a providing library at
build-time since we don't know which one it should be: libawt_headless
or libawt_xawt. That has to happen at runtime. The proposed fix filters
out relevant linker flags when libfontmanager is being built. More
details are in the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8196516
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webrev.01/

Testing: I've run this through submit[1] and got the following results.
SwingSet2 works fine for me on F27. I'm currently running some more
tests on RHEL 7.

-
Mach5 mach5-one-sgehwolf-JDK-8196516-20180409-1036-17877: Builds PASSED. 
Testing FAILURE.

0 Failed Tests

Mach5 Tasks Results Summary

NA: 0
UNABLE_TO_RUN: 0
EXECUTED_WITH_FAILURE: 0
KILLED: 0
PASSED: 82
FAILED: 1
Test

1 Failed

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_2-windows-x64-
debug-31 SetupFailedException in setup...profile run-test-prebuilt' ,
return value: 10


Not sure what this test failure means. Could somebody at Oracle shed
some light on this?

Thanks,
Severin

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Erik Joelsson]

Build changes still look good to me.

/Erik


On 2018-03-28 03:31, Magnus Ihse Bursie wrote:

On 2018-03-28 01:52, Weijun Wang wrote:


On Mar 24, 2018, at 6:03 AM, Magnus Ihse Bursie 
 wrote:


https://bugs.openjdk.java.net/browse/JDK-8200193 -- for 
jdk.security.auth
There is only one function to export and it already has JNIEXPORT, so 
you can just remove the new $(LIBJAAS_CFLAGS) [1].

Ok, thanks Max!

Are you going to update your webrev?
Here is a new webrev. It includes your recommended change in 
Lib-jdk.security.auth.gmk.


It is also updated to keep track of changes in shared native libraries 
that has happend in the mainline since my first webrev. Most notably 
is the addition of libjsig. For now, I have just added the JNIEXPORT 
markers for the platforms that need it. Hopefully we can unify libjsig 
across all platforms, but that seems to be more complicated than I 
thought, so that'll have to wait.


I have also recieved word from Phil Race that there were no testing 
issues for client, so he's happy as well.


Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.03


/Magnus



Thanks
Max

[1] 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01/make/lib/Lib-jdk.security.auth.gmk.sdiff.html

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Erik Joelsson]

I have looked at the build changes and they look good.

Will you file followups for each component team to look over their 
exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? 
It sure looks like there is some technical debt laying around here.


/Erik


On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort to 
using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility on 
Windows, for most of the shared code, we already have proper JNIEXPORT 
decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully achieve. 
I do not believe that these changes will have any actual impact on the 
product, though. I will present the differences more in detail further 
down. Those who are not interested can probably skip that.


The patch has passed tier1 testing and is currently running tier2 and 
tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and 
java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function ever 
be used on Windows in the future). I have also followed the stylistic 
convention of putting "JNIEXPORT  JNICALL" on a separate 
line. For some functions, however, this might cause a change in 
calling convention on Windows. Since this can not apply to exported 
functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. This 
actually meant that all symbols were exported. It is highly unclear if 
this was known and intended by the original make rule writer. I have 
emulated this by adding the flag $(EXPORT_ALL_SYMBOLS) to these 
libraries. Hopefully, we can remove this flag and fix proper exported 
symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a 
thourough analysis of the differences for Linux and Solaris. All 
native libraries have symbol differences, but most of them are trivial 
and/or harmless. As a result, most libraries have disasm differences 
as well, but these too seem trivial and harmless. The differences in 
symbols that are common to all libraries include:
 * Internal symbols such as __bss_start, _edata, _end and _fini are 
now global. (They are imported as such from the compiler 
libraries/archives, and we have no linker script to override this 
behavior).
 * The versioning tag SUNWprivate_1.1 is not included, and thus 
neither the .gnu.version_d symbol.
 * There are a few differences in the symbol and/or mangling of some 
local functions. I'm not sure what's causing this,

but it's unlikely to have any effect on the product.

Another common source for change in symbols is due to previous 
platform differences. For instance, if we had "JNIEXPORT int JNICALL 
do_foo() { ... }", but do_foo was not in the mapfile, the symbol was 
exported on Windows but not on Linux and Solaris. (Presumable since it 
was not needed there, even though it was compiled for those platforms 
as well.) Now, with the mapfiles gone, do_foo() will be exported on 
all platforms. And contrary, functions that are compiled on all 
platforms, and were exported in mapfiles, but now have gotten an 
JNIEXPORT 

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Erik Joelsson]

Build changes look good.

/Erik


On 2018-03-21 07:09, Magnus Ihse Bursie wrote:

On 2018-03-16 17:49, Alex Menkov wrote:



On 03/15/2018 13:09, Magnus Ihse Bursie wrote:



15 mars 2018 kl. 20:13 skrev Phil Race <philip.r...@oracle.com>:


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff


Yes, I think it is something like that for Linux
ie if at runtime a dependent-but-not-essential .so was not
installed it was not fatal. I don't know to what extent this is no 
longer a

possible issue, or one that matters.


I have not heard of any mainstream Linux distro in years that was 
lacking ALSA.


If ALSA was not present, will the libraries fall back to OSS, or 
will there be just no sound available?


No sound.
OSS support was dropped many years ago (IIRC in jdk7)

In any case, I think that whatever Linux distros we're targeting as 
supported, ALSA will be present.


Alex, did I understand you correctly that in any case, a separate 
Windows library is always unnecessary, since we can rely on 
DirectAudio always being present in our supported versions of Windows?


Yes, that's right.
Windows always has DirectSound pre-installed and its version is 
greater than required (IIRC javasoundds requires DirectX 5).


For now failure of libjsound loading is fatal (see 
com.sun.media.sound.Platform.loadLibraries()), loading of extra libs 
is non-fatal.
I believe libjsound loading failure should be made non-fatal, then 
all the functionality will remain the same as we have now.


Ok.

Here is an updated webrev. I have made the following changes:
* libjavasoundalsa and libjavasoundds is now folded into the main 
libjavasound native library, so there's exactly one library built on 
all platforms.

* Loading of libjsound is made non-fatal.
* I have cleaned out all obvious parts of the code that handle 
multiple libraries. Since loading the native library is now a 
all-or-nothing situation, the checks for various subsystems have been 
turned into a generic check if the native library is loaded.


There is a lot of defines like USE_DSOUND which are always true. This 
could probably be cleaned up further, but it is not a build issue so 
I'm leaving that to the client team to handle.


I have tested the following:
 * COMPARE_BUILD shows me just the expected changes in the build.
 * On my linux machine, failure to load libjsound.so was not fatal.
 * I have looked for sound tests. I found the test/jdk/javax/sound 
suite, which was included in tier3. So I've run tier3 testing on all 
platforms using our internal test system, and all tests pass.


I don't know if there is any other tests I should run. If so, let me 
know.


Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.03


/Magnus



--alex



/Magnus



-phil.


On 03/15/2018 12:06 PM, Alex Menkov wrote:



On 03/15/2018 11:44, Magnus Ihse Bursie wrote:

On 2018-03-15 18:23, Phil Race wrote:
I wondered if that might be the case since it was a "BSD" port 
.. using X11 ..


Maybe we should be getting rid of them ?
I agree, we should delete them. I just shuffled them around in 
the hope that they would be useful for a potential future bsd 
port, but if/when that happens, we can dig them out from mercurial.


A short explanation of how the files moved. The sound library is 
apparently composed of either a single library (solaris and 
macosx) or two libraries (linux and windows). Two building 
blocks, MIDI + ports and DirectAudio is used for all platforms, 
but they go into either the main library (libjsound) or the 
helper library.


For Windows, MIDI+Ports go into libjsound, and DirectAudio go 
into libjsoundds. On Linux, MIDI+Ports and DirectAudio go into 
libjsoundalsa. On Macosx and Solaris, MIDI+Ports and DirectAudio 
go into the main libjsound.


I have no idea why this split is necessary, but this is how the 
libraries de facto is compiled, and the code needs to match that. 
If it would be possible to move libjsoundds and libjsoundalsa 
into libjsound directly, things would be greatly simplified.


As far as I know the split was made to dynamically load 
ALSA/DirectSound stuff. If it's not available (or old unsupported 
version is installed), libjsound stuff continues to work (in 
pre-OpenJDK libjsound supported WaveIn/WaveOut on Windows and OSS 
on Linux).
For now Windows (DirectSound) libjsoundds stuff can be merged into 
libjsound, but I'm not sure we can rely on ALSA is always 
available on Linux (but most likely if ALSA is not available, 
libjsound does not provide any functionality, so I suppose 
libjsoundalsa stuff can be moved to libjsound as well)


--alex



/Magnus



-phil.


On 03/15/2018 10:21 AM, Erik Joelsson wrote:
Digging a bit, those files came with the initial Macosx 
support. It doesn't look like they were ever used.


/Erik



On 2018-03-15 09:53, Phil Race wrote:
It is very hard to follow all the moved around files, but one 
thing

that sticks out is there is a

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Erik Joelsson]

Digging a bit, those files came with the initial Macosx support. It 
doesn't look like they were ever used.


/Erik


On 2018-03-15 09:53, Phil Race wrote:

It is very hard to follow all the moved around files, but one thing
that sticks out is there is a "bsd" directory created and I can't
work out how the files in there are used.
If they are for a BSD port of OpenJDK where is rest of the support for 
that ?


On 03/15/2018 07:20 AM, Erik Joelsson wrote:
Looks good to me. I tried cleaning this up before but failed to find 
a reasonable split, but this seems like a good split between common 
and library specific.


/Erik


On 2018-03-14 18:12, Magnus Ihse Bursie wrote:
I forgot to add the client mailing lists as recipients. Sorry. (Not 
sure if "sounds" belong to "awt" or "2d".)


In fact, there is a sound-specific list, which I've added.

-phil.


/Magnus

On 2018-03-15 02:07, Magnus Ihse Bursie wrote:

From the bug description:

Moving this to a separate bug from JDK-8055190. In 
SoundLibraries.gmk, the source code splitting is not complete. The 
directory libjsound is used to build not only libjsound but 
libjsoundalsa and libjsoundds, and thus needs a complex 
include/exclude system like before.


I have tested this using COMPARE_BUILD. Windows and solaris are 
completely clean. On macosx, there's a binary diff (but nothing 
else) on libjsound.dylib. On linux, some offset seems to have 
changed, which caused a slight change in disass and fulldump for 
libjsound.so. I'm not quite sure what's causing it, but I'm 
convinced it's harmless.


Bug: https://bugs.openjdk.java.net/browse/JDK-8071469
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.01


/Magnus

Re: RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Erik Joelsson]

Looks good to me. I tried cleaning this up before but failed to find a 
reasonable split, but this seems like a good split between common and 
library specific.


/Erik


On 2018-03-14 18:12, Magnus Ihse Bursie wrote:
I forgot to add the client mailing lists as recipients. Sorry. (Not 
sure if "sounds" belong to "awt" or "2d".)


/Magnus

On 2018-03-15 02:07, Magnus Ihse Bursie wrote:

From the bug description:

Moving this to a separate bug from JDK-8055190. In 
SoundLibraries.gmk, the source code splitting is not complete. The 
directory libjsound is used to build not only libjsound but 
libjsoundalsa and libjsoundds, and thus needs a complex 
include/exclude system like before.


I have tested this using COMPARE_BUILD. Windows and solaris are 
completely clean. On macosx, there's a binary diff (but nothing else) 
on libjsound.dylib. On linux, some offset seems to have changed, 
which caused a slight change in disass and fulldump for libjsound.so. 
I'm not quite sure what's causing it, but I'm convinced it's harmless.


Bug: https://bugs.openjdk.java.net/browse/JDK-8071469
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.01


/Magnus

Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers

[Erik Joelsson]

Adding 2d-dev in the hopes of getting some input from component team. 
Seems like they should be aware of us removing the support for multiple 
data models.


Looks like you left a debug message at line 40 in GensrcX11Wrappers.gmk.

/Erik

On 2018-03-02 03:00, Magnus Ihse Bursie wrote:

On 2018-03-02 00:02, Erik Joelsson wrote:

Hello,

In xlibtypes.txt comment, should it be sizes-64.txt?


Yes, good catch.



Generating both 32 and 64 seems a bit outdated at this point. Surely 
this is a remnant of bundling 64 and 32 bit together on Solaris in 
the past? Perhaps someone in 2d can answer this? Would be nice to be 
able to clean up that part as well if possible.
Yes, you are right. We should clean up that as well. I was just too 
conservative. I've now actually checked what happens when you generate 
both 32 and 64 bit versions, and the result is that instead of code like:

    public static int getSize() { return 96; }
we get code like this:
    public static int getSize() { return ((XlibWrapper.dataModel == 
32)?(80):(96)); }


Since we do no longer support multiple data models for the same build, 
this is just unnecessary. In fact, that leads to an even better 
cleanup, since we will always need just a single input file.


I also wrapped the tool calls in ExecuteWithLog.

Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.02 



/Magnus

Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers

[Erik Joelsson]

Hello,

In xlibtypes.txt comment, should it be sizes-64.txt?

Generating both 32 and 64 seems a bit outdated at this point. Surely 
this is a remnant of bundling 64 and 32 bit together on Solaris in the 
past? Perhaps someone in 2d can answer this? Would be nice to be able to 
clean up that part as well if possible.


Otherwise good.

/Erik

On 2018-02-28 12:12, Magnus Ihse Bursie wrote:
My hunt on technical debt continues. This time my aim has been on the 
sorry mess known as GensrcX11Wrappers.


I've disentangled it into two functions, one simple gensrc part that 
is actually run during the build, and which involves just a simple 
java tool and some pre-calculated data files, and a separate step for 
updating those pre-calculated data files. This step is now run using 
"make update-x11wrappers". It involves compiling a native binary, and 
running it on the target platform, so just as before, this assumes 
that you are not cross-compiling.


I'm not sure what role the "verification" step we had before ever 
played. For all the years we've been "verifying" this, we've detected 
no differences. In fact, as far as I understand, what we *really* 
would need to verify against, is the X11 libraries on the runtime 
system, i.e. where the users executes the JRE. But then again, we can 
assume that this matches, just as anyone compiling with header files 
on one place can assume that they can use the libraries elsewhere. The 
only reason, as I see it, to keep the generation in the makefiles at 
all, is to document how the files were generated, and to be ready for 
the need to update the files if the list of datatypes in xlibtypes.txt 
changes.


Bug: https://bugs.openjdk.java.net/browse/JDK-8198844
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.01


/Magnus

Re: RFR: 8196509: Linux UI applications broken by the build change for JDK-8196218

[Erik Joelsson]

Looks good.

/Erik


On 2018-01-31 09:11, Phil Race wrote:

Adding build-dev and 2d-dev.

-phil.

On 01/31/2018 09:10 AM, Phil Race wrote:

All UI apps are broken on Linux :

Bug : https://bugs.openjdk.java.net/browse/JDK-8196509
Webrev: http://cr.openjdk.java.net/~prr/8196509/

The fix (for now) is a straightforward reversion of the change here :
http://hg.openjdk.java.net/jdk/client/rev/2f4fe7776a53

-phil

Re: Fix for JDK-8160146 : Resolve disabled GCC warning 'deprecated-declarations' for libawt_xawt

[Erik Joelsson]

Looks good.

/Erik


On 2016-10-27 11:02, Ajit Ghaisas wrote:

Hi,

  


 Fix of HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8165232"JDK-8165232 has fixed the 
'deprecated' warnings from libawt_xawt.

 Now, only removing warning suppression from makefile remains. This webrev 
does it.

  


 http://cr.openjdk.java.net/~aghaisas/8160146/webrev.0/

  


 I have run JPRT and builds are successful.

  


 Request you to review.

  


Regards,

Ajit

  

Re: Review Request: 8156960 Deprecate JSObject.getWindow(Applet) method

[Erik Joelsson]

Looks good.

/Erik

On 2016-06-09 19:58, Daniil Titov wrote:

Thank you, Mandy!

The long line is corrected.

Webrev: http://cr.openjdk.java.net/~dtitov/8156960/jdk/webrev.03
 http://cr.openjdk.java.net/~dtitov/8156960/webrev.03

Bug: https://bugs.openjdk.java.net/browse/JDK-8156960

Best regards,
Daniil




-Original Message-
From: Mandy Chung
Sent: Thursday, June 09, 2016 9:23 AM
To: Daniil Titov
Cc: Erik Joelsson; David Dehaven; Stuart Marks; build-dev; 
build-infa-...@openjdk.java.net; awt-dev; Kevin Rushforth
Subject: Re: Review Request: 8156960 Deprecate JSObject.getWindow(Applet) method



On Jun 9, 2016, at 9:08 AM, Daniil Titov <daniil.x.ti...@oracle.com> wrote:

Thank you, Erik!

Please review the new version of the patch that has "../" fixed:

Webrev: http://cr.openjdk.java.net/~dtitov/8156960/jdk/webrev.02
http://cr.openjdk.java.net/~dtitov/8156960/webrev.02/


Looks okay.  Nit: in JSObject.java - line 154 long line to be wrapped.

Mandy

Re: Review Request: 8156960 Deprecate JSObject.getWindow(Applet) method

[Erik Joelsson]

Hello,

Generally looks good, but I do think this needs to be changed:

JSOBJECT_DOCDIR := $(JRE_API_DOCSDIR)/plugin/jsobject
JSOBJECT2COREAPI := ../../../$(JDKJRE2COREAPI)

The number of ../ should probably match the directory level depth of the 
variable before it, at least it looks that way for the other setups:


JSOBJECT2COREAPI := ../../$(JDKJRE2COREAPI)

/Erik

On 2016-06-09 02:34, Daniil Titov wrote:

Hello,

Please review the new version of the fix for JDK9.

1. "forRemoval = true" is added to @Deprecated annotation for 
JSObject.getWindow(Applet) method.
2.  A new doc bundle for JSObject documentation is added in the docs build.

Webrev: http://cr.openjdk.java.net/~dtitov/8156960/jdk/webrev.01
 http://cr.openjdk.java.net/~dtitov/8156960/webrev.01


Bug: https://bugs.openjdk.java.net/browse/JDK-8156960

Thank you!

Best regards,
Daniil

-Original Message-
From: Mandy Chung
Sent: Wednesday, June 08, 2016 3:09 PM
To: Daniil Titov
Cc: David Dehaven; Stuart Marks; Erik Joelsson; build-dev; 
build-infa-...@openjdk.java.net; awt-dev; Kevin Rushforth
Subject: Re: Review Request: 8156960 Deprecate JSObject.getWindow(Applet) method

That’s right.  It requires to add a new doc bundle in the docs build.  What you 
did was the right direction.  Can you update the webrev?

FYI.  There is an effort under discussion to revisit the number of docs bundle 
generated and clean up the docs build.

Mandy


On Jun 8, 2016, at 2:48 PM, Daniil Titov <daniil.x.ti...@oracle.com> wrote:

NON_CORE_PKGS variable is not used in make/Javadoc.gmk, so just adding a new 
package in this variable will not make this package included in any docs. We 
will need to create a new javadoc target for JSObject documentation ( or add it 
to some existing target, but it doesn't look like there is one that fits it). 
For example:


diff -r 389c2d2842a5 make/Javadoc.gmk
--- a/make/Javadoc.gmk  Wed May 25 12:53:26 2016 +0200
+++ b/make/Javadoc.gmk  Thu Jun 02 16:20:35 2016 -0700
@@ -82,7 +82,7 @@
PLUGIN2_FIRST_COPYRIGHT_YEAR = 2007
JDKNET_FIRST_COPYRIGHT_YEAR = 2014
JACCESSAPI_FIRST_COPYRIGHT_YEAR = 2002
-
+JSOBJECT_FIRST_COPYRIGHT_YEAR = 1993

# Oracle name
FULL_COMPANY_NAME = Oracle and/or its affiliates @@ -1031,6 +1031,64
@@

#
#
+# jsobjectdocs
+#
+
+ALL_OTHER_TARGETS += jsobjectdoc
+
+JSOBJECT_DOCDIR := $(JRE_API_DOCSDIR)/plugin/jsobject
+JSOBJECT2COREAPI := ../../../$(JDKJRE2COREAPI) JSOBJECT_DOCTITLE :=
+Java$(TRADEMARK) JSObject Doc JSOBJECT_WINDOWTITLE := Java JSObect
+Doc JSOBJECT_HEADER := Java JSObject Doc
+JSOBJECT_BOTTOM := $(call
+CommonBottom,$(JSOBJECT_FIRST_COPYRIGHT_YEAR))
+# JSOBJECT_PKGS is located in NON_CORE_PKGS.gmk
+
+JSOBJECT_INDEX_HTML = $(JSOBJECT_DOCDIR)/index.html
+JSOBJECT_OPTIONS_FILE = $(DOCSTMPDIR)/jsobject.options
+JSOBJECT_PACKAGES_FILE = $(DOCSTMPDIR)/jsobject.packages
+
+# The modules required to be documented JSOBJECT_MODULES =
+jdk.jsobject
+
+jsobjectdocs: $(JSOBJECT_INDEX_HTML)
+
+# Set relative location to core api document root
+$(JSOBJECT_INDEX_HTML): GET2DOCSDIR=$(JSOBJECT2COREAPI)/..
+
+# Run javadoc if the index file is out of date or missing
+$(JSOBJECT_INDEX_HTML): $(JSOBJECT_OPTIONS_FILE) $(JSOBJECT_PACKAGES_FILE) 
$(COREAPI_INDEX_FILE)
+   $(prep-javadoc)
+   $(call 
JavadocSummary,$(JSOBJECT_OPTIONS_FILE),$(JSOBJECT_PACKAGES_FILE))
+   $(JAVADOC_CMD_SMALL) -d $(@D) \
+   @$(JSOBJECT_OPTIONS_FILE) @$(JSOBJECT_PACKAGES_FILE)
+
+# Create file with javadoc options in it
+$(JSOBJECT_OPTIONS_FILE):
+   $(prep-target)
+   @($(call COMMON_JAVADOCFLAGS) ; \
+  $(call COMMON_JAVADOCTAGS) ; \
+ $(call OptionOnly,-Xdoclint:none) ; \
+  $(call OptionPair,-system,none) ; \
+ $(call OptionPair,-modulesourcepath,$(RELEASEDOCS_MODULESOURCEPATH)) 
; \
+ $(call OptionPair,-addmods,$(JSOBJECT_MODULES)) ; \
+ $(call OptionPair,-encoding,ascii) ; \
+ $(call OptionOnly,-nodeprecatedlist) ; \
+ $(call OptionPair,-doctitle,$(JSOBJECT_DOCTITLE)) ; \
+ $(call OptionPair,-windowtitle,$(JSOBJECT_WINDOWTITLE) 
$(DRAFT_WINTITLE)); \
+ $(call OptionPair,-header,$(JSOBJECT_HEADER)$(DRAFT_HEADER)); \
+ $(call OptionPair,-bottom,$(JSOBJECT_BOTTOM)$(DRAFT_BOTTOM)); \
+ $(call 
OptionTrip,-linkoffline,$(JSOBJECT2COREAPI),$(COREAPI_DOCSDIR)/); \
+   ) >> $@
+
+# Create a file with the package names in it
+$(JSOBJECT_PACKAGES_FILE): $(call PackageDependencies,$(JSOBJECT_PKGS))
+   $(prep-target)
+   $(call PackageFilter,$(JSOBJECT_PKGS))
+
+
+#
+#
# mgmtdocs
#


Best regards,
Daniil



-Original Message-
From: David DeHaven
Sent: Wednesday, June 08, 2016 1:23 PM
To: Mandy Chung
Cc: Daniil Titov; Stuart Marks; Erik Joelsson; build-dev;
build-infa-...@openjdk.java.net; awt-dev; Kevin Rushforth
Subject: Re: Review Requ

Re: Fix for JDK-8074829 : Resolve disabled warnings for libawt_headless

[Erik Joelsson]

I believe at least one of the disabled warnings was specific to building 
on arm or arm64. You will need to verify those platforms as well before 
removing the gcc disabled warnings.


/Erik

On 2016-04-20 15:04, Erik Joelsson wrote:

Build change looks good.

/Erik

On 2016-04-20 13:30, Ajit Ghaisas wrote:

Adding build-dev.

-Original Message-
From: Ajit Ghaisas
Sent: Wednesday, April 20, 2016 2:28 PM
To: awt-dev@openjdk.java.net
Subject:  Fix for JDK-8074829 : Resolve disabled warnings 
for libawt_headless


Hi,

 Bug : https://bugs.openjdk.java.net/browse/JDK-8074829
 This bug is to remove warning suppressions from makefile and fix 
the warnings for libawt_headless library.


 I have removed following warning suppressions & fixed the 
warnings for libawt_headless library.

 DISABLED_WARNINGS_gcc := maybe-uninitialized int-to-pointer-cast
 DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE

 Warning suppression that cannot be removed :
 DISABLED_WARNINGS_solstudio := E_EMPTY_TRANSLATION_UNIT
 This is due to the fact that - 
jdk/src/java.desktop/share/native/common/java2d/opengl/OGLBlitLoops.c 
file becomes empty file in case of HEADLESS mode compilation.



 I have run the JPRT build on this fix. It built successfully on 
all platforms - please see :

http://scaaa637.us.oracle.com/archive/2016/04/2016-04-20-071524.ajit.client/JobStatus.txt
 Also, I have built it on Windows 7 with Visual Studio 10.

 Request you to review following webrev :
 http://cr.openjdk.java.net/~aghaisas/8074829/webrev.00/

Regards,
Ajit

Re: Fix for JDK-8074829 : Resolve disabled warnings for libawt_headless

[Erik Joelsson]

Build change looks good.

/Erik

On 2016-04-20 13:30, Ajit Ghaisas wrote:

Adding build-dev.

-Original Message-
From: Ajit Ghaisas
Sent: Wednesday, April 20, 2016 2:28 PM
To: awt-dev@openjdk.java.net
Subject:  Fix for JDK-8074829 : Resolve disabled warnings for 
libawt_headless

Hi,

 Bug : https://bugs.openjdk.java.net/browse/JDK-8074829
 This bug is to remove warning suppressions from makefile and fix the 
warnings for libawt_headless library.

 I have removed following warning suppressions & fixed the warnings for 
libawt_headless library.
 DISABLED_WARNINGS_gcc := maybe-uninitialized int-to-pointer-cast
 DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE

 Warning suppression that cannot be removed :
 DISABLED_WARNINGS_solstudio := E_EMPTY_TRANSLATION_UNIT
 This is due to the fact that - 
jdk/src/java.desktop/share/native/common/java2d/opengl/OGLBlitLoops.c file 
becomes empty file in case of HEADLESS mode compilation.


 I have run the JPRT build on this fix. It built successfully on all 
platforms - please see :
 
http://scaaa637.us.oracle.com/archive/2016/04/2016-04-20-071524.ajit.client/JobStatus.txt
 Also, I have built it on Windows 7 with Visual Studio 10.

 Request you to review following webrev :
 http://cr.openjdk.java.net/~aghaisas/8074829/webrev.00/

Regards,
Ajit

Re: [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

[Erik Joelsson]

Build changes are still good.

/Erik

On 2016-03-23 11:46, Alexander Scherbatiy wrote:

On 23/03/16 13:56, Rajeev Chamyal wrote:

Hello Alexandr,

I have updated the webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.06/


  The fix looks good to me.

  Thanks,
  Alexandr.


Regards,
Rajeev Chamyal

-Original Message-
From: Alexandr Scherbatiy
Sent: 22 March 2016 20:15
To: Rajeev Chamyal; Sergey Bylokhov; awt-dev@openjdk.java.net; 
build-...@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8145173 HiDPI 
splash screen support on Windows


On 3/22/2016 12:43 AM, Rajeev Chamyal wrote:

Hello All,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.05/

With VC2010  java.c ::ShowSplashScreen  fails with segmentation 
fault on  calling free on scaled_splash_name .
This failure is due to different C runtime used by libjli and lib 
splashScreen.

Fix: Allocating a max scaled image name length buffer in java.c and
passing it to splashscreen_sys.c

 Just few small comments:
 - SplashGetScaledImageName can return a boolean value which 
indicates that high-resolution splash screen has been found
 - The getMaxScaledImageNamePostfixLen function can be added 
which just returns max length of the scaled image postfix.
   It allows to control this value on the java.desktop side 
rather to keep it in java.c file

 - systemScale.cpp copyright need to be updated to 2016

Thanks,
Alexandr.

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal
Sent: 15 March 2016 18:26
To: Alexander Scherbatiy; Sergey Bylokhov; awt-dev@openjdk.java.net;
build-...@openjdk.java.net
Subject: RE:  [9] Review request for JDK-8145173 HiDPI splash
screen support on Windows

Hello All,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.04/

Alexandr : I have build code with VS2013 and I didn't get any errors 
you mentioned.

Still I have updated the code as suggested.

Added build team for make file review. JPRT build with fix was 
successful.


Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 14 March 2016 17:28
To: Rajeev Chamyal
Cc: Sergey Bylokhov; awt-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8145173 HiDPI splash
screen support on Windows

On 3/14/2016 8:03 AM, Rajeev Chamyal wrote:

Hello Sergey,

Could you please review the enhancement.

I have raised a new enhancement for unifying the splash screen image
names across platforms.

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


575 float *scaleFactor)
576 {
577 *scaleFactor = 1.0;
578 float dpiScaleX = -1.0f;
579 float dpiScaleY = -1.0f;
580 GetScreenDpi(getPrimaryMonitor(), , );

I have errors building the fix with VS2010:

* For target
support_native_java.desktop_libsplashscreen_splashscreen_sys.obj:
splashscreen_sys.c
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c
(578)
: error C2143: syntax error : missing ';' before 'type'
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c
(579)
: error C2143: syntax error : missing ';' before 'type'


You need to move variables declaration to the beginning of a statement.

Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 10 March 2016 18:46
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
splash screen support on Windows

The fix looks good to me.

Thanks,
Alexandr.

On 3/10/2016 3:05 AM, Rajeev Chamyal wrote:

  Hello Alexandr,

  Thanks for the review. Below is the updated webrev as per review
  comments.

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.03/


  Regards,

  Rajeev Chamyal

  *From:*Alexandr Scherbatiy
  *Sent:* 10 March 2016 11:39
  *To:* Rajeev Chamyal; awt-dev@openjdk.java.net
  ; Sergey Bylokhov
  *Subject:* Re:  [9] Review request for JDK-8145173 
HiDPI

  splash screen support on Windows

  On 3/2/2016 9:50 PM, Rajeev Chamyal wrote:


  Hello All,

  Please review the updated webrev.

  Added a free call for duplicate file name in
  splashscreen_sys.c :: SplashGetScaledImageName

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.02/



 awt_Win32GraphicsDevice.cpp
   656 dpiX = GetScreenDpi(GetMonitor());
   657 if (dpiX > 0) {
   658 dpiX = dpiX >= 96 ? dpiX / 96 : dpiX;
   659 SetScale(dpiX, dpiX);

  The Windows HiDPI graphics support was designed to handle 
both DPI

  X and Y scales. The GetScreenDpi should return both values to be
  used in SetScale method.

 

Re: [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

[Erik Joelsson]

Build changes look ok.

/Erik

On 2016-03-15 15:08, Alexander Scherbatiy wrote:

On 15/03/16 16:56, Rajeev Chamyal wrote:

Hello All,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.04/

Alexandr : I have build code with VS2013 and I didn't get any errors 
you mentioned.

Still I have updated the code as suggested.


  I would suggest to build the code with VS 2010 because it still 
complains on declaration like:

   619 free(dupFileName);
   620 FILE *fp = NULL;

 Thanks,
 Alexandr.


Added build team for make file review. JPRT build with fix was 
successful.


Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 14 March 2016 17:28
To: Rajeev Chamyal
Cc: Sergey Bylokhov; awt-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8145173 HiDPI 
splash screen support on Windows


On 3/14/2016 8:03 AM, Rajeev Chamyal wrote:

Hello Sergey,

Could you please review the enhancement.

I have raised a new enhancement for unifying the splash screen image
names across platforms.

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


   575 float *scaleFactor)
   576 {
   577 *scaleFactor = 1.0;
   578 float dpiScaleX = -1.0f;
   579 float dpiScaleY = -1.0f;
   580 GetScreenDpi(getPrimaryMonitor(), , );

I have errors building the fix with VS2010:

* For target
support_native_java.desktop_libsplashscreen_splashscreen_sys.obj:
splashscreen_sys.c
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(578) 


: error C2143: syntax error : missing ';' before 'type'
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(579) 


: error C2143: syntax error : missing ';' before 'type'


You need to move variables declaration to the beginning of a statement.

Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 10 March 2016 18:46
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
splash screen support on Windows

The fix looks good to me.

Thanks,
Alexandr.

On 3/10/2016 3:05 AM, Rajeev Chamyal wrote:

 Hello Alexandr,

 Thanks for the review. Below is the updated webrev as per review
 comments.

 http://cr.openjdk.java.net/~rchamyal/8145173/webrev.03/


 Regards,

 Rajeev Chamyal

 *From:*Alexandr Scherbatiy
 *Sent:* 10 March 2016 11:39
 *To:* Rajeev Chamyal; awt-dev@openjdk.java.net
 ; Sergey Bylokhov
 *Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
 splash screen support on Windows

 On 3/2/2016 9:50 PM, Rajeev Chamyal wrote:


 Hello All,

 Please review the updated webrev.

 Added a free call for duplicate file name in
 splashscreen_sys.c :: SplashGetScaledImageName

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.02/



awt_Win32GraphicsDevice.cpp
  656 dpiX = GetScreenDpi(GetMonitor());
  657 if (dpiX > 0) {
  658 dpiX = dpiX >= 96 ? dpiX / 96 : dpiX;
  659 SetScale(dpiX, dpiX);

 The Windows HiDPI graphics support was designed to handle both DPI
 X and Y scales. The GetScreenDpi should return both values to be
 used in SetScale method.

 systemScale.cpp

38 float scale = -2.0f;

39 if (scale == -2) {

 Initially the scale variable was defined as static to avoid 
rereading the J2D_UISCALE test variable each time.


 It is better to preserve the "// for debug purposes" comment also.


 MultiResolutionSplashTest.java

 +   scaleFactor = (float)((SunGraphics2D) 
g).surfaceData.getDefaultScaleX();



 Now it is possible to get the the scale factor using
GraphicsEnvironment.getLocalGraphicsEnvironment().getDefaultScreenDevi
ce().getDefaultConfiguration().getDefaultTransform().getScaleX()


 Thanks,
  Alexandr.



 Regards,

 Rajeev Chamyal

 *From:*Rajeev Chamyal
 *Sent:* 01 March 2016 15:45
 *To:* awt-dev@openjdk.java.net
 ; Sergey Bylokhov; Alexander
 Scherbatiy
 *Subject:* RE:  [9] Review request for JDK-8145173
 HiDPI splash screen support on Windows

 Hello All,

 Gentle reminder.

 Please review the updated webrev.


http://cr.openjdk.java.net/~rchamyal/8145173/webrev.01/


 Regards,

 Rajeev Chamyal

 *From:*Rajeev Chamyal
 *Sent:* 16 February 2016 16:01
 *To:* awt-dev@openjdk.java.net
 ; Sergey Bylokhov; Alexander
 Scherbatiy
 *Subject:*  [9] Review request for JDK-8145173 HiDPI

RFR: JDK-8151770: 9-client windows builds failed from 2016-03-11

[Erik Joelsson]

Hello,

The build is currently broken on Windows in jdk9/client. This patch 
reverts some and cleans up the makefile changes made in JDK-8145174. I 
have verified that it builds on Windows and Linux and am just starting a 
JPRT job to verify the rest.


Looking through the review thread for JDK-8145174, it seems like in the 
last version, the makefile was changed to support a windows version of 
the patch without any sources being added for Windows. The bug clearly 
states that it only applies to Linux. I removed those Windows specific 
changes and cleaned up the the rest with correct indentation, removing 
trailing whitespace and moving definition the proper place.


My work day ends now. Since this fix is quite urgent, feel free to push 
it for me to jdk9/client when it's properly reviewed, or use it as base 
for further fixes.


Bug: https://bugs.openjdk.java.net/browse/JDK-8151770
Webrev: http://cr.openjdk.java.net/~erikj/8151770/webrev.01/

/Erik

Re: RFR: JDK-8145596 Enable debug symbols for all libraries

[Erik Joelsson]

Looks good to me.

Note that this patch will conflict with the quick fix I did in jdk9/hs, 
so you will need to either wait for my patch, push in a forest that has 
my patch, or make sure the integrator can handle it correctly.


/Erik

On 2015-12-17 14:43, Magnus Ihse Bursie wrote:
For historical reasons, the FDS (Full Debug Symbols) project only 
enabled debug symbols for a few select libraries, since this was 
difficult to achieve in the old build. Also, the FDS project never 
enabled debug symbols for macosx on the JDK libraries.


With the new build system, debug symbols come for free for all 
libraries, and we actually have to do extra work to keep them out. :-&


We should just stop doing that. It hurts no-one to have proper debug 
information for all libraries, it might come in helpful, and just 
doing it everywhere would simplify build logic.


This is mainly a build change, but I'm cc:ing the component teams just 
in case.


This patch leverages JDK-8036003, but provides a cleaner 
implementation of this logic in the makefiles. Instead of the vague 
ENABLE_DEBUG_SYMBOLS, I have introduced three clearly defined variables:


COMPILE_WITH_DEBUG_SYMBOLS
COPY_DEBUG_SYMBOLS
ZIP_EXTERNAL_DEBUG_SYMBOLS

The various settings of --with-native-debug-symbols turns these 
variables on/off depending on what we want to achieve, and the 
makefiles check these variables to determine what compiler flags to 
use, whether to run objcopy (or similar) and whether to zip the 
extracted symbols, respectively.


A fourth variable (STRIP_DEBUG_SYMBOLS) is needed to fully complement 
this, but the way we handle stripping is complex in it's own right, 
and I've saved that for a separate patch.


Note that this patch intentionally does not affect the Hotspot build 
system. The variables for the hotspot build is kept unchanged. When 
the new build-infra based hotspot build system arrives, the 
functionality introduced in this patch will be automatically used. 
Until then, I prefer not to mess any more than necessary with the 
hotspot makefiles.


Bug: https://bugs.openjdk.java.net/browse/JDK-8145596
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8145596-fix-native-debug-symbols-properly/webrev.01


/Magnus

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Erik Joelsson]

The configure change looks good. Can't comment on the code changes.

/Erik

On 2015-12-16 14:50, Magnus Ihse Bursie wrote:

There is an interest from the community to build OpenJDK using Visual Studio 
2015 Community edition.

This patch is provided by Timo Kinnunen . I am 
sponsoring this patch.

The changes to the source code files are mostly casts to uintptr_t, but there 
are some other changes as well.

I'm not quite sure who's the owner of all these files. If I'm missing some 
group, please help me and forward the mail to them.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01

/Magnus

Re: AWT Dev RfR JDK-8077296 RE build fails on non-Win builds when attempting to build Win only javadoc

[Erik Joelsson]

Hello Pete,

This looks good to me.

/Erik

On 2015-05-21 07:33, Pete Brunet wrote:

Please review the following change for 8u:
http://cr.openjdk.java.net/~ptbrunet/JDK-8077296/webrev.00/

Background:

- As part of the open sourcing of the JAB and Java Accessibility
Utilities (JAU) the JAU Javadoc was setup to be added to the build.
- Due to a 8u build issue (it uses source bundles) the Javadoc had to be
temporarily disabled: JDK-8076552
- The issue was caused by JAU code being in windows instead of share.
   - For 9 the JAU code is in
jdk.accessibility/share/classes/com/sun/java/accessibility/util (and the
JAB code under jdk.accessibility/windows).
   - For 8u both the JAU and JAB code is in jdk/src/windows.
- The problem on 8u is with non-windows builds; the 8 Javadoc build
process doesn't find the JAU files because they are in a windows directory.
- The fix is to refactor so 8u is like 9 with the JAU files in share;
then the javadoc build process will find the source for all build platforms.
- Make has been changed so jaccess.jar is built for all platforms
- Some obsolete files have been removed as they were causing build
problems.  They were removed from 9 earlier but not from 8u.  These are
com/sun/java/accessibility/util/java/awt/ChoiceTranslator.java and all
the files in com/sun/java/accessibility/extensions.

Thanks, Pete

Re: AWT Dev RFR: JDK-8074859 Turn on warnings as error

[Erik Joelsson]

Looks good to me.

/Erik

On 2015-04-17 14:52, Magnus Ihse Bursie wrote:
With JDK-8074096, the number of warnings in the product was reduced to 
a minimum. This enables the next step, which is turning on the 
respective compiler flags that turns warnings into errors. In the long 
run, this is the only way to keep the warnings from creeping back.


Even with JDK-8074096, the product does not build 100% warning free. 
This is due to some warnings that cannot be disabled, or (in one case) 
where C and C++ code is mixed, and warnings for both languages cannot 
be used. A system similar to the one introduced in JDK-8074096 is 
therefore needed, in which individual libraries can be exempted from 
this flag, until such warnings are fixed. A library can thus disable 
warnings as errors with WARNINGS_AS_ERRORS := false, or (better) use a 
toolchain-specific version, e.g WARNINGS_AS_ERRORS_gcc := false. This 
is intended as a temporary measure, though. The long-term solution is 
reasonably to fix the warnings and remove that argument.


Also, different versions of compilers can generate a different set of 
warnings. It is therefore necessary to be able to turn off this 
globally. Therefor a new flag for configure is introduced: 
--disable-warnings-as-errors.


While the code compiles without errors on the build systems used 
internally at Oracle, this might not be the case on other combinations 
of operating system versions and toolchain versions. To facilitate for 
unexpecting developers, a help message is added if the build fails, 
that suggests using --disable-warnings-as-errors. This solution was 
chosen as a compromise between the hard core solution of turning on 
warnings as errors by default for anyone, and the cowar... erm, 
conservative solution of checking if the compiler versions exactly 
match what's used inside Oracle (and therefore regularly tested), and 
only turn it on in that case.


Similarly to JDK-8074096, I intend to file follow-up bugs for each 
individual library that got a WARNINGS_AS_ERRORS_* := false.


Bug: https://bugs.openjdk.java.net/browse/JDK-8074859
WebRev for top: 
http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-top/webrev.01
WebRev for jdk: 
http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-jdk/webrev.01


Some comments:

* I needed to add a few more DISABLED_WARNINGS. For windows, this is 
most likely due to the recent compiler change. For other libraries, 
I'm not sure, but it might well be the result of recent changes that 
has introduced new warnings. If so, it highlights the need of this 
patch to keep the build warning free.


* For a few libraries and toolchains, there is *both* 
WARNINGS_AS_ERRORS := false and a DISABLED_WARNINGS list. This is the 
case if not all warnings are possible to disable.


* I have removed some incorrect uses of SHARED_LIBRARY_FLAGS. This is 
included in our JDK LDFLAGS, so it should not be set separately, and 
definitely not as CFLAGS. (This caused compiler warnings, which now 
turned into errors.) However, a more suitable long-term solution is 
probably to move the knowledge of how to create shared libraries more 
specifically into SetupNativeCompilation, and not set it as part of 
the JDK flags.


/Magnus

Re: AWT Dev RfR JDK-8055831 Open Source Java Access Bridge

[Erik Joelsson]

Hello Peter,

The new source layout and Lib-jdk.accessibility.gmk look much better. 
Thanks!


Regarding the .def files, I was mistaken and missed that they were 
referenced in the LDFLAGS, they were indeed used in the build so no 
reason to remove them.


/Erik

On 2015-03-24 22:36, Pete Brunet wrote:

Here's the latest patch:
http://cr.openjdk.java.net/~ptbrunet/JDK-8055831/webrev.01/

The changes are:
- restructured the native libraries from one directory to several
directories on a per DLL/EXE basis
- that impacted jdk/make/lib/Lib-jdk.accessibility.gmk
- removed the source for the Ferret and Monkey test tools which will be
the subject of a later patch
- removed the DEF files; although they were used in the build, there are
no build or runtime problems after their removal

On 3/24/15 8:08 AM, Magnus Ihse Bursie wrote:

On 2015-03-23 18:31, Pete Brunet wrote:

Hi Erik,

I tried the restructuring about 2 weeks ago and the build failed trying
to find an h file in the common directory.  I used two directories on
the SRC := setting for SetupNativeCompilation but the build failed not
finding an h file located in the second (common) directory.

For .h files, you need to also set the -I flag correctly. This is not
done automatically from SRC. (Maybe it should.)

We have a pattern that you can copy to get this behavior. E.g.

LIBINSTRUMENT_SRC :=
$(JDK_TOPDIR)/src/java.instrument/share/native/libinstrument \
$(JDK_TOPDIR)/src/java.instrument/$(OPENJDK_TARGET_OS_TYPE)/native/libinstrument
\
 #
LIBINSTRUMENT_CFLAGS := $(CFLAGS_JDKLIB) \
 $(addprefix -I, $(LIBINSTRUMENT_SRC)) \
 -Isomewhere-else \
 #


There are a number of extra .cpp files in the libaccessbridge dir that
aren't used in any of the libraries. What is the purpose of those?
Keeping source code around that is not being built seems strange to
me. There are also extra .rc files and a bunch of .DEF files. Are the
.DEF files used for anything? If all these files really need to be
included in our source base, perhaps sort them out into a
jdk.accessibility/windows/native/misc dir or something so that it's
clear what is needed to build the product and what is not?

The Ferret and Monkey tools need to be built and added to the JDK
image.  This is JDK-8056925.  That covers 2 CPP and the 3 RC files plus
AccessInfo.cpp.  Do you want me to remove those now and add them back in
later?

I would prefer that. It is generally better to make each change
coherent, unless there is much work in making some changes that will
very shortly be undone. This is not the case here. This change should
only add the files that are needed for these dll:s to compile.


JavaAccessBridge.DEF is pretty empty.  I'll see if the build will work
without it.

WinAccessBridge.DEF seems like it might be needed.  What do you think?

Is it accessed by the compiler? Unless it is given as input to the
compiler or linker command line, it is not used in the build. Unless
the microsoft compiler does some magic trick and picks up files
unasked based on filename, that is. (Unlikely but not impossible, but
I'd like to see that proved.)

/Magnus

Re: AWT Dev RfR JDK-8055831 Open Source Java Access Bridge

[Erik Joelsson]

On 2015-03-24 14:08, Magnus Ihse Bursie wrote:




JavaAccessBridge.DEF is pretty empty.  I'll see if the build will work
without it.

WinAccessBridge.DEF seems like it might be needed.  What do you think?


Is it accessed by the compiler? Unless it is given as input to the 
compiler or linker command line, it is not used in the build. Unless 
the microsoft compiler does some magic trick and picks up files 
unasked based on filename, that is. (Unlikely but not impossible, but 
I'd like to see that proved.)


The .DEF file is not used unless the linker is called with '-def 
file.def'. AFAIK, we never did that in the jdk build. More info can be 
found here:

https://msdn.microsoft.com/en-us/library/28d6s79h.aspx

/Erik

Re: AWT Dev RfR JDK-8055831 Open Source Java Access Bridge

[Erik Joelsson]

Actually, I just noticed that we are indeed setting -def: to the linker. 
Please leave the .def files in the patch.


/Erik

On 2015-03-24 15:55, Erik Joelsson wrote:


On 2015-03-24 14:08, Magnus Ihse Bursie wrote:




JavaAccessBridge.DEF is pretty empty.  I'll see if the build will work
without it.

WinAccessBridge.DEF seems like it might be needed.  What do you think?


Is it accessed by the compiler? Unless it is given as input to the 
compiler or linker command line, it is not used in the build. Unless 
the microsoft compiler does some magic trick and picks up files 
unasked based on filename, that is. (Unlikely but not impossible, but 
I'd like to see that proved.)


The .DEF file is not used unless the linker is called with '-def 
file.def'. AFAIK, we never did that in the jdk build. More info can be 
found here:

https://msdn.microsoft.com/en-us/library/28d6s79h.aspx

/Erik

Re: AWT Dev RfR JDK-8055831 Open Source Java Access Bridge

[Erik Joelsson]

Hello Pete,

In general this looks good. However, to better fit with our intended 
source code file layout, I would prefer if the source was organized by 
the names of the libraries being built, and this would be a good time to 
get it done properly. Something like this:


jdk.accessibility/windows/native/libjavaaccessbridge/
AccessBridgeATInstance.cpp
AccessBridgeJavaEntryPoints.cpp
JavaAccessBridge.cpp

jdk.accessibility/windows/native/libwindowsaccessbridge/
AccessBridgeJavaVMInstance.cpp
AccessBridgeMessageQueue.cpp
AccessBridgeWindowsEntryPoints.cpp
WinAccessBridge.cpp
AccessBridgeEventHandler.cpp

jdk.accessibility/windows/native/common
AccessBridgeDebug.cpp
AccessBridgeMessages.cpp

jdk.accessibility/windows/native/libjabsysinfo/
AccessBridgeSysInfo.cpp

The header files needed for more than one lib would also go in common, 
otherwise in the specific lib dir. The SetupNativeCompilation calls 
would then not need to list explicit files but would only need to list 
the necessary directories.


There are a number of extra .cpp files in the libaccessbridge dir that 
aren't used in any of the libraries. What is the purpose of those? 
Keeping source code around that is not being built seems strange to me. 
There are also extra .rc files and a bunch of .DEF files. Are the .DEF 
files used for anything? If all these files really need to be included 
in our source base, perhaps sort them out into a 
jdk.accessibility/windows/native/misc dir or something so that it's 
clear what is needed to build the product and what is not?


/Erik

On 2015-03-21 05:33, Pete Brunet wrote:

Please review the following patch which will add the code of the Java
Access Bridge (JAB) and related Java Accessibility Utilities to OpenJDK.

This code is used by Assistive Technology such as screen readers and
screen magnifiers used by those who are blind or have low vision.  AT
use the JAB native API and the JAB in turn uses the Java Accessibility
API (JAAPI).  For more information on JAAPI see the javax.accessibility
package.  This is a Windows accessibility solution.

http://cr.openjdk.java.net/~ptbrunet/JDK-8055831/webrev.00/

Pete

Re: AWT Dev RFR: JDK-8074096 Disable (most) native warnings in JDK on a per-library basis

[Erik Joelsson]

Thanks, looks good!

/Erik

On 2015-03-06 17:14, Magnus Ihse Bursie wrote:

On 2015-03-04 14:31, Erik Joelsson wrote:

Hello,

Really nice to finally see this patch getting done!

Only one comment:

flags.m4:
In the grep expression, could you move the extra [] outside of the 
actual command line options to grep so that the command line could be 
copied to the shell for debugging in the future? Also, how hard would 
it be to instead do AC_COMPILE_IFELSE with a dummy warning to instead 
test for capability?


I have updated the fix to use the eminent 
FLAGS_COMPILER_CHECK_ARGUMENTS instead. :-)


http://cr.openjdk.java.net/~ihse/JDK-8074096-disable-native-warnings/webrev.02 



/Magnus

Re: AWT Dev RFR: JDK-8074096 Disable (most) native warnings in JDK on a per-library basis

[Erik Joelsson]

Hello,

Really nice to finally see this patch getting done!

Only one comment:

flags.m4:
In the grep expression, could you move the extra [] outside of the 
actual command line options to grep so that the command line could be 
copied to the shell for debugging in the future? Also, how hard would it 
be to instead do AC_COMPILE_IFELSE with a dummy warning to instead test 
for capability?


/Erik

On 2015-03-04 14:17, Magnus Ihse Bursie wrote:
When building the native code in the jdk repo, a lot of warnings are 
generated. So many that it's hard to spot any new issues.


While the ultimate goal must be to address and fix these warnings 
individually, this bug is about disabling these warnings where they 
occur, so that it is easier to spot new warnings, and that the 
existing warnings can be addressed one at a time.


Disabling warnings instead of fixing them can be problematic. If you 
are too broad with disabling warnings, you might hide future problems. 
On the other hand, there have been a lot of warnings that indicate 
serious problems that has been hidden in plain sight for a very long 
time in the code base anyway.


I have opted to disable warnings at the library level. Disabling 
warnings globally would be too broad. Disabling per file would have 
been too tedious. Many warnings also tend to repeat across multiple 
files in the same library, due to e.g. less well-suited programming 
style or design choice. A library also seems like a suitable chunk of 
work to address later on, in trying to get rid of the source of the 
warnings.


This fix will not get rid of all warnings, but the bulk of them. It 
will make the remaining warnings stick out more. The few warnings that 
remain are either:

* Not possible to disable.
* Not resulting from native compilation (but from linking, or javadoc, 
etc).
* Not possible to disable with this framework (this goes for builds on 
pre-4.4 gcc, which Oracle currently does as default on macosx), and 
libfontmanager on Solaris, which mixes C and C++ code. (While the 
solstudio C compiler does not fail on requests to disable C++ 
warnings, the converse is unfortunately not true). It did not seem 
worth the trouble to build a more extensible framework to handle this, 
compared to actually fixing these warnings.


Note that the current JPRT build environment in Oracle uses a pre-4.4 
gcc for macosx by default, so the default macosx build will still 
contain warnings. When building with --with-toolchain-type=clang, the 
clang warning disabling kicks in. (This will be the default in JPRT 
later on this year.)


I intend to file individual bugs to handle these remaining warnings, 
so the net result will be a completely warning free build.


I also intend to file individual bugs on all the libraries that has 
had warnings disabled. I expect the outcome of these bugs to be either:


A) The code modified so it does not trigger warnings, and the warnings 
re-enabled, or


B) The warnings (or a subset of them) kept disabled, but a comment 
added to the makefile describing why this is the proper course of action.


Not all bugs might be worth fixing. For instance, the GCC warnings 
type-limits, sign-compare, pointer-to-int-cast, conversion-null, 
deprecated-declarations, clobbered, int-to-pointer-cast and 
type-limits are all more or less benign, and is possibly the result of 
a false positive.


On the other hands, warnings such as format-nonliteral, unused-result, 
maybe-uninitialized, format, format-security, int-to-pointer-cast, 
reorder and delete-non-virtual-dtor are more likely to actually point 
to real issues. I recommend anyone finding these warnings on a library 
they care about to try and fix them as soon as possible.


Here is a summary of the libraries and binaries that have gotten 
warnings disabled. I'm not sure about what group owns all these 
libraries; if I missed sending this mail to the correct list, please 
help me and forward it.


* libunpack
* libfdlibm
* libverify
* libjava
* libzip
* libjli/libjli_static
* libj2gss
* libsunec
* libj2pkcs11
* libnet
* libnio
* libosxkrb5
* libosxapp
* libosx
* libapplescriptengine
* libjsound
* libjsoundalsa
* libmlib_image
* libawt
* libawt_xawt
* libawt_lwawt
* liblcms
* libjavajpeg
* libawt_headless
* libfontmanager
* libsplashscreen
* unpack200
* The JVMTI demos compiledMethodLoad and waiters

Bug: https://bugs.openjdk.java.net/browse/JDK-8074096

WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8074096-disable-native-warnings/webrev.01


/Magnus

Re: AWT Dev [9] Review Request: 8039269 images/cursors should not be in ${java.home}/lib

[Erik Joelsson]

Build changes still look good.

/Erik

On 2015-02-16 12:57, Sergey Bylokhov wrote:

Hello,
Here is an updated version of the fix:

http://cr.openjdk.java.net/~serb/8039269/webrev.01

- alan.bate...@oracle.com wrote:


On 13/02/2015 18:01, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 9.
As requested cursor related properties/images were moved from /lib

to the java.desktop.

   - Image prefixes were removed because I moved them to the os

specific location.

   - Windows version of cursors.properties was removed, because it

was mostly the same as shared version.

   - ***NoDrop images were removed, because they are the same as

invalid32x32.gif. cursors.properties was updated accordingly.

   - Note that if in the future some osx/unix will demand some

specific images/properties, it will be necessary to place them to the
correct folder only.

Bug: https://bugs.openjdk.java.net/browse/JDK-8039269
Webrevs can be found at:
  http://cr.openjdk.java.net/~serb/8039269/webrev.00/root
  http://cr.openjdk.java.net/~serb/8039269/webrev.00/jdk


As Mandy pointed, you use try-with-resources. Also I think you can use

in rather than fis as the input stream is no longer a
FileInputStream.

I think the main thing here is that it has been established that
cursor.properties is not a supported interface, that was the question

that issue was trying to establish for a long time. Also good to have

these files moved out of the conf directory.

-Alan

Re: AWT Dev [9] Review Request: 8039269 images/cursors should not be in ${java.home}/lib

[Erik Joelsson]

Hello Sergey,

The build changes look ok to me.

It's unfortunate that you had to explicitly exclude the file from 
CompileProperties, but I don't see a better solution at this time. 
Ideally, I would like to see a different file name extension for 
properties files being compiled than to those being cleaned.


/Erik

On 2015-02-13 19:01, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 9.
As requested cursor related properties/images were moved from /lib to the 
java.desktop.
  - Image prefixes were removed because I moved them to the os specific 
location.
  - Windows version of cursors.properties was removed, because it was mostly 
the same as shared version.
  - ***NoDrop images were removed, because they are the same as 
invalid32x32.gif. cursors.properties was updated accordingly.
  - Note that if in the future some osx/unix will demand some specific 
images/properties, it will be necessary to place them to the correct folder 
only.

Bug: https://bugs.openjdk.java.net/browse/JDK-8039269
Webrevs can be found at:
 http://cr.openjdk.java.net/~serb/8039269/webrev.00/root
 http://cr.openjdk.java.net/~serb/8039269/webrev.00/jdk

Re: AWT Dev [8u60] RFR: 8043340: [macosx] Fix hard-wired paths to JavaVM.framework

[Erik Joelsson]

Hello,

This looks good to me. Thanks for the detailed table!

/Erik

On 2015-01-14 04:09, David DeHaven wrote:

The --with-xcode-path argument is optional, you should also be able to build with Xcode 4 
selected via sudo xcode-select -switch /path/to/Xcode4.app. I leave MAS 
managed Xcode (currently 6) active as I'm constantly bouncing between projects and it's a 
hassle to have to remember to reset the active toolchain, so I thought it best to allow 
configure to be provided a path.

Ugh. I broke something along the way, this doesn't *quite* work.

xcrun complains with xcrun: error: missing DEVELOPER_DIR path:

I think I'm exporting an empty DEVELOPER_DIR. I shall fix and update.

TL;DR: Please review round 2:
http://cr.openjdk.java.net/~ddehaven/8043340/jdk8u/v1/

(I removed generated-configure.sh to reduce the review size, it will be 
re-generated prior to pushing)


I've tested the following configuration scenarios (output from a shell script I 
cobbled together..)

field values:
XC6 - Xcode 6 installed in /Applications/Xcode.app
XC4 - Xcode 4 installed in some other dir
(empty) - Argument not passed to configure

Result meanings:
DEV_DIR set - configure succeeded, DEVELOPER_DIR was properly exported in 
spec.gmk
DEV_DIR NOT set - configure succeeded, DEVELOPER_DIR was not needed (XC4 must 
be selected to achieve this)
Configure failed - Configure properly failed when it detected Xcode  4

Selected Xcode means version reported by xcode-select -p


| Xcode selected | --with-xcode-path | DEVELOPER_DIR | result   |
-
| XC4|   |   | DEV_DIR NOT set  |
-
| XC4|   | XC4   | DEV_DIR set  |
-
| XC4|   | XC6   | Configure failed |
-
| XC4| XC4   |   | DEV_DIR set  |
-
| XC4| XC4   | XC4   | DEV_DIR set  |
-
| XC4| XC4   | XC6   | DEV_DIR set  |
-
| XC4| XC6   |   | Configure failed |
-
| XC4| XC6   | XC4   | Configure failed |
-
| XC4| XC6   | XC6   | Configure failed |
-
| XC6|   |   | Configure failed |
-
| XC6|   | XC4   | DEV_DIR set  |
-
| XC6|   | XC6   | Configure failed |
-
| XC6| XC4   |   | DEV_DIR set  |
-
| XC6| XC4   | XC4   | DEV_DIR set  |
-
| XC6| XC4   | XC6   | DEV_DIR set  |
-
| XC6| XC6   |   | Configure failed |
-
| XC6| XC6   | XC4   | Configure failed |
-
| XC6| XC6   | XC6   | Configure failed |
-

All the results are as expected. Please note that --with-xcode-path overrides 
DEVELOPER_DIR, since that could be set in the environment.

(yeah, I went a little OCD on this...)

-DrD-

Re: AWT Dev [9] Review Request: 8056298 Separate java.awt.datatransfer from the desktop module

[Erik Joelsson]

The makefile change looks good to me.

/Erik

On 2015-01-13 12:40, Sergey Bylokhov wrote:

Hi, Alan.
On 12.01.2015 23:42, Alan Bateman wrote:
Thanks for doing this. I think it looks okay except for modules.xml 
where it looks like there may be a few issues.


1. You've updated the definition of java.corba to depend on 
java.datatransfer but I don't think this is needed (is there code in 
java.corba that uses this API?).


2. Same thing needs to be checked in java.xml.bind, jdk.jconsole and 
jdk.runtime where it's not clear to me that code in these modules 
uses this API.


java.datatransfer was removed everywhere except:

1. java.xml.bind
jaxws/src/java.xml.bind/share/classes/com/sun/xml/internal/org/jvnet/staxex/StreamingDataHandler.java:53: 
error: cannot access Transferable
public abstract class StreamingDataHandler extends DataHandler 
implements Closeable {


2. java.xml.soap
jaxws/src/java.xml.soap/share/classes/com/sun/xml/internal/messaging/saaj/soap/FastInfosetDataContentHandler.java:53: 
error: cannot find symbol

public DataFlavor[] getTransferDataFlavors() { // throws Exceptio

3. java.xml.ws
jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/ws/developer/StreamingDataHandler.java:51: 
error: cannot access Transferable
public abstract class StreamingDataHandler extends 
com.sun.xml.internal.org.jvnet.staxex.StreamingDataHandler {


The new versions:
http://cr.openjdk.java.net/~serb/8056298/webrev.02/jdk
http://cr.openjdk.java.net/~serb/8056298/webrev.02/root

Bug: https://bugs.openjdk.java.net/browse/JDK-8056298



2. The update to java.activation to depend on java.datatransfer looks 
right but shouldn't you drop the dependency on java.desktop?


I don't have any other comments except to mention that java.xml.soap 
has been subsumed into java.xml.ws. Those changes are in jdk9/dev and 
it looks like jdk9/client might be a bit out of date.


-Alan

Re: AWT Dev [8u60] RFR: 8043340: [macosx] Fix hard-wired paths to JavaVM.framework

[Erik Joelsson]

I'm happy with that answer. Thanks!

/Erik

On 2015-01-12 17:25, David DeHaven wrote:

Or rather, the point of this exercise is to eliminate the hacks to get it to 
build with Xcode 5 (I'm not sure if anyone was truly successful with that). 
It's far better to just build with Xcode 4.6.3, and with these changes you 
don't even need to pre-sanitize your Xcode environment.

A proper build setup would have Xcode 5 or 6 installed in /Applications/Xcode.app 
(generally MAS managed) and Xcode 4.6.3 (still available for download on ADC) somewhere 
NOT directly in /Applications (the Mac App Store has a nasty habit of 
upgrading it when it sees it there). I keep mine in /Applications/old along 
with a copy of Xcode 5.1.1 for test purposes.

The --with-xcode-path argument is optional, you should also be able to build with Xcode 4 
selected via sudo xcode-select -switch /path/to/Xcode4.app. I leave MAS 
managed Xcode (currently 6) active as I'm constantly bouncing between projects and it's a 
hassle to have to remember to reset the active toolchain, so I thought it best to allow 
configure to be provided a path.


I'm kind of bummed I didn't get to show my really nasty hack for working around 
the broken lipo stub tool, it was so horrible it could be considered artwork! :)

-DrD-


It won't build at all with Xcode 5, there is no gcc compiler and the clang 
changes were never backported to jdk8u.

-DrD-


Hello,

These changes look ok to me.

With these changes, configure will unconditionally fail if trying to use XCode 
5. I know we don't officially support using XCode 5 for JDK 8, but aren't 
people working around it with some patches? How hard would it be to make it at 
least build?

/Erik

On 2015-01-10 05:45, David DeHaven wrote:

Please review the open source changes for 8043340. The goal here is to get 
jdk8u to build on Mac OS X 10.9+ systems where Xcode 5+ and Xcode 4 are 
co-installed, a configuration which is becoming more and more commonplace as 
more developers are focusing on JDK 9 now (which needs Xcode 5 installed), not 
to mention new systems ship with 10.10 which only supports the Xcode 5/6 
command line tools. It's too much of a hassle to maintain separate systems for 
building jdk8u and JPRT isn't suitable for frequent changes so something must 
be done to address this.

The jdk changes are similar to those done for JDK9, though I removed the 
changes for building with Xcode 5 (JDK-8043591) since we do not support 
building JDK 8 with clang.

The hotspot changes are almost identical, the lack of SYSROOT_CFLAGS 
necessitated changing the logic in saproc.make a bit. It still builds with or 
without spec.gmk, though without it you will either need to define SDKPATH or 
have a sane Xcode 4 installation.

For the top level build system:
- most of the logic of sanitizing the Xcode build environment is in toolchain.m4
- the LIPO variable was removed since it was completely unused
- OTOOL was moved to after the Xcode sanitizing so it can be picked up from 
DEVELOPER_DIR if needed
- MACOSX_UNIVERSAL is now being set to false by default and 
ALT_MACOSX_UNIVERSAL was added to hotspot-spec.gmk.in so the hotspot build is 
in sync with the jdk build (this was a bug, IMHO)

That last change removed any need to run lipo (only done in hotspot), so the 
fact that /usr/bin/lipo is broken with Xcode 4 is a non-issue.

There is a weird case where some early versions of the Xcode 5 command line 
tools installed /usr/bin/{gcc|g++} as a symlink to {clang|clang++}, that case 
is handled by putting $DEVELOPER_DIR/usr/bin on the path so autoconf picks up 
the actual gcc executable.

JBS Issue:
https://bugs.openjdk.java.net/browse/JDK-8043340

Webrevs:
http://cr.openjdk.java.net/~ddehaven/8043340/jdk8u/v0/top
http://cr.openjdk.java.net/~ddehaven/8043340/jdk8u/v0/hotspot
http://cr.openjdk.java.net/~ddehaven/8043340/jdk8u/v0/jdk

JPRT runs are being kicked off. I'll have one run from hotspot directly. I'll 
post results here.

-DrD-

Re: AWT Dev RFR: 8056216 : Remove sun directory layer from libawt and common

[Erik Joelsson]

Looks good to me. Thanks for fixing this!

/Erik

On 2014-09-18 22:42, Phil Race wrote:

https://bugs.openjdk.java.net/browse/JDK-8056216
http://cr.openjdk.java.net/~prr/8056216/

This is all just removing the sequence sun/ from various pathnames.

Aside from the make file changes there are over 600 file moves which I 
don't think
its worth including in the webrev but they are the result of the 
following 'hg mv' commands


hg mv src/java.desktop/windows/native/libawt/sun/* 
src/java.desktop/windows/native/libawt/
hg mv src/java.desktop/windows/native/common/sun/awt 
src/java.desktop/windows/native/common
hg mv src/java.desktop/share/native/libawt/sun/* 
src/java.desktop/share/native/libawt
hg mv src/java.desktop/share/native/common/sun/* 
src/java.desktop/share/native/common
hg mv src/java.desktop/macosx/native/libawt_lwawt/sun/* 
src/java.desktop/macosx/native/libawt_lwawt
hg mv src/java.desktop/unix/native/libawt/sun/* 
src/java.desktop/unix/native/libawt
hg mv src/java.desktop/unix/native/common/sun/* 
src/java.desktop/unix/native/common
hg mv src/java.desktop/unix/native/libawt_headless/sun/awt 
src/java.desktop/unix/native/libawt_headless
hg mv src/java.desktop/unix/native/libawt_xawt/sun/* 
src/java.desktop/unix/native/libawt_xawt/


I have done full open+closed builds via JPRT on all platforms ...

-phil.

Re: AWT Dev RFR: JDK-8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Erik Joelsson]

Further testing revealed some more issues. New webrev:

http://cr.openjdk.java.net/~erikj/8038340/webrev.root.03/

I had to break out the devkit/sysroot parts from from BASIC_SETUP_PATHS 
so that it happened after the early custom hook.


/Erik

On 2014-03-26 12:36, Magnus Ihse Bursie wrote:


On 2014-03-26 12:32, Erik Joelsson wrote:

Thanks, here is a new webrev:

http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/

Looks good to me.

/Magnus

AWT Dev RFR: JDK-8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Erik Joelsson]

Hello,

(including 2d-dev/awt-dev because I'm changing linker flags on desktop 
libs.)


In preparation for upgrading compilers and build platforms, we would 
like to get better separation between the two. At least on Linux and 
Solaris this is certainly possible. Some work for this was already done 
in Jdk 8 but it was never taken all the way through into actual usage.


In this bug, the configure parameters relating to this will be changed to:

--with-sysroot: Sets a sysroot directory that will be propagated to the 
compiler (gcc --sysroot) if supported and which will be used by 
configure to look for all dependencies like headers and libraries. (old 
--with-sys-root will be kept for compatibility as an alias)


--with-toolchain-path: Prepends to the path when looking for compilers 
and other target specific tools. Replaces --with-tools-dir, which will 
also be kept for compatibility as an alias.


--with-extra-path: Prepends to the path when looking for everything.

--with-devkit: Points to the root of a devkit. A devkit may have a 
devkit.info in its root detailing values for the above three 
parameters. If not, the fallback behavior is what it used to do. (set 
toolchain_path to devkit/bin and some options for subdirs for sysroot)



Along with these changes some more was needed and done:

* To get it to work properly on Solaris, the OPENWIN variables were 
removed and replaced with X_LIBS and X_CFLAGS. Several unnecessary 
runtime paths for awt* libs were removed and some were added where they 
were actually needed instead. All the libs have been verified to still 
find their dependencies with ldd.


* The devkit generation makefiles for linux (in root/make/devkit) were 
updated to the proposed compiler versions and adding the devkit.info file.


* A minor fix in compare.sh that was missed in an earlier patch.

Bug: https://bugs.openjdk.java.net/browse/JDK-8038340
Webrevs:
http://cr.openjdk.java.net/~erikj/8038340/webrev.root.01/
http://cr.openjdk.java.net/~erikj/8038340/webrev.jdk.01/

/Erik

Re: AWT Dev RFR: JDK-8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Erik Joelsson]

Thanks, here is a new webrev:

http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/

/Erik

On 2014-03-26 12:14, Magnus Ihse Bursie wrote:

That's a lot of good changes! :-)

Some comments:

In basics.m4:
* The description of --with-devkit is not ideal. It does more than 
sets sysroot and toolchain path (e.g. the extra path, and perhaps more 
things to come). Maybe just some more vague description like use this 
devkit for compilers, tools and resources?
* The comment If a separate sysroot has not been defined, use the one 
in the devkit does not really match what's happening anymore. 
Overriding of SYSROOT happens outside that block. A more suitable 
comment would be something about what we do if DEVKIT_SYSROOT is not set.


In toolchain.m4:
* There is some logic trying to avoid setting /usr/ccs/bin to the PATH 
on Solaris if the toolchain path is set. But since toolchain path will 
be prepended just below, we don't need that precaution, and we can 
just set it always.


Apart from that, it looks great!

/Magnus

On 2014-03-26 11:32, Erik Joelsson wrote:

Hello,

(including 2d-dev/awt-dev because I'm changing linker flags on 
desktop libs.)


In preparation for upgrading compilers and build platforms, we would 
like to get better separation between the two. At least on Linux and 
Solaris this is certainly possible. Some work for this was already 
done in Jdk 8 but it was never taken all the way through into actual 
usage.


In this bug, the configure parameters relating to this will be 
changed to:


--with-sysroot: Sets a sysroot directory that will be propagated to 
the compiler (gcc --sysroot) if supported and which will be used by 
configure to look for all dependencies like headers and libraries. 
(old --with-sys-root will be kept for compatibility as an alias)


--with-toolchain-path: Prepends to the path when looking for 
compilers and other target specific tools. Replaces --with-tools-dir, 
which will also be kept for compatibility as an alias.


--with-extra-path: Prepends to the path when looking for everything.

--with-devkit: Points to the root of a devkit. A devkit may have a 
devkit.info in its root detailing values for the above three 
parameters. If not, the fallback behavior is what it used to do. (set 
toolchain_path to devkit/bin and some options for subdirs for sysroot)



Along with these changes some more was needed and done:

* To get it to work properly on Solaris, the OPENWIN variables were 
removed and replaced with X_LIBS and X_CFLAGS. Several unnecessary 
runtime paths for awt* libs were removed and some were added where 
they were actually needed instead. All the libs have been verified to 
still find their dependencies with ldd.


* The devkit generation makefiles for linux (in root/make/devkit) 
were updated to the proposed compiler versions and adding the 
devkit.info file.


* A minor fix in compare.sh that was missed in an earlier patch.

Bug: https://bugs.openjdk.java.net/browse/JDK-8038340
Webrevs:
http://cr.openjdk.java.net/~erikj/8038340/webrev.root.01/
http://cr.openjdk.java.net/~erikj/8038340/webrev.jdk.01/

/Erik

Re: AWT Dev RFR: Allow using a system installed libpng

[Erik Joelsson]

At least to me this looks good, but better let Magnus and Andrew have 
their say too.


/Erik

On 2014-02-14 18:27, Omair Majid wrote:

* Andrew Hughes gnu.and...@redhat.com [2014-02-13 23:59]:

As I said in the previous e-mail, the minimum I'd be happy with is if
the current patch was updated so settings weren't being hardcoded into
the makefiles. Passing them from configure would be sufficient for now,
then it can be replaced by PKG_CONFIG.

How does this updated patch look:
http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/03/
http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/03-jdk/

Thanks,
Omair

Re: AWT Dev RFR: 8025673: Disable X11 AWT toolkit

[Erik Joelsson]

Still looks good to me.

/Erik

On 2013-10-22 05:34, David DeHaven wrote:

Updated webrev for JDK (hotspot change is the same):
http://cr.openjdk.java.net/~ddehaven/8025673/jdk.1/

Changes since last version:
- Moved to jdk8/build/jdk to save someone a merge headache, moved changes to 
CompileNativeLibs.gmk to libs/Awt2dLibraries.gmk
- Removed HToolkit option and toolkit selection code from java_props_macosx.[ch]

-DrD-



I want to do one more iteration of this. Based on feedback it seems I can 
remove a bit more code from java_props_macosx.[ch] and make things a bit 
cleaner.

-DrD-


Thanks guys.

Anthony, can you sponsor this for me?

-DrD-


This fix looks fine to me as well.

--
best regards,
Anthony

On 10/20/2013 11:56 PM, David DeHaven wrote:

CCing: build-dev, macosx-port-dev, hotspot-dev

Request for review of JDK-8025673:
https://bugs.openjdk.java.net/browse/JDK-8025673

Proposed changes:
http://cr.openjdk.java.net/~ddehaven/8025673/

This change disables building libawt_xawt.dylib and libawt_headless.dylib on 
Mac since they are not used and not supported. There are too many challenges 
(and not enough time) in removing all X11 code from the Mac build at this time, 
so we're deferring complete removal for later (will be covered by JDK-8003900).

A small change to hotspot is required as it was looking for libawt_xawt.dylib 
and if not found would set java.awt.headless to true. Since we don't build a 
headless only JRE on Mac I just have that method return false. I'm not sure how 
to handle changes to hotspot, can it be pushed along with the jdk changes? 
Without that change the Mac builds will be broken.

Significant build system changes, build-dev guys are encouraged to comment...

I tried excluding all sun/awt/X11 classes in CompileJavaClasses.gmk but that 
broke JNI header generation on platforms still using X11 and I couldn't use the 
big list of excluded files on Mac as that resulted in Java compilation errors, 
so I just added some logic to exclude everything on Mac and left the list in 
place everywhere else.

The changes to CompileNativeLibraries.gmk will port to 
libs/AwtJava2dLibraries.gmk in jdk8/build, however there is a problem in the 
jdk8/build workspace where the build cannot find symbols in JNI libs so that 
issue needs to be resolved first. I've not had time to investigate that problem.


Question for the AWT team, we still have this in java_props_md.c:
458 PreferredToolkit prefToolkit = getPreferredToolkit();
459 if (prefToolkit == CToolkit) {
460 sprops.awt_toolkit = sun.lwawt.macosx.LWCToolkit;
461 } else {
462 // TODO: do we still need this?
463 sprops.awt_toolkit = sun.awt.HToolkit;
464 }

Is that necessary? Since we're now using libawt_lwawt in both headless and 
headful modes I would think we could remove the HToolkit option, but I'm not 
100% certain about that.


I've built and tested on Mac and a Linux VM (Ubuntu 12.04) and both seem to be 
working fine.

JPRT run for Mac is in progress, I will submit one for all other platforms when 
it finishes building.

-DrD-

Re: AWT Dev RFR: 8025673: Disable X11 AWT toolkit

[Erik Joelsson]

Hello David,

From a build point of view, the changes look fine to me.

/Erik

On 2013-10-21 01:56, David DeHaven wrote:

CCing: build-dev, macosx-port-dev, hotspot-dev

Request for review of JDK-8025673:
https://bugs.openjdk.java.net/browse/JDK-8025673

Proposed changes:
http://cr.openjdk.java.net/~ddehaven/8025673/

This change disables building libawt_xawt.dylib and libawt_headless.dylib on 
Mac since they are not used and not supported. There are too many challenges 
(and not enough time) in removing all X11 code from the Mac build at this time, 
so we're deferring complete removal for later (will be covered by JDK-8003900).

A small change to hotspot is required as it was looking for libawt_xawt.dylib 
and if not found would set java.awt.headless to true. Since we don't build a 
headless only JRE on Mac I just have that method return false. I'm not sure how 
to handle changes to hotspot, can it be pushed along with the jdk changes? 
Without that change the Mac builds will be broken.

Significant build system changes, build-dev guys are encouraged to comment...

I tried excluding all sun/awt/X11 classes in CompileJavaClasses.gmk but that 
broke JNI header generation on platforms still using X11 and I couldn't use the 
big list of excluded files on Mac as that resulted in Java compilation errors, 
so I just added some logic to exclude everything on Mac and left the list in 
place everywhere else.

The changes to CompileNativeLibraries.gmk will port to 
libs/AwtJava2dLibraries.gmk in jdk8/build, however there is a problem in the 
jdk8/build workspace where the build cannot find symbols in JNI libs so that 
issue needs to be resolved first. I've not had time to investigate that problem.


Question for the AWT team, we still have this in java_props_md.c:
  458 PreferredToolkit prefToolkit = getPreferredToolkit();
  459 if (prefToolkit == CToolkit) {
  460 sprops.awt_toolkit = sun.lwawt.macosx.LWCToolkit;
  461 } else {
  462 // TODO: do we still need this?
  463 sprops.awt_toolkit = sun.awt.HToolkit;
  464 }

Is that necessary? Since we're now using libawt_lwawt in both headless and 
headful modes I would think we could remove the HToolkit option, but I'm not 
100% certain about that.


I've built and tested on Mac and a Linux VM (Ubuntu 12.04) and both seem to be 
working fine.

JPRT run for Mac is in progress, I will submit one for all other platforms when 
it finishes building.

-DrD-

Re: AWT Dev RFR: 8016096: [macosx] jawt_md.h shipped with jdk is outdated

[Erik Joelsson]

Hello David,

From a build point of view, the changes look fine to me.

/Erik

On 2013-10-21 01:53, David DeHaven wrote:

CCing: build-dev, macosx-port-dev

Request for review of JDK-8016096:
https://bugs.openjdk.java.net/browse/JDK-8016096

Proposed changes:
http://cr.openjdk.java.net/~ddehaven/8016096/

Significant build system changes for this one so feedback from build-dev is 
appreciated. Actual functional changes are just ported from the JDK7 changes I 
made several months ago, less the X11 dependency in jawt_md.h. The autoconf 
changes also trigger an update of generated-configure.sh in jdk/make/closed, 
that will need to be pushed too.


Please note:
These patches will not work alone! You must also apply the patches for 8025673, 
which I'm also submitting for review.


Built and tested on Mac and Linux. I'm in the process of submitting JPRT runs.

-DrD-

Re: AWT Dev [7u] Review request for 7129133: [macosx] Accelerators are displayed as Meta instead of the Command symbol

[Erik Joelsson]

Build part looks ok.

/Erik

On 2013-10-01 07:39, dmitry markov wrote:

Hello,

Could you review a back-port of 7129133 to JDK 7u, please? The 
back-port and the main fix integrated into jdk8 are slightly different.


bug: http://bugs.sun.com/view_bug.do?bug_id=7129133
webrev for jdk7u: http://cr.openjdk.java.net/~dmarkov/7129133/webrev.00/
jdk8 changeset: http://hg.openjdk.java.net/jdk8/awt/jdk/rev/602e5d0141d3
technical review for jdk8: 
http://mail.openjdk.java.net/pipermail/awt-dev/2013-September/005450.html


Thanks,
Dmitry

Re: AWT Dev [8] Review request for 7129133: [macosx] Accelerators are displayed as Meta instead of the Command symbol

[Erik Joelsson]

Build part of changes looks ok. I can't comment on the AWT part.

/Erik

On 2013-09-11 11:24, Leonid Romanov wrote:

Hello,
Please review a fix for 7129133: [macosx] Accelerators are displayed as Meta 
instead of the Command symbol.

bug: http://bugs.sun.com/view_bug.do?bug_id=7129133
webrev: http://cr.openjdk.java.net/~leonidr/7129133/webrev.00/

Thanks,
Leonid.

Re: AWT Dev Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

Fixed both issues reported below

http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.05/

/Erik

On 2013-01-21 09:28, Fredrik Öhrström wrote:

double slash here: -I$(JDK_TOPDIR)//src/share/native/common

2013/1/21 David Holmesdavid.hol...@oracle.com:

Minor typo in the gmk file

  32 # These are the once used.

once -  ones

Thanks,
David


On 19/01/2013 12:05 AM, Erik Joelsson wrote:

Here is a new webrev with my sorts incorporated. This has been baking
for quite a while in build-infra now and seems to be working.

http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/

/Erik

On 2012-12-07 11:05, Erik Joelsson wrote:

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order
of the output was different in the generated sizes file compared to
the one in the repo. Sorting both resulted in a clean diff.

/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:

I just submitted a patch to build-infra for the dual generation on
all platforms since it breaks comparisons between old and new
build. In general, we can't change behavior in new build without
also changing the old before the old is removed.

Removing sizes.64-solaris-i386 breaks the old build on solaris-x86,
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:


http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/

http://cr.openjdk.java.net/%7Eohrstrom/webrev-8004151-gensrcX11wrapper-v2/



Ok, new webrev incorporating a few more Erik changes. Any comments
from the AWT experts?
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: AWT Dev Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

Here is a new webrev with my sorts incorporated. This has been baking 
for quite a while in build-infra now and seems to be working.


http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/

/Erik

On 2012-12-07 11:05, Erik Joelsson wrote:

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order 
of the output was different in the generated sizes file compared to 
the one in the repo. Sorting both resulted in a clean diff.


/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:
I just submitted a patch to build-infra for the dual generation on 
all platforms since it breaks comparisons between old and new 
build. In general, we can't change behavior in new build without 
also changing the old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, 
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 
http://cr.openjdk.java.net/%7Eohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 



Ok, new webrev incorporating a few more Erik changes. Any comments 
from the AWT experts?

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: AWT Dev Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order of 
the output was different in the generated sizes file compared to the one 
in the repo. Sorting both resulted in a clean diff.


/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:
I just submitted a patch to build-infra for the dual generation on 
all platforms since it breaks comparisons between old and new build. 
In general, we can't change behavior in new build without also 
changing the old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, 
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 
http://cr.openjdk.java.net/%7Eohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 



Ok, new webrev incorporating a few more Erik changes. Any comments 
from the AWT experts?

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik