8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
One of things that we are hoping to do in JDK 9 is to drop support for the IIOP transport from RMI connector in the JMX Remote API [1]. The motive is modules and the first step on this journey was to downgrade the support to optional from required (something that we did in JDK 8 as part of the Compact Profiles and Prepare for Modules efforts). We're not yet at the point yet where we can change the API docs and remove the implementation but we are the point where we need to build without the IIOP transport. Aside from our modularity efforts, it is useful to have it removed early in JDK 9 as this maximizes the time that anyone using it has to move another transport. This is the first time that I've added a configure option to the build so I may need help getting this right. The patch that I'm currently using is here: http://cr.openjdk.java.net/~alanb/8033366/webrev/ This adds the configure option --enable-rmiconnector-iiop to opt-in for including of the RMIConnector IIOP transport in the build. If the option is not specified (ie: the default) then the IIOP transport is left out. The only part that might need explanation is that SetRMICompilation generates the IIOP tie and stub classes if RUN_IIOP is set to anything. This is the reason why RUN_IIOP isn't just specified as $(RMICONNECTOR_IIOP). As regards the tests then we changed the JMX tests in JDK 8 so that they gracefully handle the case where the RMIConnector only supports the default transport. I did have to update one langtools test that directly references a generate stub classes. There isn't an replacement for this because there aren't any remaining individual classes excluded from compact3. The profiles content file keeps its reference to the _RMI* classes for now as this is needed for cases where someone does build with the IIOP transport. This will be re-visited later in JDK 9 of course. -Alan. [1] http://mail.openjdk.java.net/pipermail/jmx-dev/2014-January/000571.html
Re: 8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
Looks good to me. Only a minor comment on indentation in jdk/makeGenerateClasses.gmk. We try to keep block indentation at 2 spaces and continuation indentation at 4 spaces, so line 96 should only be 2 spaces. /Erik On 2014-02-06 13:06, Alan Bateman wrote: One of things that we are hoping to do in JDK 9 is to drop support for the IIOP transport from RMI connector in the JMX Remote API [1]. The motive is modules and the first step on this journey was to downgrade the support to optional from required (something that we did in JDK 8 as part of the Compact Profiles and Prepare for Modules efforts). We're not yet at the point yet where we can change the API docs and remove the implementation but we are the point where we need to build without the IIOP transport. Aside from our modularity efforts, it is useful to have it removed early in JDK 9 as this maximizes the time that anyone using it has to move another transport. This is the first time that I've added a configure option to the build so I may need help getting this right. The patch that I'm currently using is here: http://cr.openjdk.java.net/~alanb/8033366/webrev/ This adds the configure option --enable-rmiconnector-iiop to opt-in for including of the RMIConnector IIOP transport in the build. If the option is not specified (ie: the default) then the IIOP transport is left out. The only part that might need explanation is that SetRMICompilation generates the IIOP tie and stub classes if RUN_IIOP is set to anything. This is the reason why RUN_IIOP isn't just specified as $(RMICONNECTOR_IIOP). As regards the tests then we changed the JMX tests in JDK 8 so that they gracefully handle the case where the RMIConnector only supports the default transport. I did have to update one langtools test that directly references a generate stub classes. There isn't an replacement for this because there aren't any remaining individual classes excluded from compact3. The profiles content file keeps its reference to the _RMI* classes for now as this is needed for cases where someone does build with the IIOP transport. This will be re-visited later in JDK 9 of course. -Alan. [1] http://mail.openjdk.java.net/pipermail/jmx-dev/2014-January/000571.html
Re: 8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
On 06/02/2014 12:23, Erik Joelsson wrote: Looks good to me. Only a minor comment on indentation in jdk/makeGenerateClasses.gmk. We try to keep block indentation at 2 spaces and continuation indentation at 4 spaces, so line 96 should only be 2 spaces. Thanks Erik. I'll fix the indentation in GenerateClasses.gmk (and I assume CompileJavaClasses.gmk L48 too) before pushing this. -Alan
Re: 8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
On 2014-02-06 14:23, Alan Bateman wrote: On 06/02/2014 12:23, Erik Joelsson wrote: Looks good to me. Only a minor comment on indentation in jdk/makeGenerateClasses.gmk. We try to keep block indentation at 2 spaces and continuation indentation at 4 spaces, so line 96 should only be 2 spaces. Thanks Erik. I'll fix the indentation in GenerateClasses.gmk (and I assume CompileJavaClasses.gmk L48 too) before pushing this. Right, there too, thanks! /Erik
Re: code review round 0 for minor FDS makefile fix (8033714)
On 2/5/14 9:28 PM, David Holmes wrote: Hi Dan, Looks good to me. Thanks for the review! (I never run the install targets :( ) Neither do I and apparently neither does JPRT... That's how this slipped through the cracks... Dan Thanks, David On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
Re: 8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
On 2/6/2014 4:06 AM, Alan Bateman wrote: One of things that we are hoping to do in JDK 9 is to drop support for the IIOP transport from RMI connector in the JMX Remote API [1]. The motive is modules and the first step on this journey was to downgrade the support to optional from required (something that we did in JDK 8 as part of the Compact Profiles and Prepare for Modules efforts). We're not yet at the point yet where we can change the API docs and remove the implementation but we are the point where we need to build without the IIOP transport. Aside from our modularity efforts, it is useful to have it removed early in JDK 9 as this maximizes the time that anyone using it has to move another transport. This is the first time that I've added a configure option to the build so I may need help getting this right. The patch that I'm currently using is here: http://cr.openjdk.java.net/~alanb/8033366/webrev/ I also agree it's good to get this in early to maximize the time to make appropriate change. jdk/make/profile-rtjar-includes.txt references these classes. Would the profile build ignore if these classes don't exist? Mandy
Re: 8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
On 06/02/2014 16:00, Mandy Chung wrote: : jdk/make/profile-rtjar-includes.txt references these classes. Would the profile build ignore if these classes don't exist? Yes, it's just used an exclude so it harmless when the classes aren't present. Once we come to removing this feature then the exclude can be removed from the profiles build too. -Alan
Re: code review round 0 for minor FDS makefile fix (8033714)
Looks good to me too. Thanks for fixing this. tom On Feb 6, 2014, at 6:07 AM, Daniel D. Daugherty daniel.daughe...@oracle.com wrote: On 2/5/14 9:28 PM, David Holmes wrote: Hi Dan, Looks good to me. Thanks for the review! (I never run the install targets :( ) Neither do I and apparently neither does JPRT... That's how this slipped through the cracks... Dan Thanks, David On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
Re: AWT Dev RFR: Allow using a system installed libpng
- Original Message - * Andrew Hughes gnu.and...@redhat.com [2014-02-04 19:26]: On 2014-02-03 18:43, Omair Majid wrote: The following webrevs modify the build system to allow building against the system-installed copy of libpng as well as using the bundled copy of libpng ROOT: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01/ JDK: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01-jdk/ A new configure option `--with-libpng` is introduced which defaults to `bundled` but can be set to `system` to use the system-installed copy of libpng. In this respect, why does this not just use AC_ARG_ENABLE as there are only two options here (system libpng or bundled libpng)? Also, this patch hardcodes the libpng cflags and ldflags when these should be obtained from pkg-config. This is how these values are obtained in IcedTea and using this patch would thus be a regression. The answer to both of these is the same: I followed how the existing code handles zlib. If this needs fixing, then it needs to be fixed in the other cases (zlib and giflib) too. Yes (well, giflib doesn't have pkg-config but otherwise...). We said this last time this work was discussed, so I'm a little surprised to see this patch being posted in the same form again. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681 -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: 248BDC07 (https://keys.indymedia.org/) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
Re: code review round 0 for minor FDS makefile fix (8033714)
On 2/6/14 9:29 AM, Doug Simon wrote: Not sure if I’m being asked for a review, but if so, looks good. Yes, I was looking for a review. In particular because I tweaked your original fix... Thanks for the review! Dan On Feb 6, 2014, at 5:07 PM, Tom Rodriguez tom.rodrig...@oracle.com wrote: Looks good to me too. Thanks for fixing this. tom On Feb 6, 2014, at 6:07 AM, Daniel D. Daugherty daniel.daughe...@oracle.com wrote: On 2/5/14 9:28 PM, David Holmes wrote: Hi Dan, Looks good to me. Thanks for the review! (I never run the install targets :( ) Neither do I and apparently neither does JPRT... That's how this slipped through the cracks... Dan Thanks, David On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
Re: code review round 0 for minor FDS makefile fix (8033714)
Not sure if I’m being asked for a review, but if so, looks good. On Feb 6, 2014, at 5:07 PM, Tom Rodriguez tom.rodrig...@oracle.com wrote: Looks good to me too. Thanks for fixing this. tom On Feb 6, 2014, at 6:07 AM, Daniel D. Daugherty daniel.daughe...@oracle.com wrote: On 2/5/14 9:28 PM, David Holmes wrote: Hi Dan, Looks good to me. Thanks for the review! (I never run the install targets :( ) Neither do I and apparently neither does JPRT... That's how this slipped through the cracks... Dan Thanks, David On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
Re: code review round 0 for minor FDS makefile fix (8033714)
On 02/ 6/14 08:32 AM, Daniel D. Daugherty wrote: Looks good to me, Dan Tim On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
Re: code review round 0 for minor FDS makefile fix (8033714)
Thanks for the review! Dan On 2/6/14 9:53 AM, Tim Bell wrote: On 02/ 6/14 08:32 AM, Daniel D. Daugherty wrote: Looks good to me, Dan Tim On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote: This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
Re: 8033366: Add configure option to allow RMIConnector IIOP transport be selected compiled in or not
On 6 feb 2014, at 13:06, Alan Bateman alan.bate...@oracle.com wrote: This is the first time that I've added a configure option to the build so I may need help getting this right. The patch that I'm currently using is here: http://cr.openjdk.java.net/~alanb/8033366/webrev/ It looks basically good. Some comments: In spec.gmk.in: +RMICONNECTOR_IIOP=@RMICONNECTOR_IIOP@ Please use := as = has a special meaning in make (late evaluation). Simple assignment is :=. In jdk-options.m4: Please add a set of AC_MSG_CHECKING/RESULT for presenting the value of the flag. You can find examples in the code. Otherwise it looks fine. /Magnus
