Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build failure with glib 2.68 #1057

Open
rezso opened this issue Apr 4, 2021 · 10 comments · May be fixed by #1058
Open

Build failure with glib 2.68 #1057

rezso opened this issue Apr 4, 2021 · 10 comments · May be fixed by #1058

Comments

@rezso
Copy link

rezso commented Apr 4, 2021

Build with glib 2.68 fails:

In file included from /usr/lib/glib-2.0/include/glibconfig.h:9, from /usr/include/glib-2.0/glib/gtypes.h:32, from /usr/include/glib-2.0/glib/galloca.h:32, from /usr/include/glib-2.0/glib.h:30, from /usr/include/gstreamer-1.0/gst/gst.h:27, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.h:21, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:23: ../Source/WTF/wtf/glib/GRefPtr.h:33:21: error: expected unqualified-id before ‘typename’ 33 | extern "C" gpointer g_object_ref_sink(gpointer); | ^~~~~~~~~~~~~~~~~ ../Source/WTF/wtf/glib/GRefPtr.h:33:21: error: expected ‘)’ before ‘typename’ In file included from /usr/include/glib-2.0/gobject/gbinding.h:29, from /usr/include/glib-2.0/glib-object.h:22, from /usr/include/gstreamer-1.0/gst/gstenumtypes.h:6, from /usr/include/gstreamer-1.0/gst/gst.h:31, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.h:21, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:23: ../Source/WTF/wtf/glib/GRefPtr.h:33:21: note: to match this ‘(’ 33 | extern "C" gpointer g_object_ref_sink(gpointer); | ^~~~~~~~~~~~~~~~~ In file included from /usr/lib/glib-2.0/include/glibconfig.h:9, from /usr/include/glib-2.0/glib/gtypes.h:32, from /usr/include/glib-2.0/glib/galloca.h:32, from /usr/include/glib-2.0/glib.h:30, from /usr/include/gstreamer-1.0/gst/gst.h:27, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.h:21, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:23: ../Source/WTF/wtf/glib/GRefPtr.h:33:21: error: expected ‘)’ before ‘typename’ 33 | extern "C" gpointer g_object_ref_sink(gpointer); | ^~~~~~~~~~~~~~~~~ In file included from /usr/include/glib-2.0/gobject/gbinding.h:29, from /usr/include/glib-2.0/glib-object.h:22, from /usr/include/gstreamer-1.0/gst/gstenumtypes.h:6, from /usr/include/gstreamer-1.0/gst/gst.h:31, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.h:21, from ../Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:23: ../Source/WTF/wtf/glib/GRefPtr.h:33:21: note: to match this ‘(’ 33 | extern "C" gpointer g_object_ref_sink(gpointer); | ^~~~~~~~~~~~~~~~~

Removing extern "C" void g_object_unref(gpointer); and extern "C" gpointer g_object_ref_sink(gpointer); from Source/WTF/wtf/glib/GRefPtr.h solves this error.
@Vogtinator
Copy link

This is an issue with WebKit, not QtWebKit specifically.

@Vogtinator
Copy link

It looks like upstream WebKit is very lucky and inches around the problem by a different order of includes. In current qtwebkit, gst.h is included first (which includes gobject), then GRefPtr.h. Since WebKit/WebKit@ed1f9bc#diff-bd775e5dd224606dbeab641c083995ae189e80528a1ffaa19cc1dc66be3e9501, upstream has it the other way around, so the declarations aren't malformed at that point.

@Vogtinator Vogtinator linked a pull request Apr 7, 2021 that will close this issue
Open
@annulen
Copy link
Member

annulen commented Jul 2, 2023

@mnutt This issue is still relevant in master:

/usr/bin/ccache /usr/lib/llvm/16/bin/clang++ -DBUILDING_QT__=1 -DBUILDING_WEBKIT=1 -DBUILDING_WITH_CMAKE=1 -DBUILDING_WebCore -DHAVE_CONFIG_H=1 -DPAS_BMALLOC=1 -DQT_ASCII_CAST_WARNINGS -DQT_CORE_LIB -DQT_DEPRECATED_WARNINGS -DQT_DISABLE_DEPRECATED_BEFORE=0x050000 -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_DYNAMIC_CAST -DQT_NO_EXCEPTIONS -DQT_NO_NARROWING_CONVERSIONS_IN_CONNECT -DQT_USE_QSTRINGBUILDER -DSTATICALLY_LINKED_WITH_JavaScriptCore -DSTATICALLY_LINKED_WITH_PAL -DSTATICALLY_LINKED_WITH_WTF -DSTATICALLY_LINKED_WITH_bmalloc ... -c /home/ktokarev/WebKit/qtwebkit-dev-2/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerQt.cpp
In file included from /home/ktokarev/WebKit/qtwebkit-dev-2/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerQt.cpp:21:
In file included from /home/ktokarev/WebKit/qtwebkit-dev-2/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h:26:
In file included from /home/ktokarev/WebKit/qtwebkit-dev-2/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:24:
In file included from /home/ktokarev/WebKit/qtwebkit-dev-2/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h:26:
/home/ktokarev/WebKit/qtwebkit-dev-2/build/host-clang/WTF/Headers/wtf/glib/GRefPtr.h:38:21: error: expected unqualified-id
extern "C" gpointer g_object_ref_sink(gpointer);
                    ^
/usr/include/glib-2.0/gobject/gobject.h:549:34: note: expanded from macro 'g_object_ref_sink'
#define g_object_ref_sink(Obj) ((glib_typeof (Obj)) (g_object_ref_sink) (Obj))
                                 ^
/usr/include/glib-2.0/glib/glib-typeof.h:44:24: note: expanded from macro 'glib_typeof'
#define glib_typeof(t) typename std::remove_reference<decltype (t)>::type
                       ^

@mnutt
Copy link
Collaborator

mnutt commented Jul 2, 2023

I don't currently have easy access to a dev machine that runs linux with glib 2.68+, but am going to enable GitHub Actions to run there and try to reproduce the issue.

@mnutt
Copy link
Collaborator

mnutt commented Jul 6, 2023

Fixed in my fork via movableink#9

@Vogtinator
Copy link

Note also #1058

@FabioLolix
Copy link

On AUR qt5-webkit pkgbuild https://aur.archlinux.org/packages/qt5-webkit is fixed by https://aur.archlinux.org/cgit/aur.git/tree/glib-2.68.patch?h=qt5-webkit

@annulen
Copy link
Member

annulen commented Jul 29, 2023 •
edited
Loading

But I guess we all can agree that this is not a real fix of an issue with forward declarations, just a workaround.

It seems to me that the real bug is include order in GRefPtrGStreamer.h, where #include <gst/gst.h> is done before #include <wtf/glib/GRefPtr.h>. This order originates from ordering suggested by WebKit coding style, but it simply cannot work unless GRefPtr.h is already included by user of GRefPtrGStreamer.h.

diff --git a/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
index 1588a813b5d3..3f06a4571e58 100644
--- a/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
+++ b/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
@@ -21,9 +21,9 @@
 
 #if USE(GSTREAMER)
 
+#include <wtf/glib/GRefPtr.h>
 #include <gst/gst.h>
 #include <gst/pbutils/encoding-profile.h>
-#include <wtf/glib/GRefPtr.h>
 
 typedef struct _WebKitVideoSink WebKitVideoSink;
 struct WebKitWebSrc;

I guess GTK and WPE are building fine only because they use GRefPtr more extensively and it gets included earlier.

@annulen
Copy link
Member

annulen commented Jul 29, 2023

@philn: How do you think, would it be OK to change include order in GRefPtrGStreamer.h like show in a previous comment? See justification above.

@philn
Copy link

philn commented Jul 29, 2023

Yes, and an exception for build/include_order should be added for this file in the style checker...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove invalid g_object declarations to fix build with glib >= 2.68 Vogtinator/qtwebkit
6 participants
@mnutt @philn @annulen @Vogtinator @rezso @FabioLolix