unity-notifications

Merge lp:~lukas-kde/unity-notifications/cmdLineCheck into lp:unity-notifications

  1. cmdLineCheck
  2. Merge into trunk
Proposed by Lukáš Tinkl
Status: Merged
Approved by: Michael Zanetti
Approved revision: 244
Merged at revision: 242
Proposed branch: lp:~lukas-kde/unity-notifications/cmdLineCheck
Merge into: lp:unity-notifications
Diff against target: 41 lines (+1/-12)
2 files modified
include/NotificationServer.h (+0/-1)
src/NotificationServer.cpp (+1/-11)
To merge this branch: bzr merge lp:~lukas-kde/unity-notifications/cmdLineCheck
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
Unity8 CI Bot continuous-integration Approve
Christopher Townsend Approve
Review via email: mp+305653@code.launchpad.net

Commit message

Simplify the check for cmd line (notify-send) to make it work also inside a container

Description of the change

Simplify the check for cmd line (notify-send) to make the notifications work also inside a container

Fixes lp:1623142

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:242
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/6/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/2857/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2885
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2743/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2743/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/6/rebuild

review: Needs Fixing (continuous-integration)
lp:~lukas-kde/unity-notifications/cmdLineCheck updated
242. By Lukáš Tinkl

simplify the check for cmd line (notify-send) to make it work also inside a container

243. By Lukáš Tinkl

fix the logic, oops

244. By Lukáš Tinkl

no need to check for calledFromDbus(), notify-send cmd line is all we care about

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:243
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/7/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2858
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2886
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2744/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2744
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2744/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/7/rebuild

review: Approve (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Ok, I can confirm this now works for all cases of notifications of applications inside a Libertine container including the notify-send cli.

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:244
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/8/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2859
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2887
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2745/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2745
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2745/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/8/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

makes sense to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/NotificationServer.h'
2--- include/NotificationServer.h 2016-07-27 16:46:47 +0000
3+++ include/NotificationServer.h 2016-09-13 21:23:54 +0000
4@@ -83,7 +83,6 @@
5 private:
6 void incrementCounter();
7 QString messageSender() const;
8- bool isCmdLine() const;
9
10 QSharedPointer<Notification> buildNotification(NotificationID id, const QVariantMap &hints);
11 NotificationModel &model;
12
13=== modified file 'src/NotificationServer.cpp'
14--- src/NotificationServer.cpp 2016-07-27 17:12:08 +0000
15+++ src/NotificationServer.cpp 2016-09-13 21:23:54 +0000
16@@ -167,16 +167,6 @@
17 return sender;
18 }
19
20-bool NotificationServer::isCmdLine() const {
21- if (!calledFromDBus()) {
22- return false;
23- }
24- QString sender = message().service();
25- uint pid = connection().interface()->servicePid(sender);
26- QString path = QDir(QString("/proc/%1/exe").arg(pid)).canonicalPath();
27- return (path == "/usr/bin/notify-send");
28-}
29-
30 void NotificationServer::serviceUnregistered(const QString &clientId) {
31 m_watcher.removeWatchedService(clientId);
32 const auto notifications = model.removeAllNotificationsForClient(clientId);
33@@ -288,7 +278,7 @@
34
35 if (newNotification) {
36 // Don't clean up after the command line client closes
37- if (!isCmdLine()) {
38+ if (app_name != QStringLiteral("notify-send")) {
39 m_watcher.addWatchedService(clientId);
40 }
41 model.insertNotification(notification);

Subscribers

People subscribed via source and target branches

to all changes: