Re: RFR: Allow using a system installed libpng
Hi Andrew, "Client code" is basically anything in 2D, AWT, i18n, beans, a11y, ImageIO, Sound, or Swing. I.e., anything related to GUI/desktop. In this particular case Omair is changing the SplashScreen code which belongs to AWT, hence the choice of the client repo for integration. -- best regards, Anthony On 2/20/2014 7:30 PM, Andrew Hughes wrote: - Original Message - Hi Omair, Should I be pushing this to jdk9/dev ? (Or to jdk9/client?) If you change client code, then the fix should go to the client repo to avoid merge conflicts and allow for more manual testing prior to integrating the changes into the master repo. Perhaps this would be an appropriate point to clarify what constitutes 'client code'? -- best regards, Anthony On 2/19/2014 8:16 PM, Omair Majid wrote: * Magnus Ihse Bursie [2014-02-19 06:59]: On 2014-02-17 10:16, Erik Joelsson wrote: At least to me this looks good, but better let Magnus and Andrew have their say too. Looks good to me! Thanks for the reviews, everyone! I have filed https://bugs.openjdk.java.net/browse/JDK-8035341 to track the issue. Can someone please point me some docs that explains what I need to do to to this bug once I have pushed the fix? Should I be pushing this to jdk9/dev ? (Or to jdk9/client?) Thanks, Omair Thanks,
Re: RFR: Allow using a system installed libpng
On 21/02/2014 11:43, Anthony Petrov wrote: Hi Andrew, "Client code" is basically anything in 2D, AWT, i18n, beans, a11y, ImageIO, Sound, or Swing. I.e., anything related to GUI/desktop. In this particular case Omair is changing the SplashScreen code which belongs to AWT, hence the choice of the client repo for integration. Hopefully in time it will be possible to drop jdk9/client and have the changes to the client libraries pushed to jdk9/dev along with everything else. This would make things a lot simpler to understand and would also ensure that jdk9/dev always has the latest bits. I would expect it would reduce the integration effort too. -Alan.
Re: RFR: Allow using a system-installed lcms2
On 2014-02-20 23:40, Omair Majid wrote: Hi, The following is a preliminary webrev that allows OpenJDK to build and run against a system-installed copy of lcms2 rather than the copy bundled with OpenJDK: root: http://cr.openjdk.java.net/~omajid/webrevs/system-lcms/00/ jdk: http://cr.openjdk.java.net/~omajid/webrevs/system-lcms/00-jdk/ The goal is to add a new option `--with-lcms=` with possible values `bundled` or `system`. Using `--with-lcms=system` builds using the system-installed copy of lcms2 while `--with-lcms=bundled` builds with the bundled copy of lcms2 in OpenJDK. This patch is quite a bit more invasive than the libpng one [1]. There are a few issues that came up: 1. The sources for the bundled library are contained next to OpenJDK-specific sources. This is not true for the bundled copies of zlib, libpng and giflib. On the other hand, the jpeg code in OpenJDK also mixes OpenJDK-specific code with bundled libjpeg code. To make it easier to see (and compile) only non-lcms2 code, I moved the lcms2-specific code into a separate directory. There are no code changes there. The only OpenJDK-specific file here is LCMS.c, right? I made changes which made sense to me, but I am not sure how this fits in with existing conventions. Perhaps people here have suggestions on how to make this less invasive and still achieve the goal while keep things separate and distinct? I think it makes sense in separating our own code and an imported library. I think the "j2" convention is reasonable. I think the --with-lcms option is reasonable. So, the parts seems to work out fine. Still there's something bothering me with this fix, that I can't really put my finger on. Let's hear what the 2D people are saying. If they don't object, I won't object. But I like the way you're working on cleaning up our relationship to our bundled libraries! /Magnus
RFR: JDK-8035495 Improvements in autoconf integration
This bug covers several small improvements in how our configure eco-system integrates with autoconf. More specifically: * Fix so config.status --recheck works. * Let the top-level configure script determine TOPDIR, and pass it on. * Clean up usage regarding TOPDIR, SRC_ROOT and AUTOCONF_DIR. * Prohibit direct execution of common/autoconf/configure; always use configure in top-level dir. * Add "do not edit" header to generated-configure.sh * Add output on how we determine where to store the configuration. * Improve output from autogen.sh, including autoconf version. * Unify duplicated code in autogen.sh Bug: https://bugs.openjdk.java.net/browse/JDK-8035495 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8035495-improvements-in-autoconf-integration/webrev.02 /Magnus
Re: RFR: Allow using a system-installed lcms2
* Magnus Ihse Bursie [2014-02-21 08:21]: > On 2014-02-20 23:40, Omair Majid wrote: > >1. The sources for the bundled library are contained next to > >OpenJDK-specific sources. This is not true for the bundled copies of > >zlib, libpng and giflib. On the other hand, the jpeg code in OpenJDK > >also mixes OpenJDK-specific code with bundled libjpeg code. To make it > >easier to see (and compile) only non-lcms2 code, I moved the > >lcms2-specific code into a separate directory. There are no code changes > >there. > The only OpenJDK-specific file here is LCMS.c, right? Yes, that's the only one I could identify. > >I made changes which made sense to me, but I am not sure how this fits > >in with existing conventions. Perhaps people here have suggestions on > >how to make this less invasive and still achieve the goal while keep > >things separate and distinct? > > I think it makes sense in separating our own code and an imported library. Just throwing an idea out there, but has anyone considered moving all bundled code to a separate location? As in the `jdk` repo would have a `libs` or `bundled-libs` directory with one directory for each project with pristine upstream source (or a fork of upstream, as appropriate). > I think the "j2" convention is reasonable. > I think the --with-lcms option is reasonable. > > So, the parts seems to work out fine. Still there's something > bothering me with this fix, that I can't really put my finger on. > Let's hear what the 2D people are saying. If they don't object, I > won't object. Cool. But if you do have any concerns, please do share them. Even if the current patch is okay, I think everyone would like to see a future patch that is even better :) > But I like the way you're working on cleaning up our relationship to > our bundled libraries! Thanks. I am happy to find out that such patches are desired in OpenJDK! My original motivation was distributions asking to unbundle libraries. It's nice to see that everyone benefits from this :) Cheers, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: RFR: Allow using a system-installed lcms2
* Andrew Hughes [2014-02-21 10:36]: > > I think the "j2" convention is reasonable. > > This is where we disagree. I may have agreed if this was new, but we've been > using libjavalcms.so for the lifetime of 7 and I see no reason to change this. > > Also, "j2" seems pretty meaningless. I had the same thoughts at first. But I asked this same question [1] and was informed that the 'j2' is for "java to" [2]. So this is the "java to lcms" bridge. Thanks, Omair [1] http://mail.openjdk.java.net/pipermail/build-dev/2013-December/011274.html [2] http://mail.openjdk.java.net/pipermail/build-dev/2013-December/011325.html -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: RFR: Allow using a system-installed lcms2
- Original Message - > On 2014-02-20 23:40, Omair Majid wrote: > > Hi, > > > > The following is a preliminary webrev that allows OpenJDK to build and > > run against a system-installed copy of lcms2 rather than the copy > > bundled with OpenJDK: > > > > root: http://cr.openjdk.java.net/~omajid/webrevs/system-lcms/00/ > > jdk: http://cr.openjdk.java.net/~omajid/webrevs/system-lcms/00-jdk/ > > > > The goal is to add a new option `--with-lcms=` with possible values > > `bundled` or `system`. Using `--with-lcms=system` builds using the > > system-installed copy of lcms2 while `--with-lcms=bundled` builds with > > the bundled copy of lcms2 in OpenJDK. > > > > This patch is quite a bit more invasive than the libpng one [1]. There > > are a few issues that came up: > > > > 1. The sources for the bundled library are contained next to > > OpenJDK-specific sources. This is not true for the bundled copies of > > zlib, libpng and giflib. On the other hand, the jpeg code in OpenJDK > > also mixes OpenJDK-specific code with bundled libjpeg code. To make it > > easier to see (and compile) only non-lcms2 code, I moved the > > lcms2-specific code into a separate directory. There are no code changes > > there. > The only OpenJDK-specific file here is LCMS.c, right? Yes. At least, it was true in 7: http://icedtea.classpath.org/hg/icedtea7-forest/jdk/file/tip/make/sun/cmm/lcms/FILES_c_unix.gmk I don't know why we didn't move it to a sub-directory then, like we did with JPEG. I suspect simply because LCMS was done first. > > > I made changes which made sense to me, but I am not sure how this fits > > in with existing conventions. Perhaps people here have suggestions on > > how to make this less invasive and still achieve the goal while keep > > things separate and distinct? > > I think it makes sense in separating our own code and an imported library. So do I. It makes deleting the bundled copy more maintainable. > I think the "j2" convention is reasonable. This is where we disagree. I may have agreed if this was new, but we've been using libjavalcms.so for the lifetime of 7 and I see no reason to change this. Also, "j2" seems pretty meaningless. > I think the --with-lcms option is reasonable. In keeping with convention, yes. I'd still like to change these to --enable in the long run. > > So, the parts seems to work out fine. Still there's something bothering > me with this fix, that I can't really put my finger on. Let's hear what > the 2D people are saying. If they don't object, I won't object. > This is pretty much the same as we've been carrying since 2011, but modified for the new build, so I don't see any reason for objection. > But I like the way you're working on cleaning up our relationship to our > bundled libraries! It's necessary for us and we've had this work since 2007. Of course, this new build system broke most of it and meant it had to be rewritten :( > > /Magnus > -- 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: RFR: JDK-8035495 Improvements in autoconf integration
Hi Magnus: This bug covers several small improvements in how our configure eco-system integrates with autoconf. More specifically: * Fix so config.status --recheck works. * Let the top-level configure script determine TOPDIR, and pass it on. * Clean up usage regarding TOPDIR, SRC_ROOT and AUTOCONF_DIR. * Prohibit direct execution of common/autoconf/configure; always use configure in top-level dir. * Add "do not edit" header to generated-configure.sh * Add output on how we determine where to store the configuration. * Improve output from autogen.sh, including autoconf version. * Unify duplicated code in autogen.sh Bug: https://bugs.openjdk.java.net/browse/JDK-8035495 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8035495-improvements-in-autoconf-integration/webrev.02 Looks good to me. Thanks for adding the 'do not edit' banners. Tim
Re: RFR: Allow using a system installed libpng
Added 2D Victor On 21.02.2014 16:00, Alan Bateman wrote: On 21/02/2014 11:43, Anthony Petrov wrote: Hi Andrew, "Client code" is basically anything in 2D, AWT, i18n, beans, a11y, ImageIO, Sound, or Swing. I.e., anything related to GUI/desktop. In this particular case Omair is changing the SplashScreen code which belongs to AWT, hence the choice of the client repo for integration. Hopefully in time it will be possible to drop jdk9/client and have the changes to the client libraries pushed to jdk9/dev along with everything else. This would make things a lot simpler to understand and would also ensure that jdk9/dev always has the latest bits. I would expect it would reduce the integration effort too. -Alan.
Re: RFR: JDK-8035495 Improvements in autoconf integration
Looks good to me. Mike On Feb 21 2014, at 05:31 , Magnus Ihse Bursie wrote: > This bug covers several small improvements in how our configure eco-system > integrates with autoconf. More specifically: > > * Fix so config.status --recheck works. > * Let the top-level configure script determine TOPDIR, and pass it on. > * Clean up usage regarding TOPDIR, SRC_ROOT and AUTOCONF_DIR. > * Prohibit direct execution of common/autoconf/configure; always use > configure in top-level dir. > * Add "do not edit" header to generated-configure.sh > * Add output on how we determine where to store the configuration. > * Improve output from autogen.sh, including autoconf version. > * Unify duplicated code in autogen.sh > > Bug: https://bugs.openjdk.java.net/browse/JDK-8035495 > WebRev: > http://cr.openjdk.java.net/~ihse/JDK-8035495-improvements-in-autoconf-integration/webrev.02 > > /Magnus
Re: RFR: JDK-8034788 Rewrite toolchain.m4 to support multiple toolchains per platform
I say ship it given you have tested it, and I don't see anything raise a flag. But I am not a JDK reviewer. Cheers, Henry - Original Message - From: magnus.ihse.bur...@oracle.com To: erik.joels...@oracle.com Cc: build-dev@openjdk.java.net Sent: Thursday, February 20, 2014 6:46:44 AM GMT -08:00 US/Canada Pacific Subject: Re: RFR: JDK-8034788 Rewrite toolchain.m4 to support multiple toolchains per platform On 2014-02-13 22:27, Magnus Ihse Bursie wrote: > > It turned out that it was not a good idea to line break AC_MSG_* > functions. :-( I didn't verify that properly before; my bad. > > Here's a new-new review which restores the old long (but > properly-looking) AC_MSG lines. > > (If you're curious, the line break was printed verbatim. Putting a > trailing \ did remove the line break, but then the indentation level > showed up as verbatim spaces in the output.) > > WebRev: > http://cr.openjdk.java.net/~ihse/JDK-8034788-rewrite-toolchain-m4/webrev.03 Now I have finally really put this change to the test: building with and without the patch on all platforms and running the compare script, to compare the build result with and without the patch. Of course, I found some bugs that I have missed. On the upside, now I am *really* confident in this patch. I'm running a final round of confirmation tests (to make sure my latest fixes didn't cause any other breakage) but I'd be surprised if there turned up anything. Given that the remaining test round is green, I'd like to get a final round of ack's from the reviewers. Unfortunately, webrev can't provide easy differential reviews, but the changes since last time are: * Added AC_SUBST(TOOLCHAIN_TYPE) in TOOLCHAIN_DETERMINE_TOOLCHAIN_TYPE. (oops! Thank's Erik for that one) * Fixed a typo in LIB_SETUP_STATIC_LINK_LIBSTDCPP in libraries.m4, $TOOLCHAIn_TYPE was not found so we got LIBCXX wrong on macosx. * On Solaris, $TR a-z A-Z does not work as expected. Replaced with longer but safer version when deriving OPENJDK_TARGET_OS_UPPERCASE. * For solstudio, the CC_HIGHEST setting was missing spaces between some arguments, making -fsimple-fsingle-xalias_level... look like a single, invalid argument. * The compiler version string for solstudio used [...] in sed instead of @<:@...@:>@. But m4 eats the [] so we need this ugly escaping. Not the first time I forget that one. :-/ * The recent reconfigure patch was slightly incorrect, we shouldn't put "..." around the command line arguments when calling configure in the makefile. (Can't see how that one got through all my testing :-( but failed at the very first real-world test.) WebRev: http://cr.openjdk.java.net/~ihse/JDK-8034788-rewrite-toolchain-m4/webrev.04 /Magnus
