irc dump:

>[dboles]	Is there any support for GNotifications on win32 or will I need a backup plan?
>
>[TingPing]	dboles, i don't know of anybody working on it, but it shouldn't
>		be *too* hard to implement
>
>		dboles, here is a basic implementation:
>		https://github.com/hexchat/hexchat/blob/master/src/fe-gtk/notifications/notification-winrt.cpp
>
>		if somebody cleaned that up and made it into a gio module im sure we could get
>		it merged
>
>[dboles]	TingPing: i seem to recall reading that the app would crash on win32 if
>		notifications were used, which will spur me on when i test it again
>
>[dboles]	we should see what LRN and co think :)
>
>[LRN]		about what?
>
>[TingPing]	LRN, using winrt notifications from msys
>
>[dboles]	wondering whether you had any thoughts on GNotifications on win32
>
>[TingPing]	it will do nothing on win32 atm
>
>[LRN]		bloatpad crashes when it tries to send a GNotification
>
>[TingPing]	well that shouldn't happen :P
>
>[dboles]	bah! thanks for testing. yeah, that seems... unhelpful
>
>		is it an assert or just a segv?
>
>[LRN]		_g_io_module_get_default_type
>
>		(G_NOTIFICATION_BACKEND_EXTENSION_POINT_NAME, "GNOTIFICATION_BACKEND",
>		G_STRUCT_OFFSET (GNotificationBackendClass, is_supported));
>		returns 0, and everything goes downhill from there
>
>		g_object_new (0, NULL) segfaults
>
>[TingPing]	should probably just simply return, or maybe have a dummy impl
>
>[dboles]	yeah, a no-op impl would be an instant win
>
>[LRN]		there's a "Gtk Notification backend", i don't know why it isn't used as
>		a fallback
>
>[TingPing]	LRN, well its not like you have a dbus service running
>
>[LRN]		glib itself runs a session bus by default
>
>[dboles]	huh, that's interesting
>
>[LRN]		(or something like that)
>
>[TingPing]	LRN, *maybe* it wouldn't crash, but it ofc wouldn't ever work
>		either
>
>[LRN]		as for notifiations, i think we could use Windows Toast Notifications
>		(8.1+)
>
>		it will just take some COM wizardry
>
>		the code shouldn't be much different from GTK+ native filechooser dialogs
>
>[Company]	famous last words
>		"just some COM wizardry"
>
>[dboles]	LRN: that's what TingPing's implementation does
>
>[LRN]		dboles, yes, except that it should be in C, not C++
>
>[dboles]	heh oops
>		forgot it was in C++
>
>[TingPing]	does it really matter
>
>[Company]	extern "C++" {

*** Bug 784698 has been marked as a duplicate of this bug. ***

*** Bug 788237 has been marked as a duplicate of this bug. ***

With the deprecation of Gtk.StatusIcon, and GNotifications not implemented, we run out of ways to notify the User about Events.

so this would be really nice to have.

(In reply to lovetox from comment #4)
> With the deprecation of Gtk.StatusIcon, and GNotifications not implemented,
> we run out of ways to notify the User about Events.
> 
> so this would be really nice to have.

Indeed, if someone wants to write it and submit some patches, I’ll be happy to review them. Not having access to a Windows machine, and not being a Windows developer, this is not something I can quickly do myself though.

If no one has the time to do a Backend for Windows, maybe someone has the time and fix that any call to notification crashes the Application

Created attachment 369970 [details] [review]
gio: Add dummy win32 notification backend

This adds a null notification backend implementation for win32, purely
to avoid crashes due to a missing backend when applications use
GNotification. This backend does nothing except print a warning when a
notification is supposed to be emitted.

In future, it can be expanded to use win32 API to present toaster
notifications appropriately.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Here’s a dummy win32 notification backend implementation which should fix the crashes, but won’t do anything else (it certainly doesn’t implement native notifications).

The idea is that this can be extended in future to actually implement native notifications.

I’ve compile-tested this on Linux (before making it conditional on being on Windows in the build system), but have done no further testing. I’d like someone with access to a Windows machine to verify that it compiles without warnings, and stops application crashes, before it gets merged. lovetox, would you be able to do that please?

hi, i never build GTK and have no experience with building

i read through https://wiki.gnome.org/Projects/GTK+/Win32/MSVCCompilationOfGTKStack

its huge and probably a few days of work to get this working
so i hope there is someone that has actually set up a win dev env

Another, perhaps simpler approach is to use MSYS2, download the GLib PKGBUILD, add Philip's patch to it, then rebuild.

(I'm almost at a point where I have a Windows machine that can survive that build... just an idea in case anyone else can beat me to it, though)

I'm also not much of a Windows developer and never built GTK, but I would like to try and get this fixed to solve an issue in another project (https://github.com/dino/dino/issues/309).

As I see it, I can get the relevant code from https://gitlab.gnome.org/GNOME/glib. Which branch should I base my changes on? And considering the comment in GitLab, how do you want them delivered? As an attached patch here as Philip did?

Thanks for your interest. You were already in the right place: you can also get the guides to hacking and contributing there!

https://gitlab.gnome.org/GNOME/glib/blob/master/HACKING
https://gitlab.gnome.org/GNOME/glib/blob/master/README.commits (weird formatting)

Yes, patches are to be submitted on Bugzilla for the moment, not GitLab. A Git-formatted patch with a message as described in README.commits is what's needed, or maybe more than 1 if you're making many/complex changes.

(In reply to Daniel Boles from comment #12)
Thanks for the information! I had already discovered the HACKING file, but thanks for the hint about the patch format.

Now I'm only missing the hint about the branch. Just go with `master`?

(In reply to Philip Withnall from comment #8)
> someone with access to a Windows machine to verify that it compiles without
> warnings, and stops application crashes, before it gets merged.

I installed MSYS2 and all the necessary libraries via pacman. libpcre could not be found although installed (indeed there's only a DLL, no static library as mentioned in an [MSYS2 issue]), but I found out I could use the one in GLib via

```shell
./configure --with-pcre=internal
```

In the end, I could install GLib with `make; make install`. Then I created a branch based on `master` for the moment, applied Philip's patch and built the code again. No warnings occurred, but please note that I only built GLib, not GTK.

I will proceed with writing a minimal test application which sends a notification, checking if it doesn't crash and finally replacing the dummy implementation with one using the Windows notification facilities.

That's just to tell you about the current state because I'll be on vacation for a week now and continue after that.


[MSYS2 issue]: https://github.com/Alexpux/MSYS2-packages/issues/872

(In reply to Philipp from comment #13)
> (In reply to Daniel Boles from comment #12)
> Thanks for the information! I had already discovered the HACKING file, but
> thanks for the hint about the patch format.
> 
> Now I'm only missing the hint about the branch. Just go with `master`?

Yes, if a patch is relevant to master (i.e. not for functionality in an older version removed in master), then write it against master; it can be cherry-picked to older versions as appropriate.

Created attachment 370709 [details]
Minimal GNotification test application

(In reply to Philipp from comment #14)
> (In reply to Philip Withnall from comment #8)
> > someone with access to a Windows machine to verify that it compiles without
> > warnings, and stops application crashes, before it gets merged.
> 
> I will proceed with writing a minimal test application which sends a
> notification, checking if it doesn't crash and finally replacing the dummy
> implementation with one using the Windows notification facilities.

Here's the test application. It shows the notification on Xubuntu (although not upon the first call, but I ignored this for the moment).

When I run it on Windows after building and installing the GLib version with Philip's dummy implementation, it does not crash and outputs as expected:

```
(gnotification-test.exe:7060): GLib-GIO-WARNING **: 18:00:22.997: Notifications are not yet supported on Windows.
g_application_send_notification called
GApplication returned status code 0
```

To implement a functioning backend, I would try and follow the [Windows Dev Center article] about sending toast notifications from a desktop C++ app. 

Otherwise, the [WinToast library] might be helpful, but I'm not sure how such a dependency is supposed to be used by GLib. Just copy the source code? Considering the currently high activity on that library, how would updates be tracked then?

[Windows Dev Center article]: https://docs.microsoft.com/en-us/windows/uwp/design/shell/tiles-and-notifications/send-local-toast-desktop-cpp-wrl
[WinToast library]: https://github.com/mohabouje/WinToast

(In reply to Philipp from comment #16)
> Here's the test application. It shows the notification on Xubuntu (although
> not upon the first call, but I ignored this for the moment).
> 
> When I run it on Windows after building and installing the GLib version with
> Philip's dummy implementation, it does not crash and outputs as expected:

That’s great, thanks for testing. I’ll get a second opinion on the dummy backend and push it to master soon as a stopgap until you can get a full Windows backend working. You can base your work on top of it then.

> To implement a functioning backend, I would try and follow the [Windows Dev
> Center article] about sending toast notifications from a desktop C++ app. 

From my limited knowledge of Windows, that does look like an appropriate way of sending notifications.

> Otherwise, the [WinToast library] might be helpful, but I'm not sure how
> such a dependency is supposed to be used by GLib. Just copy the source code?
> Considering the currently high activity on that library, how would updates
> be tracked then?

It seems like sending a notification should be simple enough that we don’t need to include third party code for it. Typically, we don’t want to add additional dependencies to GLib which aren’t standard system libraries.

Review of attachment 369970 [details] [review]:

Looks good for a dummy backend.

Comment on attachment 369970 [details] [review]
gio: Add dummy win32 notification backend

Pushed the dummy backend to master. Leaving this bug open to handle expanding it to a proper backend. Thanks for the review, Emmanuele.

Attachment 369970 [details] pushed as b868cf5 - gio: Add dummy win32 notification backend

critical => major since the crash is now prevented

Thanks for the dummy backend, Philip, and thanks for working on the proper backend, Philipp!

I would like to ask for some guidance concerning the file structure and build, so that my addition integrates well with the existing code.

As I understand, I have to write the notification code in C++. So my idea was to put the Windows-related code in a `gwinnotification.cpp` with `gwinnotification.h` containing the `extern "C"` sending and withdrawing functions to be used by `gwin32notificationbackend.c`.

1. Where should I best put the two new files? In `gio/win32`?
2. How do I add them to the build? Certainly I have to add a flag somewhere so GCC uses a current C++ compiler for `gwinnotification.cpp`?

(In reply to Philipp from comment #21) 
> As I understand, I have to write the notification code in C++. So my idea
> was to put the Windows-related code in a `gwinnotification.cpp` with
> `gwinnotification.h` containing the `extern "C"` sending and withdrawing
> functions to be used by `gwin32notificationbackend.c`.

Sounds fine.


> 1. Where should I best put the two new files? In `gio/win32`?

That is very inconsistently done but the subdir sounds best to me.

> 2. How do I add them to the build? Certainly I have to add a flag somewhere
> so GCC uses a current C++ compiler for `gwinnotification.cpp`?

GCC can't use notifications. MINGW still doesn't have the required headers or anything.

Created attachment 371264 [details] [review]
gwin32notificationbackend: Only warn once

Use a GOnce to make sure we only warn about notifications not being
supported on Windows once, rather than on each attempted notification.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

(In reply to Philipp from comment #21)
> As I understand, I have to write the notification code in C++. So my idea
> was to put the Windows-related code in a `gwinnotification.cpp` with
> `gwinnotification.h` containing the `extern "C"` sending and withdrawing
> functions to be used by `gwin32notificationbackend.c`.
> 
> 1. Where should I best put the two new files? In `gio/win32`?

Just put them in `gio/`. See gio/gcocoanotificationbackend.c, for example, which is written in Objective C.

So I’d call it `gio/gwin32notificationbackend.cpp` or `gio/gwin32notificationbackendprivate.cpp`.

> 2. How do I add them to the build? Certainly I have to add a flag somewhere
> so GCC uses a current C++ compiler for `gwinnotification.cpp`?

You’ll need to add AC_PROG_CXX to configure.ac. I don’t know what Meson needs you to do.

> I don’t know what Meson needs you to do.

I think so long as the project() language becomes ['c', 'cpp', etc...] and the files have typical extensions, Meson will just figure out what to do from there.

(In reply to Daniel Boles from comment #25)
> > I don’t know what Meson needs you to do.
> 
> I think so long as the project() language becomes ['c', 'cpp', etc...] and
> the files have typical extensions, Meson will just figure out what to do
> from there.

You can do `add_languages()` in a conditional to avoid the dependency when not used. Is there a C compiler that glib supports that doesn't have C++ support though? Don't think so.

By this point, I wouldn't have thought so... or hope so! But your way is probably better in any case, as it seems better to indicate that C++ is used only for one backend rather than a main part of the project (and avoid minor extra work, etc.)

(In reply to Patrick Griffis (tingping) from comment #22)
> GCC can't use notifications. MINGW still doesn't have the required headers
> or anything.

Does this mean we have to use Visual Studio to build GLib on Windows? Or can I build from MSYS2 when I just install the Windows 10 SDK?

I thought that was the point of MSYS2 -- to provide everything for Windows development without having to use Visual Studio.


(In reply to Philip Withnall from comment #24)> So I’d call it `gio/gwin32notificationbackend.cpp` or
> `gio/gwin32notificationbackendprivate.cpp`.
> 
> > 2. How do I add them to the build? Certainly I have to add a flag somewhere
> > so GCC uses a current C++ compiler for `gwinnotification.cpp`?
> 
> You’ll need to add AC_PROG_CXX to configure.ac. I don’t know what Meson
> needs you to do.

Thanks, I used the `gio` folder and the latter of the proposed filenames. I found that `AC_PROG_CC` and `AC_PROG_CPP` where already included, so I could call my simple test C++ file from the C notification backend without adding anything.

I'll ignore Meson for the moment to make progress with the implementation first. Now I'm indeed stuck with the missing Windows notification headers.

(In reply to Philipp from comment #28)
> (In reply to Patrick Griffis (tingping) from comment #22)
> > GCC can't use notifications. MINGW still doesn't have the required headers
> > or anything.
> 
> Does this mean we have to use Visual Studio to build GLib on Windows? Or can
> I build from MSYS2 when I just install the Windows 10 SDK?
> 
> I thought that was the point of MSYS2 -- to provide everything for Windows
> development without having to use Visual Studio.

Only if you want notification support. I have no clue if anybody plans on working on adding the needed headers to mingw.

(In reply to Patrick Griffis (tingping) from comment #29)
> (In reply to Philipp from comment #28)
> > Does this mean we have to use Visual Studio to build GLib on Windows? Or can
> > I build from MSYS2 when I just install the Windows 10 SDK?
> > 
> Only if you want notification support. I have no clue if anybody plans on
> working on adding the needed headers to mingw.

All right, so I installed this 20 GB IDE, but when I try to compile the compat library code mentioned in the [Windows Dev Center article], I get compilation errors like "can't find base class", "class X doesn't implement interface Y" in `winrt/wrl/implements.h` and `winrt/wrl/client.h`.

I'm a bit at a loss here and can't find anything on that problem online. If someone has more experience with Windows 10 C++ development, ideas are welcome.

[Windows Dev Center article]: https://docs.microsoft.com/en-us/windows/uwp/design/shell/tiles-and-notifications/send-local-toast-desktop-cpp-wrl

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1234.