Thanks everybody for the reviews. If nobody raises a "Veto" (Phil?) I plan to push this fix tomorrow in its current form.
I've also run it through the submit repo and got an error on Windows for the test "runtime/modules/JVMDefineModule.java" which seems completely unrelated to my change which only touches the X11 implementation on Unix. Can somebody please confirm that? [Mach5] mach5-one-simonis-JDK-8213944-20181120-1629-11082: FAILED, Failed tests: 1 runtime/modules/JVMDefineModule.java tier1 windows-x64-debug othervm driver ExitCode: -1073741819 Mach5 Tasks Results Summary UNABLE_TO_RUN: 0 FAILED: 0 EXECUTED_WITH_FAILURE: 1 KILLED: 0 PASSED: 75 NA: 0 Thank you and best regards, Volker On Tue, Nov 20, 2018 at 12:05 PM Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> wrote: > > > > On 2018-11-19 18:56, Volker Simonis wrote: > > Hi Phil, > > > > I'd like to kindly ask you to suggest how we can proceed with this issue. > > > > As I wrote before, Xrandr is not officially supported on AIX and there > > are no official packages available for it. There are some OpenSource > > sites for AIX which provide Xrandr, but they are all not compatible > > with the default native libraries (e.g. the open source Xrandr package > > depends on another open source package of Xrender.so.1 but the system > > only provides Xrender.so.0 natively). We can't compile the whole JDK > > (i.e. libawt_xawt.so) against some open source package of Xrender.so.1 > > because that simply won't be available on the majority of systems. > > > > Remember that forcing people to install these open-source packages > > even as a build dependency is like expecting Linux users to install > > some non-official packages just to be able to build. Especially in > > corporate environments that's not easy. Moreover the benefit would be > > really minimal, because the Xrandr functionality won't be available at > > runtime anyway. > > > > So to cut a long story short, I see two options: > > > > 1. Go with my current patch (ugly but efficient) > > > > 2. Check-in in an AIX specific version of XRander.h/randr.h under > > src/java.desktop/aix (OK for me, but that would actually negate the > > initial purpose of "8210863: Remove Xrandr include files") > > > > Do you have a better proposal? > I think the change look good, and I vote for strategy 1. As Thomas > suggested, if the AIX ifdefs look bad we can create a new define, but > I'm not sure that's really helpful - after all, it's just on AIX we > currently have no r&r. Having a define would mostly be needed if it was > multiple OSes, or similar more complex situations, that would have/not > have the r&r extension. > > Yet another solution, to get rid of the ifdefs, is to move the relevant > Xranrd dependent functions into a new, separate file, like > awt_GraphicsEnv_randr.c, and then in the build exclude it on AIX (or, > perhaps if it's worth the trouble, on all platforms where configure did > not find Xrandr). > > /Magnus > > > > > Thank you and best regards, > > Volker > > > > On Fri, Nov 16, 2018 at 11:22 AM Volker Simonis > > <volker.simo...@gmail.com> wrote: > >> On Thu, Nov 15, 2018 at 6:01 PM Philip Race <philip.r...@oracle.com> wrote: > >>> PS I am not sure why xrandr headers would not be available for AIX. > >>> They are a standard part of the xdistribution. > >>> > >> I'm not an X11 guru, but as far as I understand, xrandr is an > >> extension and as such it doesn't have to be supported by every > >> implementation. To the best of my knowledge (I've just started another > >> poll among some experts) AIX doesn't support Xrandr and does not have > >> the corresponding headers. > >> > >>> If true, think what you are going to have to do is add a > >>> --with-xrandr-include option > >>> and provide it that way. > >>> > >> What if there are no standard Xrandr headers on a platform? Do you > >> really want to force users to get them from some dubious sources just > >> for building the OpenJDK? Sorry, but I don't think that's a good > >> solution. Than I'd rather prefer the ugly ifdefs (or I check the > >> headers back in again in an AIX-specific directory :) > >> > >> Thank you and best regards, > >> Volker > >> > >>> -phil. > >>> > >>> On 11/15/18, 8:55 AM, Philip Race wrote: > >>>> Hmm. I don't like the ifdefs. > >>>> > >>>> Xrandr is a requirement for the build. If its not there at runtime > >>>> that's OK. > >>>> > >>>> -phil. > >>>> > >>>> On 11/15/18, 8:06 AM, Volker Simonis wrote: > >>>>> Hi, > >>>>> > >>>>> can I please have a review for the following small change: > >>>>> > >>>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/ > >>>>> https://bugs.openjdk.java.net/browse/JDK-8213944 > >>>>> > >>>>> Change JDK-8210863 removed the Xrandr.h/randr.h headers from the > >>>>> OpenJDK sources but forgot to add a configure check for the Xrandr > >>>>> extension which is now a build dependency. > >>>>> > >>>>> The change also broke the AIX build. AIX never supported Xrandr, but > >>>>> that was only detected at runtime, when the JDK was unable to > >>>>> dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the > >>>>> source tree any more, we have to conditionally compile some parts of > >>>>> src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such > >>>>> that it doesn't require the definitions and declarations from > >>>>> Xrandr.h/randr.h any more. > >>>>> > >>>>> Thank you and best regards, > >>>>> Volker >
