VirtualBox

Opened 8 years ago

Closed 8 years ago

#16052 closed defect (fixed)

patches for Xorg-1.19 -> fixed in major releases greater than 5.1

Reported by: sergiomb Owned by:
Component: 3D support Version: VirtualBox 5.1.6
Keywords: Cc:
Guest type: X11 Host type: Linux

Description

Hello, Fedora 25 have RC1 for xorg 1.19 in updates-testing (don't ask me why) we have 2 problems when building in downstream way ,

[PATCH] dix: Remove redundant ChangeWindowProperty ( https://lists.x.org/archives/xorg-devel/2015-November/048030.html )

and also WakeupHandlerProcPtr was removed https://lists.x.org/archives/xorg-devel/2015-November/047844.html

Attachments (3)

for_uptream.patch (1.2 KB ) - added by sergiomb 8 years ago.
VirtualBox-4.3.10-xserver_guest.patch (2.7 KB ) - added by sergiomb 8 years ago.
VirtualBox-5.0.18-xserver_guest.patch (4.7 KB ) - added by sergiomb 8 years ago.
new version

Download all attachments as: .zip

Change History (23)

by sergiomb, 8 years ago

Attachment: for_uptream.patch added

comment:1 by Michael Thayer, 8 years ago

Thank you for the patch. I am curious though why you are still building the user-space X.Org driver rather than the kernel driver for X.Org Server 1.19.

by sergiomb, 8 years ago

comment:2 by sergiomb, 8 years ago

I (and all other downstreams btw) build Vbox with VBOX_USE_SYSTEM_XORG_HEADERS=1 , we patch also build Additions to use SYSTEM XORG HEADERS, I hope I am replying to the question. Now thinking on you are asking, I think we don't need vboxvideo_drv_system.so anymore , so maybe it it better make build skip this part, instead fixing the build ? isn't it ?

Thanks.

Last edited 8 years ago by sergiomb (previous) (diff)

comment:3 by sergiomb, 8 years ago

Michael,

Like you wrote if I don't build vboxvideo_drv_system.so, I don't need patch ./src/VBox/Additions/x11/vboxvideo

In my previous comment since we use SYSTEM XORG HEADERS, maybe this is a good opportunity to also add VBOX_USE_SYSTEM_XORG_HEADERS in ./src/VBox/Additions/common/crOpenGL/Makefile.kmk and ./src/VBox/Additions/common/VBoxGuestLib/Makefile.kmk ... I'll send to you patches that I'm thinking on, after sleep !

comment:4 by Michael Thayer, 8 years ago

Presumably all that is needed is to conditionally not define VBOX_MESA_INCS in Config.kmk. Just tested and it seems to work fine. In fact we only need this ourselves to build on old systems which do not provide sufficiently up-to-date GL libraries. I presume that a Make variable to disable building the X.Org drivers altogether would make you life easier too.

comment:5 by Michael Thayer, 8 years ago

No, wrong after all - there have been some subtle but relevant changes in GL function parameters and things.

in reply to:  5 comment:6 by sergiomb, 8 years ago

Replying to michael:

No, wrong after all - there have been some subtle but relevant changes in GL function parameters and things.

why you say that ? , I'm testing Fedora 24 in a vm-guest (Xorg 1.18) and works well , even Gnome in xorg-wayland ! Before build Vbox, we remove all bundle X11 sources with

rm -r src/VBox/Additions/x11/x11include/
rm -r src/VBox/Additions/x11/x11stubs/

Maybe you need also use Xorg system on ./src/VBox/Additions/common/crOpenGL/Makefile.kmk like VirtualBox-4.3.10-xserver_guest.patch in attach does ...

Thanks.

comment:7 by Michael Thayer, 8 years ago

Why do you not remove the VBOX_MESA_INCS bit in the GL code includes? Not that it is likely to be very relevant if you remove the files anyway.

comment:8 by sergiomb, 8 years ago

yes, I also should remove VBOX_MESA_INCS from Config.kmk, like you said is not relevant, note taken :)

by sergiomb, 8 years ago

new version

comment:9 by sergiomb, 8 years ago

Hi Michael,

I follow your advise and in ./Config.kmk I do not define VBOX_MESA_INCS (using VBOX_USE_SYSTEM_XORG_HEADERS variable), so IMHO may be included in VBox official source code.

 Config.kmk                                          |    2 ++
 src/VBox/Additions/common/VBoxGuestLib/Makefile.kmk |    2 ++
 src/VBox/Additions/common/crOpenGL/Makefile.kmk     |   20 +++++++++++++++++++-
 src/VBox/Additions/x11/vboxvideo/Makefile.kmk       |    2 +-

src/VBox/Additions/x11/vboxvideo/Makefile.kmk maybe should not be included, is the way of fix vbox build against new Xorg 1.19 .

Analysing build.log, I just found one "-I" with "x11include", and it is x11include/mesa-11.0.7 (1), happens when is building obj/VBoxEGL/egl.o . ./src/VBox/Additions/common/crOpenGL/Makefile.kmk we still define VBoxEGL_INCS = $(VBOX_PATH_X11_ROOT)/mesa-11.0.7 ... , but for now we have a big improve.

Thank you.

(1) https://copr.fedorainfracloud.org/coprs/sergiomb/vboxfor23/build/464163/

https://copr-be.cloud.fedoraproject.org/results/sergiomb/vboxfor23/fedora-rawhide-x86_64/00464163-VirtualBox/

comment:10 by Michael Thayer, 8 years ago

Just done these two commits. Will they help you?

Two notes: you don't need to disable building VBoxGuestR3LibXFree86 and VBoxGuestR3LibXOrg: make will recognise that these are dependencies of things you are not building (X server drivers) and skip them automatically. And in your patch you disabled VBOX_NO_NATIVEGL for system headers builds. Are you sure you checked that this is the right thing to do?

bugref:3810: X11 Guest Additions maintenance: Add two new defines - VBOX_USE_SYSTEM_GL_HEADERS and VBOX_NO_LEGACY_XORG_X11 - for the benefit of people doing distribution builds. They make us use system OpenGL headers for building our libGL and prevent us building X.Org drivers and stub libraries for linking on old distributions. These changes are to be tested by people who need them, not by upstream developers.

Index: Config.kmk
===================================================================
--- Config.kmk	(revision 111282)
+++ Config.kmk	(revision 111283)
@@ -2378,11 +2378,11 @@
  endif
 
  if1of ($(KBUILD_TARGET), freebsd linux solaris)
- # VBOX_PATH_MESA_SOURCE = $(PATH_ROOT)/src/libs/mesa-7.2
-  VBOX_PATH_MESA_SOURCE = $(VBOX_PATH_X11_ROOT)/mesa-7.2
-  VBOX_GL_INCS = \
+  ifndef VBOX_USE_SYSTEM_GL_HEADERS
+   VBOX_GL_INCS = \
         $(VBOX_PATH_X11_ROOT)/glproto-1.4.10 \
         $(VBOX_PATH_X11_ROOT)/mesa-7.2
+  endif
  endif
 
  VBOX_DARWIN_OPENGL_INST     = obj/VBoxOGL/GL/
Index: src/VBox/Additions/x11/Makefile.kmk
===================================================================
--- src/VBox/Additions/x11/Makefile.kmk	(revision 111282)
+++ src/VBox/Additions/x11/Makefile.kmk	(revision 111283)
@@ -20,13 +20,19 @@
 # Include sub-makefiles.
 if1of ($(KBUILD_TARGET), freebsd linux netbsd openbsd solaris)
  include $(PATH_SUB_CURRENT)/VBoxClient/Makefile.kmk
- include $(PATH_SUB_CURRENT)/vboxvideo/Makefile.kmk
- ifneq ($(KBUILD_TARGET), solaris)
-  include $(PATH_SUB_CURRENT)/vboxmouse/Makefile.kmk
+ ifndef VBOX_NO_LEGACY_XORG_X11
+  include $(PATH_SUB_CURRENT)/vboxvideo/Makefile.kmk
+  ifneq ($(KBUILD_TARGET), solaris)
+   include $(PATH_SUB_CURRENT)/vboxmouse/Makefile.kmk
+  endif
+  # This should logically only be controlled by VBOX_NO_LEGACY_XORG_X11,
+  # as it is not used for drivers at all, but rather to build X11 clients
+  # on systems missing needed libraries.
+  ## @todo fix at some later point when it will not break people's workflows.
+  ifndef VBOX_USE_SYSTEM_XORG_HEADERS
+   include $(PATH_SUB_CURRENT)/x11stubs/Makefile.kmk
+  endif
  endif
- ifndef VBOX_USE_SYSTEM_XORG_HEADERS
-  include $(PATH_SUB_CURRENT)/x11stubs/Makefile.kmk
- endif
 endif
 
 include $(FILE_KBUILD_SUB_FOOTER)

bugref:3810: X11 Guest Additions maintenance: check VBOX_USE_SYSTEM_GL_HEADERS when building the OpenGL libraries.

Index: src/VBox/Additions/common/crOpenGL/Makefile.kmk
===================================================================
--- src/VBox/Additions/common/crOpenGL/Makefile.kmk	(revision 111283)
+++ src/VBox/Additions/common/crOpenGL/Makefile.kmk	(revision 111284)
@@ -67,7 +67,10 @@
 VBoxOGL_TEMPLATE       = VBOXCROGLR3GUESTDLL
 VBoxOGL_INCS           = .
 if1of ($(KBUILD_TARGET), linux solaris freebsd)
- VBoxOGL_INCS     += \
+ ifdef VBOX_USE_SYSTEM_GL_HEADERS
+  VBoxOGL_INCS += x11
+ else
+  VBoxOGL_INCS     += \
 	$(VBOX_PATH_X11_ROOT)/libXdamage-1.1 \
 	$(VBOX_PATH_X11_ROOT)/libXcomposite-0.4.0 \
 	$(VBOX_PATH_X11_ROOT)/libXext-1.3.1 \
@@ -78,8 +81,8 @@
 	$(VBOX_PATH_X11_ROOT)/libx11-1.1.5-other \
 	$(VBOX_PATH_X11_ROOT)/xextproto-7.1.1 \
 	$(VBOX_PATH_X11_ROOT)/xproto-7.0.18 \
-	$(VBOX_GL_INCS) \
-	$(PATH_ROOT)/src/VBox/Additions/x11/x11include/libdrm-2.4.13
+	$(VBOX_GL_INCS)
+ endif
  VBoxOGL_DEFS     += VBOX_NO_NATIVEGL
 endif
 
@@ -213,11 +216,15 @@
 	$(PATH_STAGE_LIB)/additions/VBoxCrHgsmi$(VBOX_SUFF_LIB)
 
 if1of ($(KBUILD_TARGET), linux solaris freebsd)
- VBoxOGL_LIBS += \
+ ifdef VBOX_USE_SYSTEM_GL_HEADERS
+  VBoxOGL_LIBS += Xcomposite Xdamage Xfixes Xext
+ else
+  VBoxOGL_LIBS += \
  	$(PATH_STAGE_LIB)/libXcomposite.so \
  	$(PATH_STAGE_LIB)/libXdamage.so \
  	$(PATH_STAGE_LIB)/libXfixes.so \
  	$(PATH_STAGE_LIB)/libXext.so
+ endif
  ifdef VBoxOGL_FAKEDRI
   ifeq ($(KBUILD_TARGET), freebsd)
     VBoxOGL_LIBS += \

comment:11 by Michael Thayer, 8 years ago

Actually a third question. Is the "+ VBoxOGL_INCS += x11" really necessary? Though I take it you did check that.

in reply to:  10 comment:12 by sergiomb, 8 years ago

Replying to michael:

Just done these two commits. Will they help you?

Two notes: you don't need to disable building VBoxGuestR3LibXFree86 and VBoxGuestR3LibXOrg: make will recognise that these are dependencies of things you are not building (X server drivers) and skip them automatically. And in your patch you disabled VBOX_NO_NATIVEGL for system headers builds. Are you sure you checked that this is the right thing to do?

bugref:3810: X11 Guest Additions maintenance: Add two new defines - VBOX_USE_SYSTEM_GL_HEADERS and VBOX_NO_LEGACY_XORG_X11 - for the benefit of people doing distribution builds. They make us use system OpenGL headers for building our libGL and prevent us building X.Org drivers and stub libraries for linking on old distributions. These changes are to be tested by people who need them, not by upstream developers.

IIUC , you are saying that I will be able to add VBOX_USE_SYSTEM_GL_HEADERS=1 and VBOX_NO_LEGACY_XORG_X11=1 in my localconfig.kmk ? should be a awesome.

Today I also added VBOX_USE_SYSTEM_XORG_HEADERS around VBoxEGL_INCS (the last one which use x11includes sources) .

https://pkgs.rpmfusion.org/cgit/free/VirtualBox.git/tree/VirtualBox-5.0.18-xserver_guest.patch#n126

in reply to:  11 comment:13 by sergiomb, 8 years ago

Replying to michael:

Actually a third question. Is the "+ VBoxOGL_INCS += x11" really necessary? Though I take it you did check that.

I used :

+ VBoxOGL_INCS     += \
+	/usr/include/x11 \
+	/usr/include/xorg \
+	/usr/include/pixman-1 \
+ 	$(VBOX_MESA_INCS) \
+	/usr/include/drm \
+	/usr/include/libdrm

is just safer, but I can test not include it even x11

pkg-config pixman-1 --cflags
-I/usr/include/pixman-1

pkg-config libdrm --cflags
-I/usr/include/libdrm

pkg-config xorg-server --cflags
-fvisibility=hidden -I/usr/include/xorg -I/usr/include/pixman-1 -I/usr/include/libdrm -I/usr/include/X11/dri

/usr/include/X11/ (with capital X) belongs to xorg-x11-proto-devel

/usr/include/drm/ belongs to kernel-headers, this one maybe just need when we build kernel modules , but is in other build (kmod builds) .

comment:14 by Michael Thayer, 8 years ago

Yes, I was surprised about the lower case x11 too, but since you are testing this not me I left it. If you can confirm that the last change works for you I will also make VBoxEGL_INCS dependent on VBOX_USE_SYSTEM_GL_HEADERS. This will be next major release only, I currently do not plan to back-port it to 5.1. I dare say you can carry patches that long...

in reply to:  14 comment:15 by sergiomb, 8 years ago

Replying to michael:

Yes, I was surprised about the lower case x11 too,

x11 with lower case is a mistake, probably made by me, should be X11 .

but since you are testing this not me I left it. If you can confirm that the last change works for you I will also make VBoxEGL_INCS dependent on VBOX_USE_SYSTEM_GL_HEADERS. This will be next major release only, I currently do not plan to back-port it to 5.1. I dare say you can carry patches that long...

I'd love have the patches to begin test it myself .

comment:16 by sergiomb, 8 years ago

I update my patch (1) , you are correct, I can, not set VBOX_MESA_INCS. I Found another x11 with lower case , maybe is what you questioned and can be removed (line 118) (I even tested and compiles vboxvideo_drv_system.so )

(1) https://pkgs.rpmfusion.org/cgit/free/VirtualBox.git/tree/VirtualBox-5.0.18-xserver_guest.patch?id=92f4bb5773a971eaa37a9348eb630b31786a6d29

https://pkgs.rpmfusion.org/cgit/free/VirtualBox.git/plain/VirtualBox-5.0.18-xserver_guest.patch?id=92f4bb5773a971eaa37a9348eb630b31786a6d29

Last edited 8 years ago by sergiomb (previous) (diff)

comment:17 by Michael Thayer, 8 years ago

--- src/VBox/Additions/common/crOpenGL/Makefile.kmk	(revision 111284)
+++ src/VBox/Additions/common/crOpenGL/Makefile.kmk	(working copy)
@@ -67,9 +67,7 @@
 VBoxOGL_TEMPLATE       = VBOXCROGLR3GUESTDLL
 VBoxOGL_INCS           = .
 if1of ($(KBUILD_TARGET), linux solaris freebsd)
- ifdef VBOX_USE_SYSTEM_GL_HEADERS
-  VBoxOGL_INCS += x11
- else
+ ifndef VBOX_USE_SYSTEM_GL_HEADERS
   VBoxOGL_INCS     += \
 	$(VBOX_PATH_X11_ROOT)/libXdamage-1.1 \
 	$(VBOX_PATH_X11_ROOT)/libXcomposite-0.4.0 \
@@ -716,7 +714,9 @@
 
 VBoxEGL_TEMPLATE       = VBOXCROGLR3GUESTDLL
 VBoxEGL_SOURCES        = egl.c
-VBoxEGL_INCS           = $(VBOX_PATH_X11_ROOT)/mesa-11.0.7
+ifndef VBOX_USE_SYSTEM_GL_HEADERS
+ VBoxEGL_INCS           = $(VBOX_PATH_X11_ROOT)/mesa-11.0.7
+endif
 VBoxEGL_LIBS           = $(VBOX_LIB_OGL) $(VBOX_LIB_IPRT_GUEST_R3_SHARED)
 VBoxEGL_SONAME.linux   = libEGL.so.1

comment:18 by Michael Thayer, 8 years ago

Summary: patches for Xorg-1.19patches for Xorg-1.19 -> fixed in major releases greater than 5.1

I assume I can close this.

comment:19 by sergiomb, 8 years ago

yes

comment:20 by Frank Mehnert, 8 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.

© 2023 Oracle
ContactPrivacy policyTerms of Use