Merge lp:~gerboland/qtmir/desktophintenvvar into lp:qtmir

Proposed by Gerry Boland on 2017-03-14
Status: Merged
Approved by: Daniel d'Andrada on 2017-03-24
Approved revision: 620
Merged at revision: 631
Proposed branch: lp:~gerboland/qtmir/desktophintenvvar
Merge into: lp:qtmir
Diff against target: 226 lines (+75/-30)
5 files modified
src/modules/Unity/Application/application_manager.cpp (+10/-6)
src/modules/Unity/Application/proc_info.cpp (+41/-9)
src/modules/Unity/Application/proc_info.h (+12/-8)
tests/framework/mock_proc_info.cpp (+7/-5)
tests/framework/mock_proc_info.h (+5/-2)
To merge this branch: bzr merge lp:~gerboland/qtmir/desktophintenvvar
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) 2017-03-14 Approve on 2017-03-24
Unity8 CI Bot continuous-integration Approve on 2017-03-20
Review via email: mp+319829@code.launchpad.net

Commit message

Add env var equivalent to desktop_file_hint

To post a comment you must log in.
619. By Gerry Boland on 2017-03-14

Fix up a comment while I am here

Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:618
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/576/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4444
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4472
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4302
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4302/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4302
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4302/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4302
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4302/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4302
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4302/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4302
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4302/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4302
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4302/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/576/rebuild

review: Approve (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:619
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/577/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4447
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4475
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4305
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4305/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4305
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4305/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4305
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4305/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4305
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4305/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4305
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4305/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4305
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4305/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/577/rebuild

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

Please update copyright headers of proc_info.* and mock_proc_info.*

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

Other than that, code looks ok and works.

620. By Gerry Boland on 2017-03-20

Update copyrights

Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:620
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/602/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4575
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4603
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4430
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4430/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4430
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4430/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4430
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4430/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4430
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4430/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4430
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4430/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4430
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4430/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/602/rebuild

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass? If not, please explain why.
Yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/modules/Unity/Application/application_manager.cpp'
2--- src/modules/Unity/Application/application_manager.cpp 2017-02-21 18:46:30 +0000
3+++ src/modules/Unity/Application/application_manager.cpp 2017-03-20 22:49:20 +0000
4@@ -501,8 +501,8 @@
5 /*
6 * Hack: Allow applications to be launched without being managed by upstart, where AppManager
7 * itself manages processes executed with a "--desktop_file_hint=/path/to/desktopFile.desktop"
8- * parameter attached. This exists until ubuntu-app-launch can notify shell any application is
9- * and so shell should allow it.
10+ * parameter attached, or an environment variable "DESKTOP_FILE_HINT=/path/to/desktopFile.desktop".
11+ * This exists until all GUI applications are launched via ubuntu-app-launch.
12 */
13 std::unique_ptr<ProcInfo::CommandLine> info = m_procInfo->commandLine(pid);
14 if (!info) {
15@@ -516,12 +516,16 @@
16 return;
17 }
18
19- const QString desktopFileName = info->getParameter("--desktop_file_hint=");
20+ QString desktopFileName = info->getParameter("--desktop_file_hint=");
21
22 if (desktopFileName.isNull()) {
23- qCritical() << "ApplicationManager REJECTED connection from app with pid" << pid
24- << "as it was not launched by upstart, and no desktop_file_hint is specified";
25- return;
26+ auto environment = m_procInfo->environment(pid);
27+ if (!environment->contains("DESKTOP_FILE_HINT")) {
28+ qCritical() << "ApplicationManager REJECTED connection from app with pid" << pid
29+ << "as it was not launched by upstart, and no desktop_file_hint is specified";
30+ return;
31+ }
32+ desktopFileName = environment->getParameter("DESKTOP_FILE_HINT");
33 }
34
35 // Guess appId from the desktop file hint
36
37=== modified file 'src/modules/Unity/Application/proc_info.cpp'
38--- src/modules/Unity/Application/proc_info.cpp 2015-12-14 09:15:42 +0000
39+++ src/modules/Unity/Application/proc_info.cpp 2017-03-20 22:49:20 +0000
40@@ -1,5 +1,5 @@
41 /*
42- * Copyright (C) 2014-2015 Canonical, Ltd.
43+ * Copyright (C) 2014-2017 Canonical, Ltd.
44 *
45 * This program is free software: you can redistribute it and/or modify it under
46 * the terms of the GNU Lesser General Public License version 3, as published by
47@@ -23,10 +23,8 @@
48 namespace qtmir
49 {
50
51-ProcInfo::~ProcInfo() {
52-}
53-
54-std::unique_ptr<ProcInfo::CommandLine> ProcInfo::commandLine(pid_t pid) {
55+std::unique_ptr<ProcInfo::CommandLine> ProcInfo::commandLine(pid_t pid)
56+{
57 QFile cmdline(QStringLiteral("/proc/%1/cmdline").arg(pid));
58 if (!cmdline.open(QIODevice::ReadOnly | QIODevice::Text)) {
59 return nullptr;
60@@ -34,19 +32,24 @@
61
62 return std::unique_ptr<CommandLine>(new CommandLine{ cmdline.readLine().replace('\0', ' ') });
63 }
64-QStringList ProcInfo::CommandLine::asStringList() const {
65+
66+QStringList ProcInfo::CommandLine::asStringList() const
67+{
68 return QString(m_command.data()).split(' ');
69 }
70
71-bool ProcInfo::CommandLine::startsWith(char const* prefix) const {
72+bool ProcInfo::CommandLine::startsWith(char const* prefix) const
73+{
74 return m_command.startsWith(prefix);
75 }
76
77-bool ProcInfo::CommandLine::contains(char const* prefix) const {
78+bool ProcInfo::CommandLine::contains(char const* prefix) const
79+{
80 return m_command.contains(prefix);
81 }
82
83-QString ProcInfo::CommandLine::getParameter(const char* name) const {
84+QString ProcInfo::CommandLine::getParameter(const char* name) const
85+{
86 QString pattern = QRegularExpression::escape(name) + "(\\S+)";
87 QRegularExpression regExp(pattern);
88 QRegularExpressionMatch regExpMatch = regExp.match(m_command);
89@@ -58,4 +61,33 @@
90 return QString(regExpMatch.captured(1));
91 }
92
93+
94+std::unique_ptr<ProcInfo::Environment> ProcInfo::environment(pid_t pid)
95+{
96+ QFile environment(QStringLiteral("/proc/%1/environ").arg(pid));
97+ if (!environment.open(QIODevice::ReadOnly | QIODevice::Text)) {
98+ return nullptr;
99+ }
100+
101+ return std::unique_ptr<Environment>(new Environment{ environment.readLine().replace('\0', ' ') });
102+}
103+
104+bool ProcInfo::Environment::contains(char const* prefix) const
105+{
106+ return m_environment.contains(prefix);
107+}
108+
109+QString ProcInfo::Environment::getParameter(const char* name) const
110+{
111+ QString pattern = QRegularExpression::escape(name) + "=(\\S+)";
112+ QRegularExpression regExp(pattern);
113+ QRegularExpressionMatch regExpMatch = regExp.match(m_environment);
114+
115+ if (!regExpMatch.hasMatch()) {
116+ return QString();
117+ }
118+
119+ return QString(regExpMatch.captured(1));
120+}
121+
122 } // namespace qtmir
123
124=== modified file 'src/modules/Unity/Application/proc_info.h'
125--- src/modules/Unity/Application/proc_info.h 2015-10-08 11:20:30 +0000
126+++ src/modules/Unity/Application/proc_info.h 2017-03-20 22:49:20 +0000
127@@ -1,5 +1,5 @@
128 /*
129- * Copyright (C) 2014-2015 Canonical, Ltd.
130+ * Copyright (C) 2014-2017 Canonical, Ltd.
131 *
132 * This program is free software: you can redistribute it and/or modify it under
133 * the terms of the GNU Lesser General Public License version 3, as published by
134@@ -22,9 +22,6 @@
135 // std
136 #include <memory>
137
138-// boost
139-#include <boost/optional.hpp>
140-
141 // Qt
142 #include <QByteArray>
143 #include <QStringList>
144@@ -37,9 +34,7 @@
145 class ProcInfo
146 {
147 public:
148- class CommandLine
149- {
150- public:
151+ struct CommandLine {
152 QByteArray m_command;
153
154 bool startsWith(const char* prefix) const;
155@@ -47,8 +42,17 @@
156 QString getParameter(const char* name) const;
157 QStringList asStringList() const;
158 };
159+
160+ struct Environment {
161+ QByteArray m_environment;
162+
163+ bool contains(const char* prefix) const;
164+ QString getParameter(const char* name) const;
165+ };
166+
167 virtual std::unique_ptr<CommandLine> commandLine(pid_t pid);
168- virtual ~ProcInfo();
169+ virtual std::unique_ptr<Environment> environment(pid_t pid);
170+ virtual ~ProcInfo() = default;
171 };
172
173 } // namespace qtmir
174
175=== modified file 'tests/framework/mock_proc_info.cpp'
176--- tests/framework/mock_proc_info.cpp 2016-02-22 22:26:30 +0000
177+++ tests/framework/mock_proc_info.cpp 2017-03-20 22:49:20 +0000
178@@ -1,5 +1,5 @@
179 /*
180- * Copyright (C) 2015 Canonical, Ltd.
181+ * Copyright (C) 2015-2017 Canonical, Ltd.
182 *
183 * This program is free software: you can redistribute it and/or modify it under
184 * the terms of the GNU Lesser General Public License version 3, as published by
185@@ -23,10 +23,7 @@
186 {
187 using namespace ::testing;
188 ON_CALL(*this, command_line(_)).WillByDefault(Return(QByteArray()));
189-}
190-
191-MockProcInfo::~MockProcInfo()
192-{
193+ ON_CALL(*this, set_environment(_)).WillByDefault(Return(QByteArray()));
194 }
195
196 std::unique_ptr<qtmir::ProcInfo::CommandLine> MockProcInfo::commandLine(pid_t pid)
197@@ -34,4 +31,9 @@
198 return std::unique_ptr<CommandLine>(new CommandLine{command_line(pid)});
199 }
200
201+std::unique_ptr<qtmir::ProcInfo::Environment> MockProcInfo::environment(pid_t pid)
202+{
203+ return std::unique_ptr<Environment>(new Environment{set_environment(pid)});
204+}
205+
206 } // namespace qtmir
207
208=== modified file 'tests/framework/mock_proc_info.h'
209--- tests/framework/mock_proc_info.h 2016-02-22 22:26:30 +0000
210+++ tests/framework/mock_proc_info.h 2017-03-20 22:49:20 +0000
211@@ -26,10 +26,13 @@
212 struct MockProcInfo : public qtmir::ProcInfo
213 {
214 MockProcInfo();
215- virtual ~MockProcInfo();
216+ virtual ~MockProcInfo() = default;
217
218 MOCK_METHOD1(command_line, QByteArray(pid_t));
219- std::unique_ptr<CommandLine> commandLine(pid_t pid);
220+ MOCK_METHOD1(set_environment, QByteArray(pid_t));
221+
222+ std::unique_ptr<CommandLine> commandLine(pid_t pid) override;
223+ std::unique_ptr<Environment> environment(pid_t pid) override;
224 };
225
226 } // namespace qtmir

Subscribers

People subscribed via source and target branches