[patch] Allow compiling against newer X11 headers

[Mark Wielaard-4]

Hi,

Recent X11 headers have reorganized some shm related constants and
structs making it necessary to patch around how awt_GraphicsEnv.h does
some things. Fedora has included this patch for some time now, and
recently several people on this list asked about it.

I added a configure check to see whether or not it is necessary to apply
the patch. This makes icedtea6 build out of the box on older (fedora 11)
and newer (fedora 12/rawhide) systems.

2009-11-02  Mark Wielaard  <mjw@...>

    * configure.ac: Check whether the new X11/extensions/shmproto.h
    header is available.
    * Makefile.am: Add patches/icedtea-xshm.patch if new header
    detected.
    * patches/icedtea-xshm.patch: New patch.

Let me know if it looks sane and I will update Hacking and the wiki with
the info.

Thanks,

Mark

diff -r 547923046557 Makefile.am
--- a/Makefile.am Fri Oct 30 13:35:54 2009 +0000
+++ b/Makefile.am Mon Nov 02 15:00:27 2009 +0100
@@ -632,6 +632,10 @@
    patches/icedtea-nss-6763530.patch
 endif
 
+if HAS_SHMPROTO_H
+ICEDTEA_PATCHES += patches/icedtea-xshm.patch
+endif
+
 if WITH_ALT_HSBUILD
 ICEDTEA_PATCHES += patches/hotspot/$(HSBUILD)/openjdk-6886353-ignore_deoptimizealot.patch \
  patches/hotspot/$(HSBUILD)/zero.patch
diff -r 547923046557 configure.ac
--- a/configure.ac Fri Oct 30 13:35:54 2009 +0000
+++ b/configure.ac Mon Nov 02 15:00:27 2009 +0100
@@ -330,6 +330,15 @@
   fi
 fi
 
+# define HAS_SHMPROTO_H to indicate we need to apply a patch to get new
+# definitions for awt_GraphicsEnv.h that aren't available anymore in older
+# headers.
+AC_CHECK_HEADER([X11/extensions/shmproto.h],
+                [has_shmproto_h='yes'], [has_shmproto_h='no'],
+                [#include <sys/shm.h>
+                 #include <X11/Xmd.h>])
+AM_CONDITIONAL(HAS_SHMPROTO_H, test "x${has_shmproto_h}" = "xyes")
+
 if test "x${ENABLE_SYSTEMTAP}" = xyes; then
 AC_CHECK_HEADER([sys/sdt.h], [SDT_H_FOUND='yes'],
                 [SDT_H_FOUND='no';
diff -r 547923046557 patches/icedtea-xshm.patch
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/patches/icedtea-xshm.patch Mon Nov 02 15:00:27 2009 +0100
@@ -0,0 +1,29 @@
+--- old/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.h 2009-07-17 10:36:09.000000000 -0400
++++ openjdk/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.h 2009-07-30 13:25:52.000000000 -0400
+@@ -41,7 +41,10 @@
+
+ #include <sys/ipc.h>
+ #include <sys/shm.h>
+-#include <X11/extensions/XShm.h>
++#include <X11/Xmd.h>
++#include <X11/extensions/shmproto.h>
++
++#define ShmSeg CARD32
+
+ extern int XShmQueryExtension();
+
+@@ -49,6 +52,14 @@
+ void resetXShmAttachFailed();
+ jboolean isXShmAttachFailed();
+
++typedef struct {
++    ShmSeg shmseg;     /* resource id */
++    int shmid;         /* kernel id */
++    char *shmaddr;     /* address in client */
++    Bool readOnly;     /* how the server should attach it */
++} XShmSegmentInfo;
++
++#undef ShmSeg
+ #endif /* MITSHM */
+
+ /* fieldIDs for X11GraphicsConfig fields that may be accessed from C */

[Vlastimil Babka-2]

Mark Wielaard wrote:
> I added a configure check to see whether or not it is necessary to apply
> the patch. This makes icedtea6 build out of the box on older (fedora 11)
> and newer (fedora 12/rawhide) systems.
>
> Let me know if it looks sane and I will update Hacking and the wiki with
> the info.

Hi,

I was solving this problem in gentoo recently and I found the fedora
patch a bit dangerous as it defines a structure from XShm.h manually.
I've created another patch that is applied not to awt_GraphicsEnv.h but
awt_GraphicsEnv.c (where the missing constant is actually used) and it
just includes Xmd.h and shmproto.h to get the constant (surrounded by a
MITSHM ifdef, as is the place of the actual constant usage). The
awt_GraphicsEnv.h still includes Xshm.h with the struct that Fedora
patch defines manually.
But I have to admit I'm not an expert on this, maybe there were reasons
behind the Fedora patch I don't know, but this one seems to work. I'm
attaching it.

I however didn't create configure check (mostly for lack of skills there
:), we apply the patch conditionally in the gentoo ebuild for now.
But I believe the logic of the check should go like this:
- check if libXext (xext in pkg-config) is of version >= 1.1.1 - if it
is, we know that we need also the shmproto.h
- if yes, check that shmproto.h is present (or that xextproto is of
version >=7.1.1), fail if it isn't

If we check just for the existence of shmproto.h, it will fail in case
libXext is of the new version (and the constant is no longer in XShm.h)
but there is no xextproto and thus shmproto.h (it's only a build dep
AFAIK so might not be present on all systems?).

Hope it helps,
Vlastimil

--- /dev/null 2009-10-29 20:00:29.463258316 +0100
+++ patches/icedtea-shmproto.patch 2009-10-29 21:02:36.000000000 +0100
@@ -0,0 +1,13 @@
+--- openjdk/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.c.orig 2009-10-29 21:01:01.000000000 +0100
++++ openjdk/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.c 2009-10-29 21:01:56.000000000 +0100
+@@ -45,6 +45,10 @@
+ #include <stdlib.h>
+
+ #include "awt_GraphicsEnv.h"
++#ifdef MITSHM
++#include <X11/Xmd.h>
++#include <X11/extensions/shmproto.h>
++#endif
+ #include "awt_Window.h"
+ #include "awt_util.h"
+ #include "gdefs.h"
--- Makefile.am.orig 2009-10-29 21:35:23.000000000 +0100
+++ Makefile.am 2009-10-29 21:35:50.000000000 +0100
@@ -708,6 +708,7 @@
  patches/icedtea-explicit-target-arch.patch \
  patches/openjdk/6648816.patch \
  patches/openjdk/oj100103-debugger-socket-overflow.patch \
+ patches/icedtea-shmproto.patch \
  $(DISTRIBUTION_PATCHES)
 
 stamps/extract.stamp: stamps/download.stamp

[gnu_andrew]

2009/11/2 Vlastimil Babka <caster@...>:

> Mark Wielaard wrote:
>> I added a configure check to see whether or not it is necessary to apply
>> the patch. This makes icedtea6 build out of the box on older (fedora 11)
>> and newer (fedora 12/rawhide) systems.
>>
>> Let me know if it looks sane and I will update Hacking and the wiki with
>> the info.
>
> Hi,
>
> I was solving this problem in gentoo recently and I found the fedora
> patch a bit dangerous as it defines a structure from XShm.h manually.
> I've created another patch that is applied not to awt_GraphicsEnv.h but
> awt_GraphicsEnv.c (where the missing constant is actually used) and it
> just includes Xmd.h and shmproto.h to get the constant (surrounded by a
> MITSHM ifdef, as is the place of the actual constant usage). The
> awt_GraphicsEnv.h still includes Xshm.h with the struct that Fedora
> patch defines manually.
> But I have to admit I'm not an expert on this, maybe there were reasons
> behind the Fedora patch I don't know, but this one seems to work. I'm
> attaching it.
>
> I however didn't create configure check (mostly for lack of skills there
> :), we apply the patch conditionally in the gentoo ebuild for now.
> But I believe the logic of the check should go like this:
> - check if libXext (xext in pkg-config) is of version >= 1.1.1 - if it
> is, we know that we need also the shmproto.h
> - if yes, check that shmproto.h is present (or that xextproto is of
> version >=7.1.1), fail if it isn't
>
> If we check just for the existence of shmproto.h, it will fail in case
> libXext is of the new version (and the constant is no longer in XShm.h)
> but there is no xextproto and thus shmproto.h (it's only a build dep
> AFAIK so might not be present on all systems?).
>
> Hope it helps,
> Vlastimil
>
> --- /dev/null   2009-10-29 20:00:29.463258316 +0100
> +++ patches/icedtea-shmproto.patch      2009-10-29 21:02:36.000000000 +0100
> @@ -0,0 +1,13 @@
> +--- openjdk/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.c.orig      2009-10-29 21:01:01.000000000 +0100
> ++++ openjdk/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.c   2009-10-29 21:01:56.000000000 +0100
> +@@ -45,6 +45,10 @@
> + #include <stdlib.h>
> +
> + #include "awt_GraphicsEnv.h"
> ++#ifdef MITSHM
> ++#include <X11/Xmd.h>
> ++#include <X11/extensions/shmproto.h>
> ++#endif
> + #include "awt_Window.h"
> + #include "awt_util.h"
> + #include "gdefs.h"
> --- Makefile.am.orig    2009-10-29 21:35:23.000000000 +0100
> +++ Makefile.am 2009-10-29 21:35:50.000000000 +0100
> @@ -708,6 +708,7 @@
>        patches/icedtea-explicit-target-arch.patch \
>        patches/openjdk/6648816.patch \
>        patches/openjdk/oj100103-debugger-socket-overflow.patch \
> +       patches/icedtea-shmproto.patch \
>        $(DISTRIBUTION_PATCHES)
>
>  stamps/extract.stamp: stamps/download.stamp
>
>

Hi Mark, Vlastimil,

Having seen both patches before, I'd personally prefer we just added
the necessary headers if that works, rather than the additional cruft
we have in the Fedora patch.  We should do that in awt_GraphicsEnv.h
though, as in the Fedora patch, which would also remove the need for
the #ifdef MITSHM (it's in awt_GraphicsEnv.h already).  So basically
the Fedora patch but without the removal of the XShm.h include and the
additional structs.

Though I've been aware of this for a while, I deliberately haven't
upgraded my own build systems to the new version because it will break
OpenJDK builds for which we don't yet have a solution.  Does anyone
know of some versioning defines in the X headers that would avoid us
having to use a configure check, and thus allow the patch to be
submitted upstream? I believe the change
(http://lists.x.org/archives/xorg-devel/2009-June/001242.html) appears
in libXext 1.1.1 as Vlastimil says, but is there a way of detecting
that other than with autoconf?
--
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8

[Vlastimil Babka-2]

Andrew John Hughes wrote:
 > Hi Mark, Vlastimil,

Hi Andrew,

> Having seen both patches before, I'd personally prefer we just added
> the necessary headers if that works, rather than the additional cruft
> we have in the Fedora patch.  We should do that in awt_GraphicsEnv.h
> though, as in the Fedora patch, which would also remove the need for
> the #ifdef MITSHM (it's in awt_GraphicsEnv.h already).  So basically
> the Fedora patch but without the removal of the XShm.h include and the
> additional structs.

Sure, why not. I just included it in the .c file because the .h file
didn't use the constant and I hoped to minimize potential symbol
collisions or something (suspected something like that could be the
reason for the weird Fedora patch).

> Though I've been aware of this for a while, I deliberately haven't
> upgraded my own build systems to the new version because it will break
> OpenJDK builds for which we don't yet have a solution.  Does anyone
> know of some versioning defines in the X headers that would avoid us
> having to use a configure check, and thus allow the patch to be
> submitted upstream? I believe the change
> (http://lists.x.org/archives/xorg-devel/2009-June/001242.html) appears
> in libXext 1.1.1 as Vlastimil says, but is there a way of detecting
> that other than with autoconf?

I don't know about versioning defines, but maybe the simple test Diego
suggested on our IRC channel could work in this case?

#include <XShm.h>
#ifndef X_ShmAttach
#include <X11/Xmd.h>
#include <X11/extensions/shmproto.h>
#endif

Vlastimil

[gnu_andrew]

2009/11/2 Vlastimil Babka <caster@...>:

> Andrew John Hughes wrote:
>  > Hi Mark, Vlastimil,
>
> Hi Andrew,
>
>> Having seen both patches before, I'd personally prefer we just added
>> the necessary headers if that works, rather than the additional cruft
>> we have in the Fedora patch.  We should do that in awt_GraphicsEnv.h
>> though, as in the Fedora patch, which would also remove the need for
>> the #ifdef MITSHM (it's in awt_GraphicsEnv.h already).  So basically
>> the Fedora patch but without the removal of the XShm.h include and the
>> additional structs.
>
> Sure, why not. I just included it in the .c file because the .h file
> didn't use the constant and I hoped to minimize potential symbol
> collisions or something (suspected something like that could be the
> reason for the weird Fedora patch).
>
>> Though I've been aware of this for a while, I deliberately haven't
>> upgraded my own build systems to the new version because it will break
>> OpenJDK builds for which we don't yet have a solution.  Does anyone
>> know of some versioning defines in the X headers that would avoid us
>> having to use a configure check, and thus allow the patch to be
>> submitted upstream? I believe the change
>> (http://lists.x.org/archives/xorg-devel/2009-June/001242.html) appears
>> in libXext 1.1.1 as Vlastimil says, but is there a way of detecting
>> that other than with autoconf?
>
> I don't know about versioning defines, but maybe the simple test Diego
> suggested on our IRC channel could work in this case?
>
> #include <XShm.h>
> #ifndef X_ShmAttach
> #include <X11/Xmd.h>
> #include <X11/extensions/shmproto.h>
> #endif
>
> Vlastimil
>

Applying that to OpenJDk6 works here without shmproto.h:

http://cr.openjdk.java.net/~andrew/xshm/webrev.01/

Does it work for those with the new version? If so, we can send it upstream.
--
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8

[Mark Wielaard-4]

Hi Vlastimil,

On Mon, 2009-11-02 at 22:06 +0100, Vlastimil Babka wrote:
> Sure, why not. I just included it in the .c file because the .h file
> didn't use the constant and I hoped to minimize potential symbol
> collisions or something (suspected something like that could be the
> reason for the weird Fedora patch).

I also couldn't find a reason to define these directly instead of
through including both headers if necessary.

> I don't know about versioning defines, but maybe the simple test Diego
> suggested on our IRC channel could work in this case?
>
> #include <XShm.h>
> #ifndef X_ShmAttach
> #include <X11/Xmd.h>
> #include <X11/extensions/shmproto.h>
> #endif

That is much nicer and simpler than my approach. And it works fine for
my setups (Fedora 11 and rawhide) Thanks. I committed it as:

2009-11-02  Mark Wielaard  <mjw@...>
            Vlastimil Babka  <caster@...>

    * Makefile.am (ICEDTEA_PATCHES): Add patches/icedtea-xshm.patch
    * patches/icedtea-xshm.patch: New patch.
    * HACKING: Document new patch.

Cheers,

Mark

diff -r 547923046557 Makefile.am
--- a/Makefile.am Fri Oct 30 13:35:54 2009 +0000
+++ b/Makefile.am Tue Nov 03 12:49:36 2009 +0100
@@ -664,6 +664,7 @@
  patches/icedtea-s390-serialize.patch \
  patches/openjdk/6879689-hotspot_warning_fix.patch \
  patches/icedtea-no-precompiled.patch \
+ patches/icedtea-xshm.patch \
  $(DISTRIBUTION_PATCHES)
 
 stamps/extract.stamp: stamps/download.stamp
diff -r 547923046557 patches/icedtea-xshm.patch
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/patches/icedtea-xshm.patch Tue Nov 03 12:49:36 2009 +0100
@@ -0,0 +1,13 @@
+--- old/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.h 2009-07-17 10:36:09.000000000 -0400
++++ openjdk/jdk/src/solaris/native/sun/awt/awt_GraphicsEnv.h 2009-07-30 13:25:52.000000000 -0400
+@@ -42,6 +42,10 @@
+ #include <sys/ipc.h>
+ #include <sys/shm.h>
+ #include <X11/extensions/XShm.h>
++#ifndef X_ShmAttach
++#include <X11/Xmd.h>
++#include <X11/extensions/shmproto.h>
++#endif
+
+ extern int XShmQueryExtension();
+