unity-scopes-api

Merge lp:unity-scopes-api/staging into lp:unity-scopes-api

  1. staging
  2. Merge into trunk
Proposed by Marcus Tomlinson
Status: Merged
Approved by: Marcus Tomlinson
Approved revision: 334
Merged at revision: 371
Proposed branch: lp:unity-scopes-api/staging
Merge into: lp:unity-scopes-api
Diff against target: 276 lines (+70/-18)
12 files modified
data/scope-registry.conf.in (+1/-1)
data/smart-scopes-proxy.conf.in (+1/-1)
debian/control (+1/-0)
doc/tutorial.dox (+2/-1)
include/unity/scopes/internal/ConfigBase.h (+2/-0)
src/scopes/internal/ConfigBase.cpp (+19/-2)
src/scopes/internal/RegistryConfig.cpp (+2/-2)
src/scopes/internal/RuntimeConfig.cpp (+4/-4)
src/scopes/internal/ScopeConfig.cpp (+11/-4)
src/scopes/internal/Utils.cpp (+15/-2)
test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp (+1/-1)
test/gtest/scopes/internal/Utils/Utils_test.cpp (+11/-0)
To merge this branch: bzr merge lp:unity-scopes-api/staging
Reviewer Review Type Date Requested Status
Michi Henning (community) Approve
Review via email: mp+309529@code.launchpad.net

Commit message

* Be stricter on user-configurable idle timeout (1s to 5min).
* Fix crash when reading scope metadata in a Turkish locale.
* Handle running inside a snap by using the $SNAP prefix.

To post a comment you must log in.
Revision history for this message
Michi Henning (michihenning) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/scope-registry.conf.in'
2--- data/scope-registry.conf.in 2014-05-07 11:45:50 +0000
3+++ data/scope-registry.conf.in 2016-11-01 09:06:25 +0000
4@@ -7,4 +7,4 @@
5 respawn
6 respawn limit 10 60
7
8-exec @CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/@UNITY_SCOPES_LIB@/scoperegistry
9+exec $SNAP/@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/@UNITY_SCOPES_LIB@/scoperegistry
10
11=== modified file 'data/smart-scopes-proxy.conf.in'
12--- data/smart-scopes-proxy.conf.in 2014-05-07 11:45:50 +0000
13+++ data/smart-scopes-proxy.conf.in 2016-11-01 09:06:25 +0000
14@@ -9,4 +9,4 @@
15
16 expect stop
17
18-exec @CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/@UNITY_SCOPES_LIB@/smartscopesproxy upstart
19+exec $SNAP/@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/@UNITY_SCOPES_LIB@/smartscopesproxy upstart
20
21=== modified file 'debian/control'
22--- debian/control 2016-09-09 06:57:47 +0000
23+++ debian/control 2016-11-01 09:06:25 +0000
24@@ -18,6 +18,7 @@
25 exuberant-ctags,
26 google-mock,
27 graphviz,
28+ language-pack-tr,
29 libaccounts-glib-dev,
30 libapparmor-dev,
31 libboost-filesystem-dev,
32
33=== modified file 'doc/tutorial.dox'
34--- doc/tutorial.dox 2016-06-15 09:16:47 +0000
35+++ doc/tutorial.dox 2016-11-01 09:06:25 +0000
36@@ -1560,7 +1560,8 @@
37
38 The `IdleTimeout` key controls how long a scope can remain idle before it is told to stop by the registry (or
39 killed if it does not stop within 4 seconds). The default idle timeout is 40 seconds, meaning that a
40-scope will be told to stop if no query was sent to it for that amount of time.
41+scope will be told to stop if no query was sent to it for that amount of time. Only values in the range 1 to
42+300 seconds are accepted.
43
44 `ResultTtl` determines how long results should be cached by the UI before they are considered "stale"
45 and should be refreshed. `None` indicates that results remain valid indefinitely; `Small` indicates
46
47=== modified file 'include/unity/scopes/internal/ConfigBase.h'
48--- include/unity/scopes/internal/ConfigBase.h 2014-11-03 05:31:30 +0000
49+++ include/unity/scopes/internal/ConfigBase.h 2016-11-01 09:06:25 +0000
50@@ -59,6 +59,7 @@
51 virtual std::string get_middleware(std::string const& group, std::string const& key) const;
52
53 protected:
54+ std::string snap_root() const;
55 void throw_ex(::std::string const& reason) const;
56 bool path_exists(::std::string const& path) const;
57
58@@ -70,6 +71,7 @@
59 private:
60 unity::util::IniParser::SPtr parser_;
61 std::string configfile_;
62+ std::string snap_root_;
63 };
64
65 } // namespace internal
66
67=== modified file 'src/scopes/internal/ConfigBase.cpp'
68--- src/scopes/internal/ConfigBase.cpp 2014-08-28 00:20:56 +0000
69+++ src/scopes/internal/ConfigBase.cpp 2016-11-01 09:06:25 +0000
70@@ -43,10 +43,22 @@
71 // If configfile is the empty string, we create a default instance that returns "Zmq" for the middleware
72 // and throws for the other methods.
73
74-ConfigBase::ConfigBase(string const& configfile, string const& dflt_file) :
75+ConfigBase::ConfigBase(string const& configfile, string const& base_dflt_file) :
76 parser_(nullptr),
77 configfile_(configfile)
78 {
79+ char const* sroot = getenv("SNAP");
80+ if (sroot)
81+ {
82+ snap_root_ = sroot;
83+ if (!snap_root_.empty() && snap_root_[snap_root_.size() - 1] != '/')
84+ {
85+ snap_root_ += '/';
86+ }
87+ }
88+
89+ const string dflt_file = base_dflt_file.empty() ? "" : snap_root_ + base_dflt_file;
90+
91 if (!configfile.empty())
92 {
93 boost::filesystem::path path(configfile);
94@@ -157,6 +169,11 @@
95 return val;
96 }
97
98+string ConfigBase::snap_root() const
99+{
100+ return snap_root_;
101+}
102+
103 void ConfigBase::throw_ex(string const& reason) const
104 {
105 string s = "\"" + configfile_ + "\": " + reason;
106@@ -219,7 +236,7 @@
107
108 void ConfigBase::to_lower(string & str)
109 {
110- locale locale("");
111+ locale locale("C"); // Use "C" to avoid the Turkish I problem
112 const ctype<char>& ct = use_facet<ctype<char> >(locale);
113 transform(str.begin(), str.end(), str.begin(),
114 bind1st(std::mem_fun(&ctype<char>::tolower), &ct));
115
116=== modified file 'src/scopes/internal/RegistryConfig.cpp'
117--- src/scopes/internal/RegistryConfig.cpp 2016-08-31 17:54:04 +0000
118+++ src/scopes/internal/RegistryConfig.cpp 2016-11-01 09:06:25 +0000
119@@ -53,7 +53,7 @@
120 identity_ = identity;
121 mw_kind_ = get_middleware(registry_config_group, mw_kind_key);
122 mw_configfile_ = get_optional_string(registry_config_group, mw_kind_ + configfile_key);
123- scope_installdir_ = get_optional_string(registry_config_group, scope_installdir_key, DFLT_SCOPE_INSTALL_DIR);
124+ scope_installdir_ = get_optional_string(registry_config_group, scope_installdir_key, snap_root() + DFLT_SCOPE_INSTALL_DIR);
125 oem_installdir_ = get_optional_string(registry_config_group, oem_installdir_key, DFLT_OEM_INSTALL_DIR);
126 click_installdir_ = get_optional_string(registry_config_group, click_installdir_key);
127 if (click_installdir_.empty())
128@@ -65,7 +65,7 @@
129 }
130 click_installdir_ = string(home) + "/.local/share/unity-scopes/";
131 }
132- scoperunner_path_ = get_optional_string(registry_config_group, scoperunner_path_key, DFLT_SCOPERUNNER_PATH);
133+ scoperunner_path_ = get_optional_string(registry_config_group, scoperunner_path_key, snap_root() + DFLT_SCOPERUNNER_PATH);
134 if (scoperunner_path_[0] != '/')
135 {
136 throw ConfigException(configfile + ": " + scoperunner_path_key + " must be an absolute path");
137
138=== modified file 'src/scopes/internal/RuntimeConfig.cpp'
139--- src/scopes/internal/RuntimeConfig.cpp 2016-02-22 01:29:01 +0000
140+++ src/scopes/internal/RuntimeConfig.cpp 2016-11-01 09:06:25 +0000
141@@ -63,11 +63,11 @@
142 if (configfile.empty()) // Default config
143 {
144 registry_identity_ = DFLT_REGISTRY_ID;
145- registry_configfile_ = DFLT_REGISTRY_INI;
146+ registry_configfile_ = snap_root() + DFLT_REGISTRY_INI;
147 ss_registry_identity_ = DFLT_SS_REGISTRY_ID;
148- ss_configfile_ = DFLT_SS_REGISTRY_INI;
149+ ss_configfile_ = snap_root() + DFLT_SS_REGISTRY_INI;
150 default_middleware_ = DFLT_MIDDLEWARE;
151- default_middleware_configfile_ = DFLT_ZMQ_MIDDLEWARE_INI;
152+ default_middleware_configfile_ = snap_root() + DFLT_ZMQ_MIDDLEWARE_INI;
153 reap_expiry_ = DFLT_REAP_EXPIRY;
154 reap_interval_ = DFLT_REAP_INTERVAL;
155 cache_directory_ = default_cache_directory();
156@@ -83,7 +83,7 @@
157 default_middleware_ = get_middleware(runtime_config_group, default_middleware_key);
158 default_middleware_configfile_ = get_optional_string(runtime_config_group,
159 default_middleware_ + default_middleware_configfile_key,
160- DFLT_MIDDLEWARE_INI);
161+ snap_root() + DFLT_MIDDLEWARE_INI);
162 reap_expiry_ = get_optional_int(runtime_config_group, reap_expiry_key, DFLT_REAP_EXPIRY);
163 if (reap_expiry_ < 1 && reap_expiry_ != -1)
164 {
165
166=== modified file 'src/scopes/internal/ScopeConfig.cpp'
167--- src/scopes/internal/ScopeConfig.cpp 2016-03-31 09:27:33 +0000
168+++ src/scopes/internal/ScopeConfig.cpp 2016-11-01 09:06:25 +0000
169@@ -115,6 +115,10 @@
170 try
171 {
172 string art = parser()->get_string(scope_config_group, art_key);
173+ if (!art.empty() && art[0] == '/')
174+ {
175+ art = snap_root() + art;
176+ }
177 art_.reset(new string(art));
178 }
179 catch (LogicException const&)
180@@ -123,6 +127,10 @@
181 try
182 {
183 string icon = parser()->get_string(scope_config_group, icon_key);
184+ if (!icon.empty() && icon[0] == '/')
185+ {
186+ icon = snap_root() + icon;
187+ }
188 icon_.reset(new string(icon));
189 }
190 catch (LogicException const&)
191@@ -173,12 +181,11 @@
192
193 idle_timeout_ = get_optional_int(scope_config_group, idle_timeout_key, DFLT_SCOPE_IDLE_TIMEOUT);
194
195- // Negative values and values greater than max int (once multiplied by 1000 (s to ms)) are illegal
196- const int max_idle_timeout = std::numeric_limits<int>::max() / 1000;
197- if ((idle_timeout_ < 0 || idle_timeout_ > max_idle_timeout) && idle_timeout_ != -1)
198+ // Values less than 1s and greater than 300s are illegal
199+ if (idle_timeout_ < 1 || idle_timeout_ > 300)
200 {
201 throw_ex("Illegal value (" + std::to_string(idle_timeout_) + ") for " + idle_timeout_key +
202- ": value must be >= 0 and <= " + std::to_string(max_idle_timeout));
203+ ": value must be >= 1 and <= 300");
204 }
205
206 results_ttl_type_ = ScopeMetadata::ResultsTtlType::None;
207
208=== modified file 'src/scopes/internal/Utils.cpp'
209--- src/scopes/internal/Utils.cpp 2015-11-25 08:55:19 +0000
210+++ src/scopes/internal/Utils.cpp 2016-11-01 09:06:25 +0000
211@@ -113,7 +113,7 @@
212
213 string uncamelcase(string const& str)
214 {
215- const locale loc("");
216+ const locale loc("C"); // Use "C" to avoid the Turkish I problem
217 if (str.size() == 0)
218 {
219 return str;
220@@ -266,7 +266,20 @@
221 }
222 else
223 {
224- result += custom_exec_arg + " ";
225+ // The path provided is already absolute. Ensure that the SNAP env var is honored.
226+
227+ char const* sroot = getenv("SNAP");
228+ std::string snap_root;
229+ if (sroot)
230+ {
231+ snap_root = sroot;
232+ if (!snap_root.empty() && snap_root[snap_root.size() - 1] != '/')
233+ {
234+ snap_root += '/';
235+ }
236+ }
237+
238+ result += snap_root + custom_exec_arg + " ";
239 }
240 }
241 result.resize(result.size() - 1);
242
243=== modified file 'test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp'
244--- test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp 2015-05-14 02:50:35 +0000
245+++ test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp 2016-11-01 09:06:25 +0000
246@@ -156,7 +156,7 @@
247 catch(ConfigException const& e)
248 {
249 boost::regex r("unity::scopes::ConfigException: \".*\": Illegal value \\(-2\\) for IdleTimeout: "
250- "value must be >= 0 and <= [0-9]+");
251+ "value must be >= 1 and <= [0-9]+");
252 EXPECT_TRUE(boost::regex_match(e.what(), r));
253 }
254 }
255
256=== modified file 'test/gtest/scopes/internal/Utils/Utils_test.cpp'
257--- test/gtest/scopes/internal/Utils/Utils_test.cpp 2015-09-15 07:36:09 +0000
258+++ test/gtest/scopes/internal/Utils/Utils_test.cpp 2016-11-01 09:06:25 +0000
259@@ -37,6 +37,17 @@
260 EXPECT_EQ("foo-bar", uncamelcase("foo-Bar"));
261 }
262
263+TEST(Utils, uncamelcase_turkish)
264+{
265+ char *old_locale = strdup(getenv("LC_ALL"));
266+ setenv("LC_ALL", "tr_TR.UTF-8", 1);
267+
268+ EXPECT_EQ("small-i", uncamelcase("smallI"));
269+
270+ setenv("LC_ALL", old_locale, 1);
271+ free(old_locale);
272+}
273+
274 TEST(Utils, convert_to)
275 {
276 {

Subscribers

People subscribed via source and target branches

to all changes: