mate-desktop / mate-notification-daemon

Daemon to display passive pop-up notifications
https://mate-desktop.org
GNU General Public License v2.0
30 stars 26 forks source link

Wayland: Allow building do not disturb applet in process #215

Closed lukefromdc closed 1 year ago

lukefromdc commented 1 year ago

Allow building the do not disturb panel applet in-process, it can then be used in wayland

lukefromdc commented 1 year ago

Note that for some reason, the code in Data/Makefile.AM for building org.mate.applets.MateNotificationApplet.desktop from org.mate.applets.MateNotificationApplet.desktop.in.in was unable to handle both writing the applet location and handling the translations. The removed code for the latter


$(applet_DATA): $(applet_in_files)
if USE_NLS
    $(AM_V_GEN) $(MSGFMT) --desktop --keyword=Name --keyword=Description --template $< -d $(top_srcdir)/po -o $@
else
    $(AM_V_GEN) cp -f $< $@
endif

overwrote the output from the former it seems, thus trashing the output from this block:

$(applet_DATA): $(applet_in_files)
    $(AM_V_GEN)sed \
        -e "s|\@APPLET_LOCATION\@|$(APPLET_LOCATION)|" \
        -e "s|\@ENABLE_IN_PROCESS\@|$(ENABLE_IN_PROCESS)|" \
        -e "s|\@VERSION\@|$(PACKAGE_VERSION)|" \
        $< > $@

I think this job is supposed to be split between the .in.in->.in and the .in to final .desktop final jobs, but code cribbed from any other applet I tried for this failed

raveit65 commented 1 year ago

When building with --disable-in-process or without new in-process flag rpm-building failed because this file is missing.

  File not found: /builddir/build/BUILDROOT/mate-notification-daemon-1.27.1-1.fc38.x86_64/usr/share/mate-panel/applets/org.mate.applets.MateNotificationApplet.mate-panel-applet

And it tries to pack a new file into the rpm.

    Installed (but unpackaged) file(s) found:
   /usr/share/mate-panel/applets/org.mate.applets.MateNotificationApplet.desktop.in

So something is wrong with out-of-process build.

Edit: Installed (but unpackaged) error message means there is a new file in build root but it's not listed in rpm spec file.

lukefromdc commented 1 year ago

I have had a lot of trouble gettting the build system right, because this one didn't originally use the same methods in data/Makefile.am to create the final org.mate.applets.MateNotificationApplet.mate-panel-applet file. An earlier version completed out of process builds OK, but in order to get the applet location to the offending file the translation code had to be entirely removed for that one file, thus the WIP designation.

The big problem is getting

$(applet_DATA): $(applet_in_files)
    $(AM_V_GEN)sed \
        -e "s|\@APPLET_LOCATION\@|$(APPLET_LOCATION)|" \
        -e "s|\@ENABLE_IN_PROCESS\@|$(ENABLE_IN_PROCESS)|" \
        -e "s|\@VERSION\@|$(PACKAGE_VERSION)|" \
        $< > $@

to properly modify org.mate.applets.MateNotificationApplet.desktop.in.in to create org.mate.applets.MateNotificationApplet.desktop.in and then for

$(applet_DATA): $(applet_in_files)
if USE_NLS
    $(AM_V_GEN) $(MSGFMT) --desktop --keyword=Name --keyword=Description --template $< -d $(top_srcdir)/po -o $@
else
    $(AM_V_GEN) cp $< $@
endif

to create the final org.mate.applets.MateNotificationApplet.mate-panel-applet file from that one and in that order. First edit the applet location (in-process or not always works, I think from being defined in configure.ac) , then the translations from the resulting file. Every other applet had a variation of the first stanza and only needed simple editing to work, this one did not and I've had endless problems getting it to work right

lukefromdc commented 1 year ago

I think I found a way to do it: use a nearly unchanged Makefile.am and define the applet location in configure.ac, a test for out of process just worked with that.

EDIT: now have that working straight from configure.ac the translations are back, and a valid output file is created in-process or out

lukefromdc commented 1 year ago

Those stanzas are the same in process and out on many other applets, only the variables fed to them changing. Documentation on doing this right with automake is seemingly impossible to find

raveit65 commented 1 year ago

Out-of-process build works fine now, but in-process build like to install a *.desktop.in file.

 Installed (but unpackaged) file(s) found:
   /usr/share/mate-panel/applets/org.mate.applets.MateNotificationApplet.desktop.in
lukefromdc commented 1 year ago

I was just unable to duplicate that. Debian and Fedora might be behaving differently here. Testing with Checkinstall (which I use to pack .debs from autotools builds), I found these two .desktop files installed and the correct file in /usr/share/mate-panel. The applet runs correctly in-process or out

in /etc/xdg/autostart: mate-notification-daemon.desktop

in /usr/share/applications: mate-notification-properties.desktop

in /usr/share/mate-panel/applets org.mate.applets.MateNotificationApplet.mate-panel-applet

If that last file gets appended with .desktop the applet probably won't run, tested this with the fish. We need to find out why we are getting different results

raveit65 commented 1 year ago

Out-of-process build works fine now, but in-process build like to install a *.desktop.in file.

 Installed (but unpackaged) file(s) found:
   /usr/share/mate-panel/applets/org.mate.applets.MateNotificationApplet.desktop.in

Sorry, my fault. I used a older tarball. It builds fine now with in-process and it works in wayland session. But i am still wondering that the package installs the the dbus service file for in-process. /usr/share/dbus-1/services/org.mate.panel.applet.MateNotificationAppletFactory.service

lukefromdc commented 1 year ago

Oh yeah-that's from the unchanged data/Makefile.am and should be simple to strip out

lukefromdc commented 1 year ago

Last force-push installs /usr/share/dbus-1/services/org.mate.panel.applet.MateNotificationAppletFactory.service only in out of process builds

raveit65 commented 1 year ago

Building works fine now and it runs in wayland session.

lukefromdc commented 1 year ago

Pretty sure we've now covered all of the applets, either allowing wayland compatable builds or marking them incompatable. That leaves some of the panel action buttons, which do not put files in /usr/share/mate-panel/applets and are loaded in another way. Such things as suspending and shutting down can be done from the command line and some time ago I was able to build a test version of the panel where clicking the shut down button was an immediate shutdown without showing the dialog. Way too easy to invoke by accident, but this means it should be possible to write a wayland session dialog that instead of invoking the session manager offers the same timeout (30 sec) and can shut down or suspend. Logout would probably be compositor-specific unless there is a wlroots-wide method of doing so already.

raveit65 commented 1 year ago

Would be nice when in-/out-of-process can be selectable in-sensor-applet. Also i noticed of mate-notification-daemon in wayland session after suspend-to-disc. M-n-d called mate-screensaver via dbus. I will file out a report about.

lukefromdc commented 1 year ago

I do not have suspend to disk capability on my encrypted system with a custom implementaton over dracut, only suspend to RAM. Will make the sensor applet selectable for in-process or out though, that should be no problem

raveit65 commented 1 year ago

Same with suspend-to-ram. I use wlogout to suspend or hibernate wayfire session. After suspend you will find this coredump in journal.

Aug 20 20:23:29 mother.mother.loc dbus-daemon[85005]: [session uid=1004 pid=85003] Activating service name='org.mate.ScreenSaver' requested by ':1.2' (uid=1004 pid=85036 comm="/usr/libexec/mate-notification-daemon")
Aug 20 20:23:29 mother.mother.loc mate-screensave[85962]: gdk_x11_window_get_xid: assertion 'GDK_IS_X11_WINDOW (window)' failed
Aug 20 20:23:29 mother.mother.loc systemd-coredump[85969]: [🡕] Process 85962 (mate-screensave) of user 1004 dumped core.

I will open a report with detail steps to reproduce. I guess it is this part in code. https://github.com/mate-desktop/mate-notification-daemon/blob/master/src/daemon/daemon.c#L1125 I tried to put this behind have_x11 flag today, but issue is still there.

lukefromdc commented 1 year ago

Probably we just need this function to exit on detecting that it is not running in x11, as wayland screenlockers do exist but are of course totally different than those for x11.

raveit65 commented 1 year ago

Done https://github.com/mate-desktop/mate-notification-daemon/issues/216

lukefromdc commented 1 year ago

I found an even simpler test: after a resume crashed m-n-d, I ran it from terminal, and got ** (mate-notification-daemon:6139): WARNING **: 21:58:47.966: Failed to get dbus connection: Error calling StartServiceByName for org.mate.ScreenSaver: Process org.mate.ScreenSaver exited with status 1 after just sending a test notification.

Turns out I didn't have the screensaver installed after a previous round of tests, so didn't see this in initial development.