Bug 248727

Summary:	gdesktopappinfo.h not excluded on macOS
Product:	WebKit	Reporter:	echassiers.09-regards
Component:	WebKitGTK	Assignee:	Michael Catanzaro <mcatanzaro>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	bugs-noreply, mcatanzaro, webkit-bug-importer
Priority:	P2
Version:	Other
Hardware:	Mac (Apple Silicon)
OS:	macOS 13

Description echassiers.09-regards 2022-12-03 17:30:04 PST

After dropping libnotify in 2.38.0, we now have

#include <gio/gdesktopappinfo.h>

in Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp.

This prevents WebKitGTK from being built on macOS, where gdesktopappinfo.h is not available.

A quick fix would be to simply wrap the affected parts with #ifndef __APPLE__.

Comment 1 Michael Catanzaro 2022-12-05 10:29:11 PST Comment hidden (obsolete)

Please test this:

diff --git a/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp b/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp
index 61e4c5e50cb8..0b34a2041193 100644
--- a/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp
+++ b/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp
@@ -31,7 +31,6 @@
 #include <WebCore/NotificationResources.h>
 #include <WebCore/RefPtrCairo.h>
 #include <cairo.h>
-#include <gio/gdesktopappinfo.h>
 #include <gio/gio.h>
 #include <glib/gi18n-lib.h>
 #include <mutex>
@@ -51,6 +50,10 @@
 #include <WebCore/GtkVersioning.h>
 #endif
 
+#if OS(UNIX)
+#include <gio/gdesktopappinfo.h>
+#endif
+
 namespace WebKit {
 
 static const Seconds s_dbusCallTimeout = 20_ms;
@@ -293,6 +296,7 @@ void NotificationService::processCapabilities(GVariant* variant)
 
 static const char* applicationIcon(const char* applicationID)
 {
+#if OS(UNIX)
     static std::optional<CString> appIcon;
     if (!appIcon) {
         appIcon = [applicationID]() -> CString {
@@ -330,6 +334,9 @@ static const char* applicationIcon(const char* applicationID)
     }
 
     return appIcon->data();
+#else
+    return { };
+#endif
 }
 
 bool NotificationService::showNotification(const WebNotification& notification, const RefPtr<WebCore::NotificationResources>& resources)


If that works for you, I'll submit a merge request.

Comment 2 Michael Catanzaro 2022-12-05 10:33:41 PST Comment hidden (obsolete)

Actually wait, that contains an error. Sec...

Comment 3 Michael Catanzaro 2022-12-05 11:06:25 PST

OK, indeed this one particular header is special... unlike the rest of gio-unix-2.0, gdesktopappinfo really is specifically excluded from macOS. Odd.

Comment 4 Michael Catanzaro 2022-12-05 11:08:57 PST

So here's a second attempt. Try this. If it works, I'll create a pull request.

diff --git a/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp b/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp
index 61e4c5e50cb8..820c02e9afeb 100644
--- a/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp
+++ b/Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp
@@ -31,7 +31,6 @@
 #include <WebCore/NotificationResources.h>
 #include <WebCore/RefPtrCairo.h>
 #include <cairo.h>
-#include <gio/gdesktopappinfo.h>
 #include <gio/gio.h>
 #include <glib/gi18n-lib.h>
 #include <mutex>
@@ -51,6 +50,10 @@
 #include <WebCore/GtkVersioning.h>
 #endif
 
+#if OS(UNIX) && !OS(DARWIN)
+#include <gio/gdesktopappinfo.h>
+#endif
+
 namespace WebKit {
 
 static const Seconds s_dbusCallTimeout = 20_ms;
@@ -294,6 +297,7 @@ void NotificationService::processCapabilities(GVariant* variant)
 static const char* applicationIcon(const char* applicationID)
 {
     static std::optional<CString> appIcon;
+#if OS(UNIX) && !OS(DARWIN)
     if (!appIcon) {
         appIcon = [applicationID]() -> CString {
             if (!applicationID)
@@ -328,6 +332,7 @@ static const char* applicationIcon(const char* applicationID)
             return { };
         }();
     }
+#endif // OS(UNIX) && !OS(DARWIN)
 
     return appIcon->data();
 }

Comment 5 echassiers.09-regards 2022-12-05 13:20:08 PST

It works. Thanks!

There are still some errors preventing the build, but I guess that deserves a new issue?

[6035/6040] Generating WebKit2WebExtension-4.0.gir
In file included from /private/tmp/nix-build-webkitgtk-2.38.2+abi=4.0.drv-0/webkitgtk-2.38.2/g-ir-cpp-76gq265z.c:6:
In file included from /private/tmp/nix-build-webkitgtk-2.38.2+abi=4.0.drv-0/webkitgtk-2.38.2/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/webkitdom.h:24:
In file included from /private/tmp/nix-build-webkitgtk-2.38.2+abi=4.0.drv-0/webkitgtk-2.38.2/build/WebKit2Gtk/Headers/webkit2gtk-webextension/webkitdom/WebKitDOMAttr.h:28:
/private/tmp/nix-build-webkitgtk-2.38.2+abi=4.0.drv-0/webkitgtk-2.38.2/build/WebKit2Gtk/Headers/webkit2gtk-webextension/webkitdom/WebKitDOMNode.h:28:10: fatal error: 'jsc/jsc.h' file not found
#include <jsc/jsc.h>

[6036/6040] Generating WebKit2-4.0.gir
In file included from /private/tmp/nix-build-webkitgtk-2.38.2+abi=4.0.drv-0/webkitgtk-2.38.2/g-ir-cpp-5z6p65h8.c:35:
/private/tmp/nix-build-webkitgtk-2.38.2+abi=4.0.drv-0/webkitgtk-2.38.2/Source/WebKit/UIProcess/API/gtk/WebKitJavascriptResult.h:27:10: fatal error: 'JavaScriptCore/JSBase.h' file not found
#include <JavaScriptCore/JSBase.h>

Comment 6 Michael Catanzaro 2022-12-05 13:48:27 PST

(In reply to echassiers.09-regards from comment #5)
> There are still some errors preventing the build, but I guess that deserves
> a new issue?

A separate bug would probably be better, but up to you. For the include directory issues, you'll need to provide a patch yourself unfortunately, since those errors really need to be fixed by the person encountering them. Good luck.

Comment 7 Michael Catanzaro 2022-12-05 13:55:26 PST

Pull request: https://github.com/WebKit/WebKit/pull/7162

Comment 8 EWS 2022-12-22 14:29:21 PST

Committed 258260@main (1446ba237462): <https://commits.webkit.org/258260@main>

Reviewed commits have been landed. Closing PR #7162 and removing active labels.