Re: [8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Tim Bell]

In that case, the review looks good.

Thanks-

Tim

On 04/13/18 13:15, Kevin Walls wrote:

Hi Erik - thanks for clarifying.


On 13/04/2018 20:58, Erik Joelsson wrote:

ccache shouldn't be commented out. That must have been a local edit
mistake in the original webrev.

/Erik


On 2018-04-13 12:51, Kevin Walls wrote:

Thanks Tim -

There's a later webrev in the review thread:
http://cr.openjdk.java.net/~erikj/8038340/webrev.root.03/ in which I
see the common/autoconf/configure.ac change... It's in the commit in
9 also.  I think we need it. 8-)

But that webrev.03 also has the commenting out of "Building
ccache..." in make/devkit/Makefile , though I don't see it happening
in 8038340 in the 9 change, or in latest jdk either. I'm thinking
that ccache was disabled for local testing and wasn't intended as
part of 8038340 - do let me know if you think otherwise!

Thanks
Kevin


On 13/04/2018 20:08, Tim Bell wrote:

Kevin - looks good in general with a few remarks (see below):


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

8038340: Cleanup and fix sysroot and devkit handling on Linux and
Solaris
JBS: https://bugs.openjdk.java.net/browse/JDK-8038340

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a
(clean import to jdk8u-dev)

JDK repo change imports cleanly.  base repo changes require some
manual
fixups in:

common/autoconf/basics.m4 addition of
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4 minor change
didn't import
make/common/NativeCompilation.gmkminor change
didn't import

...plus run autogen to recreate generated files.


9 review thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link
there to the previous emails in the thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )

8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin


common/autoconf/configure.ac

This change looks straightforward, but I don't see the file at all
in the JDK 9 webrev...


make/common/autoconf/configure.acdevkit/Makefile

lines 91,92 ... Do you want ccache?  It is commented out in the JDK
9 changes
(http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/make/devkit/Makefile.frames.html)

Re: [8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Kevin Walls]

Hi Erik - thanks for clarifying.


On 13/04/2018 20:58, Erik Joelsson wrote:
ccache shouldn't be commented out. That must have been a local edit 
mistake in the original webrev.


/Erik


On 2018-04-13 12:51, Kevin Walls wrote:

Thanks Tim -

There's a later webrev in the review thread: 
http://cr.openjdk.java.net/~erikj/8038340/webrev.root.03/ in which I 
see the common/autoconf/configure.ac change... It's in the commit in 
9 also.  I think we need it. 8-)


But that webrev.03 also has the commenting out of "Building 
ccache..." in make/devkit/Makefile , though I don't see it happening 
in 8038340 in the 9 change, or in latest jdk either. I'm thinking 
that ccache was disabled for local testing and wasn't intended as 
part of 8038340 - do let me know if you think otherwise!


Thanks
Kevin


On 13/04/2018 20:08, Tim Bell wrote:

Kevin - looks good in general with a few remarks (see below):


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

8038340: Cleanup and fix sysroot and devkit handling on Linux and 
Solaris

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

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a
(clean import to jdk8u-dev)

JDK repo change imports cleanly.  base repo changes require some 
manual

fixups in:

common/autoconf/basics.m4 addition of
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4 minor change
didn't import
make/common/NativeCompilation.gmk    minor change 
didn't import


...plus run autogen to recreate generated files.


9 review thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link
there to the previous emails in the thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )

8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin


common/autoconf/configure.ac

This change looks straightforward, but I don't see the file at all 
in the JDK 9 webrev...



make/common/autoconf/configure.acdevkit/Makefile

lines 91,92 ... Do you want ccache?  It is commented out in the JDK 
9 changes 
(http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/make/devkit/Makefile.frames.html)




/Tim

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

[Phil Race]

On 04/13/2018 12:44 PM, Volker Simonis wrote:


Phil Race > 
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.


That should be sufficient ...

-phil.

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).



-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
> 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: [8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Erik Joelsson]

ccache shouldn't be commented out. That must have been a local edit 
mistake in the original webrev.


/Erik


On 2018-04-13 12:51, Kevin Walls wrote:

Thanks Tim -

There's a later webrev in the review thread: 
http://cr.openjdk.java.net/~erikj/8038340/webrev.root.03/ in which I 
see the common/autoconf/configure.ac change... It's in the commit in 9 
also.  I think we need it. 8-)


But that webrev.03 also has the commenting out of "Building ccache..." 
in make/devkit/Makefile , though I don't see it happening in 8038340 
in the 9 change, or in latest jdk either. I'm thinking that ccache was 
disabled for local testing and wasn't intended as part of 8038340 - do 
let me know if you think otherwise!


Thanks
Kevin


On 13/04/2018 20:08, Tim Bell wrote:

Kevin - looks good in general with a few remarks (see below):


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

8038340: Cleanup and fix sysroot and devkit handling on Linux and 
Solaris

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

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a
(clean import to jdk8u-dev)

JDK repo change imports cleanly.  base repo changes require some manual
fixups in:

common/autoconf/basics.m4 addition of
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4    minor change
didn't import
make/common/NativeCompilation.gmk    minor change didn't 
import


...plus run autogen to recreate generated files.


9 review thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link
there to the previous emails in the thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )

8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin


common/autoconf/configure.ac

This change looks straightforward, but I don't see the file at all in 
the JDK 9 webrev...



make/common/autoconf/configure.acdevkit/Makefile

lines 91,92 ... Do you want ccache?  It is commented out in the JDK 9 
changes 
(http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/make/devkit/Makefile.frames.html)




/Tim

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

>
> Regards,
> Volker
>
> On Fri, Apr 13, 2018 at 6:00 PM, Erik Joelsson
> 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: [8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Kevin Walls]

Thanks Tim -

There's a later webrev in the review thread: 
http://cr.openjdk.java.net/~erikj/8038340/webrev.root.03/ in which I see 
the common/autoconf/configure.ac change... It's in the commit in 9 
also.  I think we need it. 8-)


But that webrev.03 also has the commenting out of "Building ccache..." 
in make/devkit/Makefile , though I don't see it happening in 8038340 in 
the 9 change, or in latest jdk either. I'm thinking that ccache was 
disabled for local testing and wasn't intended as part of 8038340 - do 
let me know if you think otherwise!


Thanks
Kevin


On 13/04/2018 20:08, Tim Bell wrote:

Kevin - looks good in general with a few remarks (see below):


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

8038340: Cleanup and fix sysroot and devkit handling on Linux and 
Solaris

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

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a
(clean import to jdk8u-dev)

JDK repo change imports cleanly.  base repo changes require some manual
fixups in:

common/autoconf/basics.m4 addition of
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4    minor change
didn't import
make/common/NativeCompilation.gmk    minor change didn't 
import


...plus run autogen to recreate generated files.


9 review thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link
there to the previous emails in the thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )

8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin


common/autoconf/configure.ac

This change looks straightforward, but I don't see the file at all in 
the JDK 9 webrev...



make/common/autoconf/configure.acdevkit/Makefile

lines 91,92 ... Do you want ccache?  It is commented out in the JDK 9 
changes 
(http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/make/devkit/Makefile.frames.html)




/Tim

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

[Volker Simonis]

Phil Race  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).


> -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 
> 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: [8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Tim Bell]

Kevin - looks good in general with a few remarks (see below):


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

8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris
JBS: https://bugs.openjdk.java.net/browse/JDK-8038340

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a
(clean import to jdk8u-dev)

JDK repo change imports cleanly.  base repo changes require some manual
fixups in:

common/autoconf/basics.m4 addition of
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4minor change
didn't import
make/common/NativeCompilation.gmkminor change didn't import

...plus run autogen to recreate generated files.


9 review thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link
there to the previous emails in the thread:
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )

8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin


common/autoconf/configure.ac

This change looks straightforward, but I don't see the file at all in 
the JDK 9 webrev...



make/common/autoconf/configure.acdevkit/Makefile

lines 91,92 ... Do you want ccache?  It is commented out in the JDK 9 
changes 
(http://cr.openjdk.java.net/~erikj/8038340/webrev.root.02/make/devkit/Makefile.frames.html)




/Tim

Re: [8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Erik Joelsson]

Looks good.

/Erik


On 2018-04-13 09:22, Kevin Walls wrote:

Hi,

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

8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris
JBS: https://bugs.openjdk.java.net/browse/JDK-8038340

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a 
(clean import to jdk8u-dev)


JDK repo change imports cleanly.  base repo changes require some 
manual fixups in:


common/autoconf/basics.m4 addition of 
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4                            minor change 
didn't import
make/common/NativeCompilation.gmk                minor change didn't 
import


...plus run autogen to recreate generated files.


9 review thread: 
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link 
there to the previous emails in the thread: 
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )


8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin

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

[Phil Race]

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

[Volker Simonis]

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

[8u] RFR: 8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris

[Kevin Walls]

Hi,

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

8038340: Cleanup and fix sysroot and devkit handling on Linux and Solaris
JBS: https://bugs.openjdk.java.net/browse/JDK-8038340

9 changesets:
base repo: http://hg.openjdk.java.net/jdk9/dev/rev/9786ef8ca58c
JDK: repo: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/fdeb6a8b0f3a 
(clean import to jdk8u-dev)


JDK repo change imports cleanly.  base repo changes require some manual 
fixups in:


common/autoconf/basics.m4 addition of 
AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], fails, done manually
common/autoconf/toolchain.m4                            minor change 
didn't import

make/common/NativeCompilation.gmk                minor change didn't import

...plus run autogen to recreate generated files.


9 review thread: 
http://mail.openjdk.java.net/pipermail/2d-dev/2014-April/004361.html
(thread starts in March, not that it's long, but I don't see a link 
there to the previous emails in the thread: 
http://mail.openjdk.java.net/pipermail/2d-dev/2014-March/004333.html )


8u webrev:http://cr.openjdk.java.net/~kevinw/8038340/webrev.00/

Many thanks!
Kevin

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: [11] RFR: 8201507: Generate alias entries in j.t.f.ZoneName from tzdb at build time

[Erik Joelsson]

Build changes look good.

/Erik


On 2018-04-12 17:34, Naoto Sato wrote:

Hi,

Please review the fix to the subject issue. While fixing 8189784 [1], 
I noticed that not only CLDR zones but also tzdb link entries are also 
hard coded. So I further modified j.t.f.ZoneName to generate tzdb 
entries at the build time.


The issue: https://bugs.openjdk.java.net/browse/JDK-8201507
Fix: http://cr.openjdk.java.net/~naoto/8201507/webrev.00/

Like the last time, including build-dev for makefile modification review.

Naoto

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

Fix for JDK-8201483 broken '-' containing features

[Chris Dennis]

Hi All,

It looks like the addition of support for disabling features has broken support 
for enabling features that contain a ‘-‘ in their name (amusingly this includes 
the 'all-gcs’ feature).

Pretty sure this is the offending code is the use of 'match($i, /-.*/)’ to 
distinguish between disabled and enabled features. That regex needs to match a 
‘-' only at the beginning of the line otherwise configures thinks you are 
disabling the ‘ll-gcs’ feature.

Thanks,

Chris Dennis

Re: RFR: JDK-8201483 Make it possible to disable JVM features

[David Holmes]

On 13/04/2018 5:40 PM, Magnus Ihse Bursie wrote:

On 2018-04-12 23:30, David Holmes wrote:

On 12/04/2018 11:33 PM, Magnus Ihse Bursie wrote:

On 2018-04-12 14:15, David Holmes wrote:

Hi Magnus,

On 12/04/2018 9:39 PM, Magnus Ihse Bursie wrote:
It is currently easy to add new JVM features to the JVM build, but 
it is not possible to remove features.


With this change, features can be both added or removed from the 
default set. They are added using --with-jvm-features=f1,f2 and 
removed using --with-jvm-features=-f1,-f2. The syntax can be 
combined, so --with-jvm-features=dtrace,-nmt will enable dtrace but 
disable native memory tracking.


I need to point out that we have never tested disabling individual 
VM features likes this. They are either all on, or all off for the 
minimal VM! There may be implicit dependencies between features.


Well, I have. :-) However, I don't do that regularly, and changes 
might very well have crept in. As always, if you build something 
non-standard that is not regularly tested, you're on your own.


Feels to me like you've taken away the safety-fence and are 
encouraging people to attempt these unsupported configurations. 
Whether that was your intent or not.


It is always possible to configure something that does not work. :-) We 
can make it more easy to do the right thing, but if we were to make 
impossible everything that has not been tested, then we would also make 
things impossible that are needed for some use cases.


I don't buy that.

Wrt JVM features, this has *always* been possible. If you use 
"--with-jvm-variants=custom --with-jvm-features=jvmti,nmt" then you 
*are* going to build an almost (or fully?) useless JVM. In fact, this 
method made it *much* harder to try to get a functioning JVM without a 
specific feature.


I have no recollection of jvm variant "custom" being introduced.



I think I should update the build documentation regarding JVM features 
though, and I can definitely add some wisely worded warnings about 
unsupported combinations of features, especially if removing them.


If you can document what the allowed configurations are then you should 
be able to add constraints that only allows supported sets of features. 
This is what the original set of JVM variants provided - it was fixed 
not arbitrary! As I said I don't recall you adding the "custom" option. :(




In any case, the purpose of this is not so much to disable existing 
JVM features (after all, no one has really been missing this 
functionality), as to pave the way for the upcoming patch for 
including/excluding individual GCs.


Surely a GC selection flag would have sufficed.
It was the common agreement of both the build team and the GC folks 
responsive for the upcoming selectively GC inclusion patch, that this 
was better handled as JVM features than a separate GC flag.


Maybe something that affects all of hotspot should have been discussed 
more broadly.


David
-


/Magnus


David


/Magnus



David

I also included some additional code cleanup and fixes, such as 
printing the JVM feature set at the summary.


Bug: https://bugs.openjdk.java.net/browse/JDK-8201483
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8201483-disable-JVM-features/webrev.01 

Re: Invalid use of -m32 on certain targets

[John Paul Adrian Glaubitz]

Hi Severin!

On 04/13/2018 02:16 PM, Severin Gehwolf wrote:

Is there a bug for this already?


Here is one:
https://bugs.openjdk.java.net/browse/JDK-8201536

Great, thank you. I couldn't reply earlier as I was just on my way.

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: RFR: JDK-8201483 Make it possible to disable JVM features

[David Holmes]

On 13/04/2018 5:25 PM, Volker Simonis wrote:

On Thu, Apr 12, 2018 at 11:30 PM, David Holmes  wrote:

On 12/04/2018 11:33 PM, Magnus Ihse Bursie wrote:


On 2018-04-12 14:15, David Holmes wrote:


Hi Magnus,

On 12/04/2018 9:39 PM, Magnus Ihse Bursie wrote:


It is currently easy to add new JVM features to the JVM build, but it is
not possible to remove features.

With this change, features can be both added or removed from the default
set. They are added using --with-jvm-features=f1,f2 and removed using
--with-jvm-features=-f1,-f2. The syntax can be combined, so
--with-jvm-features=dtrace,-nmt will enable dtrace but disable native memory
tracking.



I need to point out that we have never tested disabling individual VM
features likes this. They are either all on, or all off for the minimal VM!
There may be implicit dependencies between features.



Well, I have. :-) However, I don't do that regularly, and changes might
very well have crept in. As always, if you build something non-standard that
is not regularly tested, you're on your own.



Feels to me like you've taken away the safety-fence and are encouraging
people to attempt these unsupported configurations. Whether that was your
intent or not.


I think that would be great. If people would try out different
configurations AND fix them that would be a great code clean-up for
HotSpot. I've recently tried various "unsupported" configurations
(i.e. minimal plus some selected features) and found that the
resulting build failures are mostly artificial because the
corresponding features aren't correctly ifdefed.


These features were never expected to be individually selectable. The 
minimal VM was a special case. There is likely more work needed than 
just build time ifdefs to allow this to actually work.


For fun try building on 64-bit and ask for no compiler2 to see what 
happens. It makes no sense. But there are no constraints implemented so 
you can ask for nonsensical things. That's not a good thing in my book.


David
-







In any case, the purpose of this is not so much to disable existing JVM
features (after all, no one has really been missing this functionality), as
to pave the way for the upcoming patch for including/excluding individual
GCs.



Surely a GC selection flag would have sufficed.

David



/Magnus



David


I also included some additional code cleanup and fixes, such as printing
the JVM feature set at the summary.

Bug: https://bugs.openjdk.java.net/browse/JDK-8201483
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8201483-disable-JVM-features/webrev.01

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

[Volker Simonis]

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: Invalid use of -m32 on certain targets

[Severin Gehwolf]

On Fri, 2018-04-13 at 13:17 +0200, Severin Gehwolf wrote:
> On Thu, 2018-04-12 at 18:56 +0200, Magnus Ihse Bursie wrote:
> > > 12 apr. 2018 kl. 15:49 skrev John Paul Adrian Glaubitz 
> > > :
> > > 
> > > Hi!
> > > 
> > > I have been playing around with Zero on new (old) architectures and one
> > > of them is hppa, which needs some additional work due to its stack growing
> > > from bottom to top but that's another story.
> > > 
> > > Anyway, adding hppa to platform.m4 and running configure results in the
> > > following error message:
> > > 
> > > configure:35290: checking whether the C compiler works
> > > configure:35312: /usr/bin/hppa-linux-gnu-gcc -m32-m32   conftest.c  
> > > >&5
> > > hppa-linux-gnu-gcc: error: unrecognized command line option '-m32'
> > > hppa-linux-gnu-gcc: error: unrecognized command line option '-m32'
> > > configure:35316: $? = 1
> > > configure:35354: result: no
> > > configure: failed program was:
> > > 
> > > Didn't we recently have the discussion about which target gcc versions
> > > understand "-m32" and which don't? Apparently, someone thought hppa's
> > > gcc supports that option but apparently it doesn't.
> > > 
> > > Was there any conclusion on this discussion? I remember that some
> > > people mentioned that the current situation is not ideal.
> > 
> > I agree. It's not ideal, and should be fixed. It's on my todo list
> > but keep sinking to the bottom. If you want, you can open a bug on it
> > and I just might get around to it a bit quicker. :)
> 
> I've just attempted "configure" on s390 and here is what I get:
> configure:35266: checking whether the C compiler works
> configure:35288: /usr/bin/gcc -m32-m32   conftest.c  >&5
> gcc: error: unrecognized command line option '-m32'
> gcc: error: unrecognized command line option '-m32'
> configure:35292: $? = 1
> configure:35330: result: no
> configure: failed program was:
> > /* confdefs.h */
> > #define PACKAGE_NAME "OpenJDK"
> > #define PACKAGE_TARNAME "openjdk"
> > #define PACKAGE_VERSION "jdk9"
> > #define PACKAGE_STRING "OpenJDK jdk9"
> > #define PACKAGE_BUGREPORT "build-dev@openjdk.java.net"
> > #define PACKAGE_URL "http://openjdk.java.net;
> > /* end confdefs.h.  */
> > 
> > int
> > main ()
> > {
> > 
> >   ;
> >   return 0;
> > }
> 
> configure:35335: error: in `/jdk-hs':
> configure:35337: error: C compiler cannot create executables
> 
> Indeed:
>  sh-4.2# gcc -m32 conftest.c 
> gcc: error: unrecognized command line option ‘-m32’
>  sh-4.2# echo $?
> 1
>  sh-4.2# gcc conftest.c
>  sh-4.2# echo $?
> 0 
> 
> This must have broken recently :( I'll take a look. As it's preventing
> me to test JDK-8201495
> 
> Is there a bug for this already?

Here is one:
https://bugs.openjdk.java.net/browse/JDK-8201536

Cheers,
Severin

> Thanks,
> Severin
> 
> 
> > /Magnus
> > 
> > > 
> > > Adrian
> > > 
> > > -- 
> > > .''`.  John Paul Adrian Glaubitz
> > > : :' :  Debian Developer - glaub...@debian.org
> > > `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
> > >  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
> > > 
> > 
> > 

Re: Invalid use of -m32 on certain targets

[Severin Gehwolf]

On Thu, 2018-04-12 at 18:56 +0200, Magnus Ihse Bursie wrote:
> > 12 apr. 2018 kl. 15:49 skrev John Paul Adrian Glaubitz 
> > :
> > 
> > Hi!
> > 
> > I have been playing around with Zero on new (old) architectures and one
> > of them is hppa, which needs some additional work due to its stack growing
> > from bottom to top but that's another story.
> > 
> > Anyway, adding hppa to platform.m4 and running configure results in the
> > following error message:
> > 
> > configure:35290: checking whether the C compiler works
> > configure:35312: /usr/bin/hppa-linux-gnu-gcc -m32-m32   conftest.c  >&5
> > hppa-linux-gnu-gcc: error: unrecognized command line option '-m32'
> > hppa-linux-gnu-gcc: error: unrecognized command line option '-m32'
> > configure:35316: $? = 1
> > configure:35354: result: no
> > configure: failed program was:
> > 
> > Didn't we recently have the discussion about which target gcc versions
> > understand "-m32" and which don't? Apparently, someone thought hppa's
> > gcc supports that option but apparently it doesn't.
> > 
> > Was there any conclusion on this discussion? I remember that some
> > people mentioned that the current situation is not ideal.
> 
> I agree. It's not ideal, and should be fixed. It's on my todo list
> but keep sinking to the bottom. If you want, you can open a bug on it
> and I just might get around to it a bit quicker. :)

I've just attempted "configure" on s390 and here is what I get:
configure:35266: checking whether the C compiler works
configure:35288: /usr/bin/gcc -m32-m32   conftest.c  >&5
gcc: error: unrecognized command line option '-m32'
gcc: error: unrecognized command line option '-m32'
configure:35292: $? = 1
configure:35330: result: no
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "OpenJDK"
| #define PACKAGE_TARNAME "openjdk"
| #define PACKAGE_VERSION "jdk9"
| #define PACKAGE_STRING "OpenJDK jdk9"
| #define PACKAGE_BUGREPORT "build-dev@openjdk.java.net"
| #define PACKAGE_URL "http://openjdk.java.net;
| /* end confdefs.h.  */
| 
| int
| main ()
| {
| 
|   ;
|   return 0;
| }
configure:35335: error: in `/jdk-hs':
configure:35337: error: C compiler cannot create executables

Indeed:
 sh-4.2# gcc -m32 conftest.c 
gcc: error: unrecognized command line option ‘-m32’
 sh-4.2# echo $?
1
 sh-4.2# gcc conftest.c
 sh-4.2# echo $?
0 

This must have broken recently :( I'll take a look. As it's preventing
me to test JDK-8201495

Is there a bug for this already?

Thanks,
Severin


> /Magnus
> 
> > 
> > Adrian
> > 
> > -- 
> > .''`.  John Paul Adrian Glaubitz
> > : :' :  Debian Developer - glaub...@debian.org
> > `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
> >  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
> > 
> 
> 

Re: RFR: 8201274: Launch Single-File Source-Code Programs

[Maurizio Cimadamore]

One small followup question - the test SourceLauncherTest is not 
covering any shebang cases - is that deliberate? I see that those seem 
to be covered in the launcher test too, but I wonder if we should have 
tests for clearly broken stuff, such as


'#'

'#!'

'#!\n'

Another small question - I see that the shebang process essentially 
replaces the first line separator with \n - that is probably ok given 
that the file is processed internally after that - but I wonder if you 
have thought about pros and cons of preserving the original line separator.


Cheers
Maurizio


On 12/04/18 21:46, Maurizio Cimadamore wrote:
Looks great - some initial comments (I can't really comment on the 
launcher changes):


* This logic is efficient:

int magic = (in.read() << 8) + in.read();
boolean shebang = magic == (('#' << 8) + '!');

but convoluted to read; perhaps could be improved slightly by making 
'#' << 8) + '!' a static constant, and comparing against that.


* I note that the reading logic in general could be simplified, e.g. 
using Files.lines(Path) - but I assume that you wrote the code as is 
for performance reasons (e.g. to avoid creating too many string 
objects) ?


* I see that both the file manager and the class loader reasonably 
share the same context: a Map. I would make this more 
explicit, by having a Context class, whose state is the map, and then 
have the context provide two methods:


getClassLoader()
getFileManager()

This way the sharing will be automatic, no need to extract one field 
from one place and pass it over to the other place.


* Big whohoo for being able to use the enhanced diagnostic framework 
with a couple of tweaks on the makefile - I hope that would have been 
the case when I put in the support, but since we have never done it - 
wasn't 100% sure it would work ;-)



Overall I like it quite a lot and I think you went for a really clean 
design - kudos!


Maurizio



On 12/04/18 21:15, Jonathan Gibbons wrote:

Please review an initial implementation for the feature described in
JEP 330: Launch Single-File Source-Code Programs.

The work is described in the JEP and CSR, and falls into various parts:

  * The part to handle the new command-line options is in the native
    Java launcher code.
  * The part to invoke the compiler and subsequently execute the code
    found in the source file is in a new class in the jdk.compiler 
module.

  * There are some minor Makefile changes, to add support for a new
    resource file.

There are no changes to javac itself.

JEP: http://openjdk.java.net/jeps/330
JBS: https://bugs.openjdk.java.net/browse/JDK-8201274
CSR: https://bugs.openjdk.java.net/browse/JDK-8201275
Webrev: http://cr.openjdk.java.net/~jjg/8201274/webrev.00/

-- Jon

Re: RFR: 8201274: Launch Single-File Source-Code Programs

[Jan Lahoda]

The javac part looks OK to me.

A nit comment, in:
launcher/Main.java/MemoryFileManager#createInMemoryClassFile, there is:
return new FilterOutputStream(new ByteArrayOutputStream()) { ...

It could I think be written as:
return new ByteArrayOutputStream() {
@Override
public void close() throws IOException {
super.close();
byte[] bytes = toByteArray();
map.put(className, bytes);
}
};

(I.e. without the cast to ByteArrayOutputStream.)

Jan

On 12.4.2018 22:15, Jonathan Gibbons wrote:

Please review an initial implementation for the feature described in
JEP 330: Launch Single-File Source-Code Programs.

The work is described in the JEP and CSR, and falls into various parts:

  * The part to handle the new command-line options is in the native
Java launcher code.
  * The part to invoke the compiler and subsequently execute the code
found in the source file is in a new class in the jdk.compiler module.
  * There are some minor Makefile changes, to add support for a new
resource file.

There are no changes to javac itself.

JEP: http://openjdk.java.net/jeps/330
JBS: https://bugs.openjdk.java.net/browse/JDK-8201274
CSR: https://bugs.openjdk.java.net/browse/JDK-8201275
Webrev: http://cr.openjdk.java.net/~jjg/8201274/webrev.00/

-- Jon

Re: RFR: JDK-8201483 Make it possible to disable JVM features

[Magnus Ihse Bursie]

On 2018-04-12 23:30, David Holmes wrote:

On 12/04/2018 11:33 PM, Magnus Ihse Bursie wrote:

On 2018-04-12 14:15, David Holmes wrote:

Hi Magnus,

On 12/04/2018 9:39 PM, Magnus Ihse Bursie wrote:
It is currently easy to add new JVM features to the JVM build, but 
it is not possible to remove features.


With this change, features can be both added or removed from the 
default set. They are added using --with-jvm-features=f1,f2 and 
removed using --with-jvm-features=-f1,-f2. The syntax can be 
combined, so --with-jvm-features=dtrace,-nmt will enable dtrace but 
disable native memory tracking.


I need to point out that we have never tested disabling individual 
VM features likes this. They are either all on, or all off for the 
minimal VM! There may be implicit dependencies between features.


Well, I have. :-) However, I don't do that regularly, and changes 
might very well have crept in. As always, if you build something 
non-standard that is not regularly tested, you're on your own.


Feels to me like you've taken away the safety-fence and are 
encouraging people to attempt these unsupported configurations. 
Whether that was your intent or not.


It is always possible to configure something that does not work. :-) We 
can make it more easy to do the right thing, but if we were to make 
impossible everything that has not been tested, then we would also make 
things impossible that are needed for some use cases.


Wrt JVM features, this has *always* been possible. If you use 
"--with-jvm-variants=custom --with-jvm-features=jvmti,nmt" then you 
*are* going to build an almost (or fully?) useless JVM. In fact, this 
method made it *much* harder to try to get a functioning JVM without a 
specific feature.


I think I should update the build documentation regarding JVM features 
though, and I can definitely add some wisely worded warnings about 
unsupported combinations of features, especially if removing them.




In any case, the purpose of this is not so much to disable existing 
JVM features (after all, no one has really been missing this 
functionality), as to pave the way for the upcoming patch for 
including/excluding individual GCs.


Surely a GC selection flag would have sufficed.
It was the common agreement of both the build team and the GC folks 
responsive for the upcoming selectively GC inclusion patch, that this 
was better handled as JVM features than a separate GC flag.


/Magnus


David


/Magnus



David

I also included some additional code cleanup and fixes, such as 
printing the JVM feature set at the summary.


Bug: https://bugs.openjdk.java.net/browse/JDK-8201483
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8201483-disable-JVM-features/webrev.01 

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

[Severin Gehwolf]

Hi Volker,

On Fri, 2018-04-13 at 09:03 +0200, Volker Simonis wrote:
> Hi Severin,
> 
> I'm currently looking at the AIX-side of this bug.
> 
> The problem I see with your solution is that it uses LDFLAGS (which is
> generic) to filter out Linux specific linker flags. If we would extend
> this to AIX, we would have to add yet another substitution for AIX
> which filters out "-Wl,bernotok". This is not beautiful and gets
> uglier with every new platform.

Right. Note, though, that Magnus seems to be working on a more
comprehensive solution to this:
http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021625.html

FWIW, the substitution for -Wl,-z,defs was there in LDFLAGS all along
(it was missing the other syntax). Not sure if -Wl,-z,defs is generic
enough to warrant it being on LDFLAGS directly, not LDFLAGS_linux or
some other OS specific variant. Either way, that's the model I've
followed.

Thanks,
Severin

> But as this change has already been pushed and because (fortunately)
> on AIX options which come later on the command line take precedence
> over earlier ones I can easily fix this with a specific LDFLAGS_aix
> setting. I've opened "8201524: [AIX] Don't link libfontmanager against
> libawt_headless" [1] for it and I'll send out a new RFR for it soon.
> 
> Regards,
> Volker
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8201524
> 
> 
> On Wed, Apr 11, 2018 at 10:17 AM, Severin Gehwolf  wrote:
> > On Tue, 2018-04-10 at 14:51 -0700, Sergey Bylokhov wrote:
> > > LIBS_aix := -lawt_headless,
> > > I guess that AIX team should have a similar fix.
> > 
> > Possibly. I have no way of testing it, though. So will leave it to AIX
> > folk to have a look. My experience was that it isn't easily
> > reproducible. Some observations:
> > 
> >1. Run swing app such as SwingSet2 on a headfull system. Since
> >   fontmanager will have a link dep on lawt_headless, and awt code
> >   loads libawt_xawt (headfull) on a headfull system, both libraries
> >   providing symbols needed by libfontmanager will be loaded. Then it
> >   depends whether this is a problem on that particular system or not.
> >   In my experience this worked on some systems and not on others.
> >2. Solaris was build-time linking to libawt_headless causing bug
> >   8194870. So build-time linking got removed with that bug. Not sure
> >   why that bug is private :(
> > 
> > Thanks,
> > Severin
> > 
> > > On 10/04/2018 09:34, Erik Joelsson wrote:
> > > > 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.0
> > > > > 2
> > > > > 
> > > > > 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.h
> > > > > > tml
> > > > > > 
> > > > > > 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/webr
> > > > > > > ev.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
> 

Re: RFR: JDK-8201483 Make it possible to disable JVM features

[Volker Simonis]

On Thu, Apr 12, 2018 at 11:30 PM, David Holmes  wrote:
> On 12/04/2018 11:33 PM, Magnus Ihse Bursie wrote:
>>
>> On 2018-04-12 14:15, David Holmes wrote:
>>>
>>> Hi Magnus,
>>>
>>> On 12/04/2018 9:39 PM, Magnus Ihse Bursie wrote:

 It is currently easy to add new JVM features to the JVM build, but it is
 not possible to remove features.

 With this change, features can be both added or removed from the default
 set. They are added using --with-jvm-features=f1,f2 and removed using
 --with-jvm-features=-f1,-f2. The syntax can be combined, so
 --with-jvm-features=dtrace,-nmt will enable dtrace but disable native 
 memory
 tracking.
>>>
>>>
>>> I need to point out that we have never tested disabling individual VM
>>> features likes this. They are either all on, or all off for the minimal VM!
>>> There may be implicit dependencies between features.
>>
>>
>> Well, I have. :-) However, I don't do that regularly, and changes might
>> very well have crept in. As always, if you build something non-standard that
>> is not regularly tested, you're on your own.
>
>
> Feels to me like you've taken away the safety-fence and are encouraging
> people to attempt these unsupported configurations. Whether that was your
> intent or not.

I think that would be great. If people would try out different
configurations AND fix them that would be a great code clean-up for
HotSpot. I've recently tried various "unsupported" configurations
(i.e. minimal plus some selected features) and found that the
resulting build failures are mostly artificial because the
corresponding features aren't correctly ifdefed.

>
>> In any case, the purpose of this is not so much to disable existing JVM
>> features (after all, no one has really been missing this functionality), as
>> to pave the way for the upcoming patch for including/excluding individual
>> GCs.
>
>
> Surely a GC selection flag would have sufficed.
>
> David
>
>
>> /Magnus
>>
>>>
>>> David
>>>
 I also included some additional code cleanup and fixes, such as printing
 the JVM feature set at the summary.

 Bug: https://bugs.openjdk.java.net/browse/JDK-8201483
 WebRev:
 http://cr.openjdk.java.net/~ihse/JDK-8201483-disable-JVM-features/webrev.01

>>
>

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

[Volker Simonis]

Hi Severin,

I'm currently looking at the AIX-side of this bug.

The problem I see with your solution is that it uses LDFLAGS (which is
generic) to filter out Linux specific linker flags. If we would extend
this to AIX, we would have to add yet another substitution for AIX
which filters out "-Wl,bernotok". This is not beautiful and gets
uglier with every new platform.

But as this change has already been pushed and because (fortunately)
on AIX options which come later on the command line take precedence
over earlier ones I can easily fix this with a specific LDFLAGS_aix
setting. I've opened "8201524: [AIX] Don't link libfontmanager against
libawt_headless" [1] for it and I'll send out a new RFR for it soon.

Regards,
Volker

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


On Wed, Apr 11, 2018 at 10:17 AM, Severin Gehwolf  wrote:
> On Tue, 2018-04-10 at 14:51 -0700, Sergey Bylokhov wrote:
>> LIBS_aix := -lawt_headless,
>> I guess that AIX team should have a similar fix.
>
> Possibly. I have no way of testing it, though. So will leave it to AIX
> folk to have a look. My experience was that it isn't easily
> reproducible. Some observations:
>
>1. Run swing app such as SwingSet2 on a headfull system. Since
>   fontmanager will have a link dep on lawt_headless, and awt code
>   loads libawt_xawt (headfull) on a headfull system, both libraries
>   providing symbols needed by libfontmanager will be loaded. Then it
>   depends whether this is a problem on that particular system or not.
>   In my experience this worked on some systems and not on others.
>2. Solaris was build-time linking to libawt_headless causing bug
>   8194870. So build-time linking got removed with that bug. Not sure
>   why that bug is private :(
>
> Thanks,
> Severin
>
>> On 10/04/2018 09:34, Erik Joelsson wrote:
>> > 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.0
>> > > 2
>> > >
>> > > 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.h
>> > > > tml
>> > > >
>> > > > 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/webr
>> > > > > ev.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
>>
>>