Re: RFR(S): 8211350: Remove jprt support

[Jonathan Gibbons]

*Chuckle* at the long-obsolete reference in test/langtools/Makefile

 292 apt:JTREG_TESTDIRS = tools/apt

Otherwise, apart from other overdue cleanup, test/langtools/Makefile 
looks OK.


-- Jon

On 10/02/2018 12:21 AM, Mikael Vidstedt wrote:

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/

* Background (from the issue)

The Oracle internal JPRT system was once used to build and test the JDK. It has 
been superseded and can no longer be used to build/test mainline JDK. The 
support in the JDK code for running jobs in JPRT should be removed.


* About the change

The change touches several areas: build/make, hotspot, and jdk. I’m 
optimistically sending it out as a single webrev anyway. If it helps, I’m 
expecting the reviews to roughly map like so:

build-dev: make related files, jprt.*
hotspot-dev: {src,test}/hotspot
core-libs-dev: test/jdk  - I’m especially interested in hearing if the change 
to test/jdk/tools/lib/tests/Helper.java (to throw if the jmods directory is 
missing) is valid

Of course I’d welcome reviews across those mappings as well.

* Testing

tier1 (passed), tier2-3 still running (looking good so far).

Cheers,
Mikael

Re: RFR(S): 8211350: Remove jprt support

[David Holmes]

Looks good. Ship it!

David

On 3/10/2018 4:17 AM, Mikael Vidstedt wrote:


Thanks for the reviews. I’ve reverted the changes related to Helper and “just” 
adjusted the comments instead.

webrev:  
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01/open/webrev/
incremental (from webrev.00): 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01.incr/open/webrev/


Btw, I notice that "Test not run, NO jmods directory” will be printed twice 
when jmods is missing - once in Helper::newHelper and once in the methods calling 
it. In general, the handling of a null return from newHelper could use some clean 
up, but that is out of scope for this change.

Cheers,
Mikael


On Oct 2, 2018, at 8:40 AM, Mandy Chung  wrote:



On 10/2/18 12:21 AM, Mikael Vidstedt wrote:

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350 

webrev: http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/ 




test/jdk/tools/lib/tests/Helper.java is used by test/jdk/tools/jimage and 
test/jdk/tools/jlink tests which will be skipped, rather than failing, when 
running with images where jmods directory is not present (e.g. exploded image).

I think Helper class should continue to return null if jmods does not exist.  
test/jdk/tools/jimage/JImageTest.java should also be reverted.

Otherwise, the clean up looks good.

Mandy

Re: RFR(S): 8211350: Remove jprt support

[Alan Bateman]

On 02/10/2018 19:17, Mikael Vidstedt wrote:

Thanks for the reviews. I’ve reverted the changes related to Helper and “just” 
adjusted the comments instead.

webrev:  
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01/open/webrev/
incremental (from webrev.00): 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01.incr/open/webrev/


Btw, I notice that "Test not run, NO jmods directory” will be printed twice 
when jmods is missing - once in Helper::newHelper and once in the methods calling 
it. In general, the handling of a null return from newHelper could use some clean 
up, but that is out of scope for this change.

At some point I think the test for jlink will need to be cleaned up 
anyway but that is way outside of scope of what you are doing. The 
updated webrev looks okay to me.


-Alan

Re: RFR(S): 8211350: Remove jprt support

[Erik Joelsson]

Looks good to me.

/Erik


On 2018-10-02 11:17, Mikael Vidstedt wrote:

Thanks for the reviews. I’ve reverted the changes related to Helper and “just” 
adjusted the comments instead.

webrev:  
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01/open/webrev/
incremental (from webrev.00): 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01.incr/open/webrev/


Btw, I notice that "Test not run, NO jmods directory” will be printed twice 
when jmods is missing - once in Helper::newHelper and once in the methods calling 
it. In general, the handling of a null return from newHelper could use some clean 
up, but that is out of scope for this change.

Cheers,
Mikael


On Oct 2, 2018, at 8:40 AM, Mandy Chung  wrote:



On 10/2/18 12:21 AM, Mikael Vidstedt wrote:

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350 

webrev: http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/ 



test/jdk/tools/lib/tests/Helper.java is used by test/jdk/tools/jimage and 
test/jdk/tools/jlink tests which will be skipped, rather than failing, when 
running with images where jmods directory is not present (e.g. exploded image).

I think Helper class should continue to return null if jmods does not exist.  
test/jdk/tools/jimage/JImageTest.java should also be reverted.

Otherwise, the clean up looks good.

Mandy

Re: RFR(S): 8211350: Remove jprt support

[Mandy Chung]

+1

Mandy

On 10/2/18 11:17 AM, Mikael Vidstedt wrote:


Thanks for the reviews. I’ve reverted the changes related to Helper 
and “just” adjusted the comments instead.


webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01/open/webrev/ 

incremental (from webrev.00): 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01.incr/open/webrev/ 




Btw, I notice that "Test not run, NO jmods directory” will be printed 
twice when jmods is missing - once in Helper::newHelper and once in 
the methods calling it. In general, the handling of a null return from 
newHelper could use some clean up, but that is out of scope for this 
change.


Cheers,
Mikael

Re: RFR(S): 8211350: Remove jprt support

[Mikael Vidstedt]

Thanks for the reviews. I’ve reverted the changes related to Helper and “just” 
adjusted the comments instead.

webrev:  
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01/open/webrev/
incremental (from webrev.00): 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.01.incr/open/webrev/


Btw, I notice that "Test not run, NO jmods directory” will be printed twice 
when jmods is missing - once in Helper::newHelper and once in the methods 
calling it. In general, the handling of a null return from newHelper could use 
some clean up, but that is out of scope for this change.

Cheers,
Mikael

> On Oct 2, 2018, at 8:40 AM, Mandy Chung  wrote:
> 
> 
> 
> On 10/2/18 12:21 AM, Mikael Vidstedt wrote:
>> Please review this change which removes support for, and references to, the 
>> (Oracle internal) JPRT system.
>> 
>> bug: https://bugs.openjdk.java.net/browse/JDK-8211350 
>> 
>> webrev: 
>> http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/ 
>> 
>> 
> 
> test/jdk/tools/lib/tests/Helper.java is used by test/jdk/tools/jimage and 
> test/jdk/tools/jlink tests which will be skipped, rather than failing, when 
> running with images where jmods directory is not present (e.g. exploded 
> image).
> 
> I think Helper class should continue to return null if jmods does not exist.  
> test/jdk/tools/jimage/JImageTest.java should also be reverted.
> 
> Otherwise, the clean up looks good.
> 
> Mandy

Re: RFR(S): 8211350: Remove jprt support

[Erik Joelsson]

Build changes look good to me.

/Erik


On 2018-10-02 00:21, Mikael Vidstedt wrote:

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/

* Background (from the issue)

The Oracle internal JPRT system was once used to build and test the JDK. It has 
been superseded and can no longer be used to build/test mainline JDK. The 
support in the JDK code for running jobs in JPRT should be removed.


* About the change

The change touches several areas: build/make, hotspot, and jdk. I’m 
optimistically sending it out as a single webrev anyway. If it helps, I’m 
expecting the reviews to roughly map like so:

build-dev: make related files, jprt.*
hotspot-dev: {src,test}/hotspot
core-libs-dev: test/jdk  - I’m especially interested in hearing if the change 
to test/jdk/tools/lib/tests/Helper.java (to throw if the jmods directory is 
missing) is valid

Of course I’d welcome reviews across those mappings as well.

* Testing

tier1 (passed), tier2-3 still running (looking good so far).

Cheers,
Mikael

Re: RFR(S): 8211350: Remove jprt support

[Mandy Chung]

On 10/2/18 12:21 AM, Mikael Vidstedt wrote:

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/



test/jdk/tools/lib/tests/Helper.java is used by test/jdk/tools/jimage 
and test/jdk/tools/jlink tests which will be skipped, rather than 
failing, when running with images where jmods directory is not present 
(e.g. exploded image).


I think Helper class should continue to return null if jmods does not 
exist.  test/jdk/tools/jimage/JImageTest.java should also be reverted.


Otherwise, the clean up looks good.

Mandy

Re: RFR(S): 8211350: Remove jprt support

[David Holmes]

On 2/10/2018 6:34 PM, Alan Bateman wrote:

On 02/10/2018 08:21, Mikael Vidstedt wrote:
Please review this change which removes support for, and references 
to, the (Oracle internal) JPRT system.


bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/


Does the change to Helper.java and tests that use it break testing of 
exploded builds? I don't know why it has the comment "JPRT not yet 
ready" but whether $JAVA_HOME/jmods exists or not isn't connected to 
JPRT or any build or test system.


Yes it will break as now an exception will be thrown instead of just 
cleanly exiting with an error message. I think the Helper changes can be 
left as-is, just update the comment.


David


-Alan

Re: RFR(S): 8211350: Remove jprt support

[Alan Bateman]

On 02/10/2018 08:21, Mikael Vidstedt wrote:

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/

Does the change to Helper.java and tests that use it break testing of 
exploded builds? I don't know why it has the comment "JPRT not yet 
ready" but whether $JAVA_HOME/jmods exists or not isn't connected to 
JPRT or any build or test system.


-Alan

Re: RFR(S): 8211350: Remove jprt support

[David Holmes]

Hi Mikael,

This all looks fine to me.

Thanks for cleaning it up!

David

On 2/10/2018 5:21 PM, Mikael Vidstedt wrote:


Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/

* Background (from the issue)

The Oracle internal JPRT system was once used to build and test the JDK. It has 
been superseded and can no longer be used to build/test mainline JDK. The 
support in the JDK code for running jobs in JPRT should be removed.


* About the change

The change touches several areas: build/make, hotspot, and jdk. I’m 
optimistically sending it out as a single webrev anyway. If it helps, I’m 
expecting the reviews to roughly map like so:

build-dev: make related files, jprt.*
hotspot-dev: {src,test}/hotspot
core-libs-dev: test/jdk  - I’m especially interested in hearing if the change 
to test/jdk/tools/lib/tests/Helper.java (to throw if the jmods directory is 
missing) is valid

Of course I’d welcome reviews across those mappings as well.

* Testing

tier1 (passed), tier2-3 still running (looking good so far).

Cheers,
Mikael

RFR(S): 8211350: Remove jprt support

[Mikael Vidstedt]

Please review this change which removes support for, and references to, the 
(Oracle internal) JPRT system.

bug: https://bugs.openjdk.java.net/browse/JDK-8211350
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8211350/webrev.00/open/webrev/

* Background (from the issue)

The Oracle internal JPRT system was once used to build and test the JDK. It has 
been superseded and can no longer be used to build/test mainline JDK. The 
support in the JDK code for running jobs in JPRT should be removed.


* About the change

The change touches several areas: build/make, hotspot, and jdk. I’m 
optimistically sending it out as a single webrev anyway. If it helps, I’m 
expecting the reviews to roughly map like so:

build-dev: make related files, jprt.*
hotspot-dev: {src,test}/hotspot
core-libs-dev: test/jdk  - I’m especially interested in hearing if the change 
to test/jdk/tools/lib/tests/Helper.java (to throw if the jmods directory is 
missing) is valid

Of course I’d welcome reviews across those mappings as well.

* Testing

tier1 (passed), tier2-3 still running (looking good so far).

Cheers,
Mikael