Re: 8199271: [TESTBUG] open source VM testbase stress tests

[serguei.spit...@oracle.com]

Hi Leonid,

Looks good to me too.

Thanks,
Serguei


On 5/14/18 14:04, Leonid Mesnik wrote:

Misha

Thank you for review. I still need one more review from 'R'eviewer.

Leonid


On May 11, 2018, at 9:10 AM, Mikhailo Seledtsov  
wrote:

Looks good to me,

Misha

On 5/8/18, 2:23 PM, Leonid Mesnik wrote:

Hi

Please review this change open sourcing vm testbase stress tests. These tests 
have been developed a long time ago for internal test harness and don't looks 
very nice now.
They are open sourced with minimal changes only. The long term plan is to 
review and improve them moving out of vmTestbase directory in corresponding 
components.

The fix introduce vmTestbase_nsk_stress test group and have make files changes 
required to build native part of tests.

I've verified that "make -- run-test TEST=:vmTestbase_nsk_stress" pass on my 
linux locally.

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

Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Philip Race]

Hi,

OK .. if you can convince upstream this is worth doing, then we can 
accept it

as we would not regress when updating. As I noted previously :
http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html
this is still an issue in the currently being developed 9c train.

-phil.

On 5/14/18, 3:06 AM, Adam Farley8 wrote:

Hi Phil,

Would an acceptable compromise be to deliver the source code change
and send the code to the upstream community, allowing them to include
the fix if/when they are able?

I believe Magnus was advocating this idea as well. See below.

Best Regards

Adam Farley

> Same here. I would like to have this fix in, but do not want to go
> over Phils head.
>
> I think Phil was the main objector, maybe he could reconsider?`
>
> Thanks, Thomas
>
> On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie
>  wrote:
> > I don't object, but it's not build code so I don't have the final say.
> >
> > /Magnus
> >
> >
> > On 2018-04-25 17:43, Adam Farley8 wrote:
> >
> > Hi All,
> >
> > Does anyone have an objection to pushing this tiny change in?
> >
> > It doesn't break anything, it fixes a build break on two supported
> > platforms, and it seems
> > like we never refresh the code from upstream.
> >
> > - Adam
> >
> >> I also advocate the source code fix, as Make isn't meant to use 
the sort

> >> of logic required
> >> to properly analyse the toolchain version string.
> >>
> >> e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 
4.8.6,

> >> and Make doesn't
> >> seem to do substring stuff unless you mess around with shells.
> >>
> >> Plus, as people have said, it's better to solve problem x (or work 
around

> >> a specific
> >> instance of x) than to ignore the exception, even if the ignoring 
code is

> >> toolchain-
> >> specific.
> >>
> >> - Adam Farley
> >>
> >> > On 2018-03-27 18:44, Phil Race wrote:
> >> >
> >> >> As I said I prefer the make file change, since this is a change 
to an

> >> >> upstream library.
> >> >
> >> > Newtons fourth law: For every reviewer, there's an equal and 
opposite

> >> > reviewer. :)
> >> >
> >> > Here I am, advocating a source code fix. As Thomas says, the 
compiler
> >> > complaint seems reasonable, and disabling it might cause us to 
miss other

> >> > future errors.
> >> >
> >> > Why can't we push the source code fix, and also submit it upstream?
> >> >
> >> > /Magnus
> >> >
> >> >
> >> > I've looked at jpeg-9c and it still looks identical to what we 
have in

> >> > 6b, so this code
> >> > seems to have stood the test of time. I'm also unclear why the 
compiler

> >> > would
> >> > complain about that and not the one a few lines later
> >> >
> >> >
> >> >  819   while (bits[i] == 0)  /* find largest codelength 
still in

> >> > use */
> >> >  820 i--;
> >> >
> >> > A push to jchuff.c will get blown away if/when we upgrade.
> >> > A tool-chain specific fix in the makefile with an appropriate 
comment is

> >> > more targeted.
> >>
> >> Phil,
> >>
> >> Returning to this.
> >>
> >> While I understand your reluctance to patch upstream code, let me 
point
> >> out that we have not updated libjpeg a single time since the JDK 
was open
> >> sourced. We're using 6b from 27-Mar-1998. I had a look at the 
Wikipedia page
> >> on libjpeg, and this is the latest "uncontroversial" version of 
the source.
> >> Versions 7 and up have proprietary extensions, which in turn has 
resulted in
> >> multiple forks of libjpeg. As it stands, it seems unlikely that we 
will ever

> >> replace libjpeg 6b with a simple upgrade from upstream.
> >>
> >> I therefore maintain my position that a source code fix would be 
the best

> >> way forward here.
> >>
> >> /Magnus
> >>
> >> >
> >> >
> >> > -phil.
> >> >
> >> >
> >> > On 03/27/2018 05:44 AM, Thomas Stüfe wrote:
> >> >
> >> > Hi all,
> >> >
> >> >
> >> > just a friendly reminder. I would like to push this tiny fix 
because
> >> > tripping over this on our linux s390x builds is annoying (yes, 
we can

> >> > maintain patch queues, but this is a constant error source).
> >> >
> >> >
> >> > I will wait for 24 more hours until a reaction. If no serious 
objections
> >> > are forcoming, I want to push it (tier1 tests ran thru, and me 
and Christoph

> >> > langer are both Reviewers).
> >> >
> >> >
> >> > Thanks! Thomas
> >> >
> >> >
> >> > On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe 


> >> > wrote:
> >> >
> >> > Hi all,
> >> >
> >> >
> >> > may I please have another review for this really trivial change. It
> >> > fixes a gcc warning on s390 and ppc. Also, it is probably a good 
idea to fix

> >> > this.
> >> >
> >> >
> >> > bug: https://bugs.openjdk.java.net/browse/JDK-8200052
> >> > webrev:
> >> > 
http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/ 
 


> >> >
> >> >
> >> > This was contributed by Adam 

Re: RFR: 8191522: Remove references to Lucida fonts from OpenJDK sources

[Erik Joelsson]

Build changes look good.

/Erik


On 2018-05-16 15:52, Phil Race wrote:

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

The Lucida fonts have never been part of OpenJDK but many places in 
the code
and tests reference them. There are even a couple of stray fonts.dir 
files that

probably should not have been in there.

This fix cleans up as much of this as we are also intending to no 
longer ship

these fonts with Oracle JDK as it converges with OpenJDK.

There is one minor build change in here, so I've included the build list.

-phil.

RFR: 8191522: Remove references to Lucida fonts from OpenJDK sources

[Phil Race]

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

The Lucida fonts have never been part of OpenJDK but many places in the code
and tests reference them. There are even a couple of stray fonts.dir 
files that

probably should not have been in there.

This fix cleans up as much of this as we are also intending to no longer 
ship

these fonts with Oracle JDK as it converges with OpenJDK.

There is one minor build change in here, so I've included the build list.

-phil.

Re: [8u] RFR: 8042707: Source changes needed to build JDK 9 with Visual Studio 2013 (VS2013)

[Erik Joelsson]

Sounds good.

/Erik


On 2018-05-16 14:14, Kevin Walls wrote:

Hi,

FYI, I haven't pushed this to 8u yet but am about to, with two changes 
in jdk/make/CopyFiles.gmk:


define copy-and-chmod had a colon and extra space on the end of the 
line. LIB_DST_DIR is not defined here in 8u, it should be 
$(JDK_OUTPUTDIR)/bin


Thanks
Kevin


On 26/04/2018 16:57, Erik Joelsson wrote:


Looks good.

/Erik


On 2018-04-26 01:38, Kevin Walls wrote:


Thanks Erik -

I went ahead with the jdk's make/CopyFiles.gmk change, and added 
SetupCopyFiles to the base repo's make/common/MakeBase.gmk.


I updated the webrev, to include base and jdk repos:

http://cr.openjdk.java.net/~kevinw/8042707/webrev.01/

I'm getting these build OK with VS2012, but there will be further 
hotspot change at least for VS2013 to be a working option in 8u.


Thanks
Kevin
(my previous reply was not the the list, so this is the open response!)



On 20/04/2018 23:27, Erik Joelsson wrote:

The root repo changes look ok.

The changes in Copy-java.base.gmk applies to 
jdk/make/CopyFiles.gmk. Those changes are definitely needed.


/Erik


On 2018-04-20 13:18, Kevin Walls wrote:

Hi,

I'd like to request a review of the backport from 9 to 8u:

8042707: Source changes needed to build JDK 9 with Visual Studio 
2013 (VS2013)

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

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/jdk9/rev/39ee0ee4f890
jdk repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/c622a8ba90ad

9 review thread: 
http://mail.openjdk.java.net/pipermail/build-dev/2015-January/014029.html



Notes:
base repo:
toolchain_windows.m4: quite a bit of manual work, but no conflicts.
make/common/MakeBase.gmk: changes in SetupCopyFiles which we don't 
have in 8u
flags.m4: we don't call it COMMON_CXXFLAGS_JDK in 8u, but made the 
same change.


jdk repo:
make/copy/Copy-java.base.gmk we don't have in 8u. The other two 
files apply cleanly.



Clearly this backport isn't to change anything about what 
compilers are supported or recommended,

it's just about the build infrastructure.


8u change: webrev of the base repo changes:
http://cr.openjdk.java.net/~kevinw/8042707/webrev.00/

Many thanks
Kevin

Re: [8u] RFR: 8042707: Source changes needed to build JDK 9 with Visual Studio 2013 (VS2013)

[Kevin Walls]

Hi,

FYI, I haven't pushed this to 8u yet but am about to, with two changes 
in jdk/make/CopyFiles.gmk:


define copy-and-chmod had a colon and extra space on the end of the 
line. LIB_DST_DIR is not defined here in 8u, it should be 
$(JDK_OUTPUTDIR)/bin


Thanks
Kevin


On 26/04/2018 16:57, Erik Joelsson wrote:


Looks good.

/Erik


On 2018-04-26 01:38, Kevin Walls wrote:


Thanks Erik -

I went ahead with the jdk's make/CopyFiles.gmk change, and added 
SetupCopyFiles to the base repo's make/common/MakeBase.gmk.


I updated the webrev, to include base and jdk repos:

http://cr.openjdk.java.net/~kevinw/8042707/webrev.01/

I'm getting these build OK with VS2012, but there will be further 
hotspot change at least for VS2013 to be a working option in 8u.


Thanks
Kevin
(my previous reply was not the the list, so this is the open response!)



On 20/04/2018 23:27, Erik Joelsson wrote:

The root repo changes look ok.

The changes in Copy-java.base.gmk applies to jdk/make/CopyFiles.gmk. 
Those changes are definitely needed.


/Erik


On 2018-04-20 13:18, Kevin Walls wrote:

Hi,

I'd like to request a review of the backport from 9 to 8u:

8042707: Source changes needed to build JDK 9 with Visual Studio 
2013 (VS2013)

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

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/jdk9/rev/39ee0ee4f890
jdk repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/c622a8ba90ad

9 review thread: 
http://mail.openjdk.java.net/pipermail/build-dev/2015-January/014029.html



Notes:
base repo:
toolchain_windows.m4: quite a bit of manual work, but no conflicts.
make/common/MakeBase.gmk: changes in SetupCopyFiles which we don't 
have in 8u
flags.m4: we don't call it COMMON_CXXFLAGS_JDK in 8u, but made the 
same change.


jdk repo:
make/copy/Copy-java.base.gmk we don't have in 8u. The other two 
files apply cleanly.



Clearly this backport isn't to change anything about what compilers 
are supported or recommended,

it's just about the build infrastructure.


8u change: webrev of the base repo changes:
http://cr.openjdk.java.net/~kevinw/8042707/webrev.00/

Many thanks
Kevin

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: JDK-8194327 [macos] AWT windows have incorrect main/key window behaviors

[Sergey Bylokhov]

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



--
Best regards, Sergey.

Re: RFR: JDK-8203221 Makefile fixes after Flight Recorder

[Magnus Ihse Bursie]

> 16 maj 2018 kl. 01:39 skrev Erik Joelsson :
> 
> Hello,
> 
> In GensrcJfr, JFR_TOOLS_OUTPUTDIR is defined twice.

Oops, will fix. 

> Other build tools are in make/{jdk,hotspot}/src/classes. Do you think we 
> should be moving them to one place? Regardless, I think we need an INCLUDE in 
> the SetupJavaCompilation so that we only build the tool we need. There is 
> currently no other tools in make/src/classes, but that directory is inviting 
> for others.

I think we should move all tools to a common place. The current solution is 
more an effect of how things ended up due to the old multi-repo design. 

So I though: let's start doing things right, and put this one in a common build 
tool directory, and then we can move the rest there in a follow up.

But if you think that is confusing (I could really agree) we can skip doing 
that now. If so, I'll just move this to some jfr_build_tools_classes dir 
instead. 

/Magnus

> 
> /Erik
> 
> 
>> On 2018-05-15 16:14, Magnus Ihse Bursie wrote:
>> Cleanups of the build system after Flight Recorder.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203221
>> WebRev: 
>> http://cr.openjdk.java.net/~ihse/JDK-8203221-makefile-fixes-after-flight-recorder/webrev.01
>> 
>> /Magnus
> 

RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

[Langer, Christoph]

Hi Matthias,

yes, reviewed.

Best regards
Christoph

From: Baesken, Matthias
Sent: Mittwoch, 16. Mai 2018 09:06
To: Langer, Christoph ; 'build-dev@openjdk.java.net' 
; ppc-aix-port-...@openjdk.java.net; 
core-libs-...@openjdk.java.net
Cc: Lindenmaier, Goetz 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

Hi  Christoph can I add you as second reviewer  (other reviewer was Erik 
Joelsson) can push the change ?

Best regards, Matthias



From: Langer, Christoph
Sent: Donnerstag, 26. April 2018 16:38
To: Baesken, Matthias 
>; 
'build-dev@openjdk.java.net' 
>; 
ppc-aix-port-...@openjdk.java.net; 
core-libs-...@openjdk.java.net
Cc: Simonis, Volker >
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

Hi Matthias,

to me the change in principal looks good.

I'm wondering if it is possible to do a comparison like xlc < 13 (e.g. extract 
major number before the first dot, then compare numerically) - but maybe it is 
too complicated and the current single version compare suits our needs ?

Best regards
Christoph

From: Baesken, Matthias
Sent: Donnerstag, 26. April 2018 16:14
To: 'build-dev@openjdk.java.net' 
>; 
ppc-aix-port-...@openjdk.java.net; 
core-libs-...@openjdk.java.net
Cc: Langer, Christoph 
>; Simonis, Volker 
>
Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

Hello  ,  could you please review this small adjustment to  the symbol 
visibility compilation settings on AIX ?
Currently  we use  XLC 12.1  to compile  JDK on AIX .

However XLC 12.1   does not support  the "-qvisibility=hidden"  setting 
currently set on AIX.
It was introduced with  XLC 13.1 . Christoph found some info about it here :

https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html

Setting it  only generates  hundreds  of warnings  in the build log , warnings 
look like this :
XlC12.1

bash-4.4$ xlC -qversion
IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)
Version: 12.01..0019

bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc
1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list of valid 
options.

Compare to XLC13.1

bash-3.00$ xlC -qversion
IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07)
Version: 13.01..0008
bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc
bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc


So it is better to avoid  setting these flags when using xlc12.1   .
Please review :

Bug :

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

Change :

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


Best regards, Matthias

RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

[Baesken, Matthias]

Hi  Christoph can I add you as second reviewer  (other reviewer was Erik 
Joelsson) can push the change ?

Best regards, Matthias



From: Langer, Christoph
Sent: Donnerstag, 26. April 2018 16:38
To: Baesken, Matthias ; 'build-dev@openjdk.java.net' 
; ppc-aix-port-...@openjdk.java.net; 
core-libs-...@openjdk.java.net
Cc: Simonis, Volker 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

Hi Matthias,

to me the change in principal looks good.

I'm wondering if it is possible to do a comparison like xlc < 13 (e.g. extract 
major number before the first dot, then compare numerically) - but maybe it is 
too complicated and the current single version compare suits our needs ?

Best regards
Christoph

From: Baesken, Matthias
Sent: Donnerstag, 26. April 2018 16:14
To: 'build-dev@openjdk.java.net' 
>; 
ppc-aix-port-...@openjdk.java.net; 
core-libs-...@openjdk.java.net
Cc: Langer, Christoph 
>; Simonis, Volker 
>
Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

Hello  ,  could you please review this small adjustment to  the symbol 
visibility compilation settings on AIX ?
Currently  we use  XLC 12.1  to compile  JDK on AIX .

However XLC 12.1   does not support  the "-qvisibility=hidden"  setting 
currently set on AIX.
It was introduced with  XLC 13.1 . Christoph found some info about it here :

https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html

Setting it  only generates  hundreds  of warnings  in the build log , warnings 
look like this :
XlC12.1

bash-4.4$ xlC -qversion
IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)
Version: 12.01..0019

bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc
1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list of valid 
options.

Compare to XLC13.1

bash-3.00$ xlC -qversion
IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07)
Version: 13.01..0008
bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc
bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc


So it is better to avoid  setting these flags when using xlc12.1   .
Please review :

Bug :

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

Change :

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


Best regards, Matthias