1328153 - Replace the dbus notifications with the notify-send system command

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Bill Gianopoulos [:WG9s]]

Bug 981146 added dbus notifications to mozbuild for Linux builds.  The problem with this is that if you do not run the freedesktop, the attempt at nofication fails with:

Notification center failed: The name org.freedesktop.Notifications was not provided by any .service files

It seems that in this case you need to install the package xfce4-notifyd, which is not installed by the bootstrap.py that is provided to install all the required build tools.

However, installing that only makes the issue worse.  Because it now can attempt the notification, this adds a new issue.  If there is no x desktop session running the notification waits for a 2 minute timeout trying to connect to a non-existent service.

It seems to me all of this can be avoided by using the notify-send system command which is part of libnotify.  libnotify is a required package for building and is therefore installed by bootstrap.py and it also does not suffer from the long timeout if no xsession is running.

I propose replacing the dbus notification code with just using the notify-send command.

Comment 1

[Bill Gianopoulos [:WG9s]]

I am working on, and currently testing a patch for this.

Comment 2

[Bill Gianopoulos [:WG9s]]

Attachment: Use notify-send for build nitifications

This fixes all of the issues for me under fedora25 using GNOME3 desktop under Wayland.  Besides the review I am looking for feedback from :evilpie to make sure this still addresses his original issue.

Comment 3

[Bill Gianopoulos [:WG9s]]

Attachment: Use notify-send for build notifications

Fix typos

Comment 4

[Bill Gianopoulos [:WG9s]]

By the way, this patch addresses the concerns form Bug 981146 comment #1.  Perhaps we should have looked more seriously at those.

Comment 5

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #4)
> By the way, this patch addresses the concerns form Bug 981146 comment #1. 
> Perhaps we should have looked more seriously at those.

Well actually it is a similar concern.  The issue is not that the Python dbus package might not be installed, but that the XFCE Notify Daemon is probably not installed.

Comment 6

[Bill Gianopoulos [:WG9s]]

Attachment: Use notify-send for build notifications

Make it clearer that notify-send is usually provided by libnotify in the error message that appears it if is absent.

Comment 7

[Tom S [:evilpie]]

I am surprised, I assumed notify-send would use the same freedesktop API.

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to Tom Schuster [:evilpie] from comment #7)
> I am surprised, I assumed notify-send would use the same freedesktop API.

I suspect it is cleverer than that and uses whatever the native interface is for your Operating System and desktop environment.   It certainly works for me without me installing the freedesktop notify daemon.

So my feedback request was kind of more like does the command:

notify-send --app-name 'Mozilla Build System' 'Mozilla Build System' 'Build complete'

work in the original environment that you did not see notifications under?

Comment 9

[Tom S [:evilpie]]

Comment on attachment 8823120 [details] [diff] [review]
Use notify-send for build notifications

Works for me right now, but this is not the same system, but still Linux Mint. I super happy about this approach if it supports more stuff.

Comment 10

[Bill Gianopoulos [:WG9s]]

OH, and for those who think the 2 instances of 'Mozilla Build System' are redundant, the first one sets the application name, which is used on the lock screen to say <app-name> has x notification(s).  The second one sets the title for the actual notification pop-up which is 2 lines the first has the title in bold and the message on the second line in normal font.

Comment 11

[Bill Gianopoulos [:WG9s]]

(In reply to Tom Schuster [:evilpie] from comment #9)
> Comment on attachment 8823120 [details] [diff] [review]
> Use notify-send for build notifications
> 
> Works for me right now, but this is not the same system, but still Linux
> Mint. I super happy about this approach if it supports more stuff.

Great otherwise i was going to come up with a way more complex patch to try the dbus if notify-send did not work and then figure out what to do about what to tell people to do to make notifications work if neither suceeded.

Comment 12

[Bill Gianopoulos [:WG9s]]

By the way I filed this Python issue upstream on the fact that the resultant log entry created by this run process is an ill formatted command.

http://bugs.python.org/issue29135

Comment 13

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #12)
> By the way I filed this Python issue upstream on the fact that the resultant
> log entry created by this run process is an ill formatted command.
> 
> http://bugs.python.org/issue29135

It turns out that run_process is actually part of the mach mixins, so I will be filing a Mozilla bug on this issues.

Comment 14

[Bill Gianopoulos [:WG9s]]

Attachment: Use notify-send for build notifications

Not sure why the previous version of this patch worked.  This version fixes the specification of the --app-name parameter to conform to the command documentation.

Comment 15

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #13)
> (In reply to Bill Gianopoulos [:WG9s] from comment #12)
> > By the way I filed this Python issue upstream on the fact that the resultant
> > log entry created by this run process is an ill formatted command.
> > 
> > http://bugs.python.org/issue29135
> 
> It turns out that run_process is actually part of the mach mixins, so I will
> be filing a Mozilla bug on this issues.

I filled bug 1328393 on this issue.

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8823275 [details] [diff] [review]
Use notify-send for build notifications

Review of attachment 8823275 [details] [diff] [review]:
-----------------------------------------------------------------

This patch on its own is fine.

There should be additional work to python/mozboot to install the package providing `notify-send` (which on Debian/Ubuntu appears to be libnotify-bin). Also, we can remove occurrences of "python-dbus" from installed packages, since we no longer need that Python package to send notifications. I'd prefer the bootstrap changes land at the same time. But I don't want to hold up your forward progress. So land this if you want.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e179dbf1d095
Change Linux notify to use the system command notify-send to avoid freedesktop dependency, r=gps

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e179dbf1d095

Comment 19

[Bill Gianopoulos [:WG9s]]

I guess we don't need to leave this open. Greg filed a followup.