Merge lp:~marcustomlinson/unity-scopes-api/fix_relative_custom_exec_args into lp:unity-scopes-api/devel
- fix_relative_custom_exec_args
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Paweł Stołowski |
Approved revision: | 631 |
Merged at revision: | 631 |
Proposed branch: | lp:~marcustomlinson/unity-scopes-api/fix_relative_custom_exec_args |
Merge into: | lp:unity-scopes-api/devel |
Diff against target: |
407 lines (+184/-90) 8 files modified
include/unity/scopes/internal/Utils.h (+5/-0) scoperegistry/scoperegistry.cpp (+1/-51) src/scopes/internal/Utils.cpp (+83/-0) test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.cpp (+5/-1) test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.ini.in (+1/-1) test/gtest/scopes/internal/Utils/Utils_test.cpp (+86/-0) test/gtest/scopes/internal/zmq_middleware/RegistryI/RegistryI_test.cpp (+2/-37) unity-scopes.map (+1/-0) |
To merge this branch: | bzr merge lp:~marcustomlinson/unity-scopes-api/fix_relative_custom_exec_args |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paweł Stołowski (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Review via email: mp+270777@code.launchpad.net |
Commit message
Loop through each argument of the custom scope runner command and ensure that all path arguments are absolute
Description of the change
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:630
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 631. By Marcus Tomlinson
-
Added tests
Marcus Tomlinson (marcustomlinson) wrote : | # |
> Do you mind adding some tests for convert_
> would be good to have a check for arguments which contain escaped spaces, e.g.
> Foo\ Bar.) in the Utils test
> (test/gtest/
> much better test than with a test scope.
> Other than that looks good and clean!
Added tests.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:631
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Paweł Stołowski (stolowski) wrote : | # |
Awesome, thanks! +1
Preview Diff
1 | === modified file 'include/unity/scopes/internal/Utils.h' | |||
2 | --- include/unity/scopes/internal/Utils.h 2015-04-10 03:55:25 +0000 | |||
3 | +++ include/unity/scopes/internal/Utils.h 2015-09-15 07:36:31 +0000 | |||
4 | @@ -22,6 +22,8 @@ | |||
5 | 22 | #include <string> | 22 | #include <string> |
6 | 23 | #include <sstream> | 23 | #include <sstream> |
7 | 24 | 24 | ||
8 | 25 | #include <boost/filesystem.hpp> | ||
9 | 26 | |||
10 | 25 | namespace unity | 27 | namespace unity |
11 | 26 | { | 28 | { |
12 | 27 | 29 | ||
13 | @@ -57,6 +59,9 @@ | |||
14 | 57 | 59 | ||
15 | 58 | void make_directories(std::string const& path_name, mode_t mode); | 60 | void make_directories(std::string const& path_name, mode_t mode); |
16 | 59 | 61 | ||
17 | 62 | std::vector<std::string> split_exec_args(std::string const& id, std::string const& custom_exec); | ||
18 | 63 | std::string convert_exec_rel_to_abs(std::string const& id, boost::filesystem::path const& scope_dir, std::string const& custom_exec); | ||
19 | 64 | |||
20 | 60 | } // namespace internal | 65 | } // namespace internal |
21 | 61 | 66 | ||
22 | 62 | } // namespace scopes | 67 | } // namespace scopes |
23 | 63 | 68 | ||
24 | === modified file 'scoperegistry/scoperegistry.cpp' | |||
25 | --- scoperegistry/scoperegistry.cpp 2015-07-01 11:02:46 +0000 | |||
26 | +++ scoperegistry/scoperegistry.cpp 2015-09-15 07:36:31 +0000 | |||
27 | @@ -259,30 +259,6 @@ | |||
28 | 259 | } | 259 | } |
29 | 260 | } | 260 | } |
30 | 261 | 261 | ||
31 | 262 | string get_first_component(string const& id, string const& custom_exec) | ||
32 | 263 | { | ||
33 | 264 | if (custom_exec.empty()) | ||
34 | 265 | { | ||
35 | 266 | throw unity::InvalidArgumentException("Invalid scope runner executable for scope: " + id); | ||
36 | 267 | } | ||
37 | 268 | |||
38 | 269 | wordexp_t exp; | ||
39 | 270 | string result; | ||
40 | 271 | |||
41 | 272 | // Split command into program and args | ||
42 | 273 | if (wordexp(custom_exec.c_str(), &exp, WRDE_NOCMD) == 0) | ||
43 | 274 | { | ||
44 | 275 | util::ResourcePtr<wordexp_t*, decltype(&wordfree)> free_guard(&exp, wordfree); | ||
45 | 276 | result = exp.we_wordv[0]; | ||
46 | 277 | } | ||
47 | 278 | else | ||
48 | 279 | { | ||
49 | 280 | throw unity::InvalidArgumentException("Invalid scope runner executable for scope: " + id); | ||
50 | 281 | } | ||
51 | 282 | |||
52 | 283 | return result; | ||
53 | 284 | } | ||
54 | 285 | |||
55 | 286 | // For each scope, open the config file for the scope, create the metadata info from the config, | 262 | // For each scope, open the config file for the scope, create the metadata info from the config, |
56 | 287 | // and add an entry to the RegistryObject. | 263 | // and add an entry to the RegistryObject. |
57 | 288 | // If the scope uses settings, also parse the settings file and add the settings to the metadata. | 264 | // If the scope uses settings, also parse the settings file and add the settings to the metadata. |
58 | @@ -435,33 +411,7 @@ | |||
59 | 435 | 411 | ||
60 | 436 | try | 412 | try |
61 | 437 | { | 413 | { |
89 | 438 | auto custom_exec = sc.scope_runner(); | 414 | exec_data.custom_exec = convert_exec_rel_to_abs(scope.first, scope_dir, sc.scope_runner()); |
63 | 439 | filesystem::path program(get_first_component( scope.first, custom_exec)); | ||
64 | 440 | if (program.is_relative()) | ||
65 | 441 | { | ||
66 | 442 | // First look inside the arch-specific directory | ||
67 | 443 | if (filesystem::exists(scope_dir / DEB_HOST_MULTIARCH / program)) | ||
68 | 444 | { | ||
69 | 445 | // Join the full command, not just the program path | ||
70 | 446 | exec_data.custom_exec = (scope_dir / DEB_HOST_MULTIARCH / custom_exec).native(); | ||
71 | 447 | } | ||
72 | 448 | // Next try in the non arch-aware directory | ||
73 | 449 | else if (filesystem::exists(scope_dir / program)) | ||
74 | 450 | { | ||
75 | 451 | // Join the full command, not just the program path | ||
76 | 452 | exec_data.custom_exec = (scope_dir / custom_exec).native(); | ||
77 | 453 | } | ||
78 | 454 | else | ||
79 | 455 | { | ||
80 | 456 | throw unity::InvalidArgumentException( | ||
81 | 457 | "Nonexistent scope runner '" + custom_exec | ||
82 | 458 | + "' executable for scope: " + scope.first); | ||
83 | 459 | } | ||
84 | 460 | } | ||
85 | 461 | else | ||
86 | 462 | { | ||
87 | 463 | exec_data.custom_exec = custom_exec; | ||
88 | 464 | } | ||
90 | 465 | } | 415 | } |
91 | 466 | catch (NotFoundException const&) | 416 | catch (NotFoundException const&) |
92 | 467 | { | 417 | { |
93 | 468 | 418 | ||
94 | === modified file 'src/scopes/internal/Utils.cpp' | |||
95 | --- src/scopes/internal/Utils.cpp 2015-04-10 03:55:25 +0000 | |||
96 | +++ src/scopes/internal/Utils.cpp 2015-09-15 07:36:31 +0000 | |||
97 | @@ -19,12 +19,14 @@ | |||
98 | 19 | #include <unity/scopes/internal/Utils.h> | 19 | #include <unity/scopes/internal/Utils.h> |
99 | 20 | #include <unity/scopes/ScopeExceptions.h> | 20 | #include <unity/scopes/ScopeExceptions.h> |
100 | 21 | #include <unity/UnityExceptions.h> | 21 | #include <unity/UnityExceptions.h> |
101 | 22 | #include <unity/util/ResourcePtr.h> | ||
102 | 22 | 23 | ||
103 | 23 | #include <boost/filesystem.hpp> | 24 | #include <boost/filesystem.hpp> |
104 | 24 | 25 | ||
105 | 25 | #include <iomanip> | 26 | #include <iomanip> |
106 | 26 | #include <locale> | 27 | #include <locale> |
107 | 27 | #include <mutex> | 28 | #include <mutex> |
108 | 29 | #include <wordexp.h> | ||
109 | 28 | 30 | ||
110 | 29 | #include <sys/stat.h> | 31 | #include <sys/stat.h> |
111 | 30 | 32 | ||
112 | @@ -190,6 +192,87 @@ | |||
113 | 190 | } | 192 | } |
114 | 191 | } | 193 | } |
115 | 192 | 194 | ||
116 | 195 | vector<string> split_exec_args(string const& id, string const& custom_exec) | ||
117 | 196 | { | ||
118 | 197 | if (custom_exec.empty()) | ||
119 | 198 | { | ||
120 | 199 | throw unity::InvalidArgumentException("Invalid empty executable for scope: '" + id + "'"); | ||
121 | 200 | } | ||
122 | 201 | |||
123 | 202 | wordexp_t exp; | ||
124 | 203 | vector<string> result; | ||
125 | 204 | |||
126 | 205 | // Split command into program and args | ||
127 | 206 | if (wordexp(custom_exec.c_str(), &exp, WRDE_NOCMD) == 0) | ||
128 | 207 | { | ||
129 | 208 | util::ResourcePtr<wordexp_t*, decltype(&wordfree)> free_guard(&exp, wordfree); | ||
130 | 209 | for (size_t i = 0; i < exp.we_wordc; ++i ) | ||
131 | 210 | { | ||
132 | 211 | auto argument = string(exp.we_wordv[i]); | ||
133 | 212 | if(argument.find_first_of(' ') != std::string::npos) | ||
134 | 213 | { | ||
135 | 214 | // This argument contains spaces, enclose it in quotation marks | ||
136 | 215 | result.emplace_back("\"" + argument + "\""); | ||
137 | 216 | } | ||
138 | 217 | else | ||
139 | 218 | { | ||
140 | 219 | result.emplace_back(argument); | ||
141 | 220 | } | ||
142 | 221 | } | ||
143 | 222 | } | ||
144 | 223 | else | ||
145 | 224 | { | ||
146 | 225 | throw unity::InvalidArgumentException("Invalid executable for scope: '" + id + "'"); | ||
147 | 226 | } | ||
148 | 227 | |||
149 | 228 | return result; | ||
150 | 229 | } | ||
151 | 230 | |||
152 | 231 | string convert_exec_rel_to_abs(string const& id, boost::filesystem::path const& scope_dir, string const& custom_exec) | ||
153 | 232 | { | ||
154 | 233 | string result; | ||
155 | 234 | |||
156 | 235 | // Loop through each argument of the scope runner command and ensure all path args are absolute | ||
157 | 236 | auto custom_exec_args = split_exec_args(id, custom_exec); | ||
158 | 237 | for (auto custom_exec_arg : custom_exec_args) | ||
159 | 238 | { | ||
160 | 239 | boost::filesystem::path argument(custom_exec_arg); | ||
161 | 240 | if (argument.is_relative()) | ||
162 | 241 | { | ||
163 | 242 | // First look inside the arch-specific directory | ||
164 | 243 | if (boost::filesystem::exists(scope_dir / DEB_HOST_MULTIARCH / argument)) | ||
165 | 244 | { | ||
166 | 245 | // Append the argument as a relative path | ||
167 | 246 | result += (scope_dir / DEB_HOST_MULTIARCH / argument).native() + " "; | ||
168 | 247 | } | ||
169 | 248 | // Next try in the non arch-aware directory | ||
170 | 249 | else if (boost::filesystem::exists(scope_dir / argument)) | ||
171 | 250 | { | ||
172 | 251 | // Append the argument as a relative path | ||
173 | 252 | result += (scope_dir / argument).native() + " "; | ||
174 | 253 | } | ||
175 | 254 | // If this is the first argument (program name) it must exist, throw here | ||
176 | 255 | else if (result.empty()) | ||
177 | 256 | { | ||
178 | 257 | throw unity::InvalidArgumentException( | ||
179 | 258 | "Nonexistent scope runner executable: '" + custom_exec_arg | ||
180 | 259 | + "' for scope: '" + id + "'"); | ||
181 | 260 | } | ||
182 | 261 | // Otherwise just append the argument as is | ||
183 | 262 | else | ||
184 | 263 | { | ||
185 | 264 | result += custom_exec_arg + " "; | ||
186 | 265 | } | ||
187 | 266 | } | ||
188 | 267 | else | ||
189 | 268 | { | ||
190 | 269 | result += custom_exec_arg + " "; | ||
191 | 270 | } | ||
192 | 271 | } | ||
193 | 272 | result.resize(result.size() - 1); | ||
194 | 273 | return result; | ||
195 | 274 | } | ||
196 | 275 | |||
197 | 193 | } // namespace internal | 276 | } // namespace internal |
198 | 194 | 277 | ||
199 | 195 | } // namespace scopes | 278 | } // namespace scopes |
200 | 196 | 279 | ||
201 | === modified file 'test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.cpp' | |||
202 | --- test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.cpp 2014-12-05 03:05:37 +0000 | |||
203 | +++ test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.cpp 2015-09-15 07:36:31 +0000 | |||
204 | @@ -25,6 +25,9 @@ | |||
205 | 25 | #include <unity/scopes/Runtime.h> | 25 | #include <unity/scopes/Runtime.h> |
206 | 26 | #include <unity/scopes/PreviewReply.h> | 26 | #include <unity/scopes/PreviewReply.h> |
207 | 27 | 27 | ||
208 | 28 | #include <boost/filesystem.hpp> | ||
209 | 29 | |||
210 | 30 | using namespace boost; | ||
211 | 28 | using namespace std; | 31 | using namespace std; |
212 | 29 | using namespace unity::scopes; | 32 | using namespace unity::scopes; |
213 | 30 | 33 | ||
214 | @@ -102,10 +105,11 @@ | |||
215 | 102 | std::string scope_config = argc > 2 ? argv[2] : "FAIL"; | 105 | std::string scope_config = argc > 2 ? argv[2] : "FAIL"; |
216 | 103 | std::string rt_config = argc > 3 ? argv[3] : "FAIL"; | 106 | std::string rt_config = argc > 3 ? argv[3] : "FAIL"; |
217 | 104 | std::string unused_arg_2 = argc > 4 ? argv[4] : "FAIL"; | 107 | std::string unused_arg_2 = argc > 4 ? argv[4] : "FAIL"; |
218 | 108 | filesystem::path abs_path_arg(argc > 5 ? argv[5] : "FAIL"); | ||
219 | 105 | 109 | ||
220 | 106 | // In order to test that custom exec splitting works, we check here that our arbitrary arguments | 110 | // In order to test that custom exec splitting works, we check here that our arbitrary arguments |
221 | 107 | // have been delivered to us as expected. | 111 | // have been delivered to us as expected. |
223 | 108 | if (unused_arg_1 == "unused arg 1" && unused_arg_2 == "unused arg 2") | 112 | if (unused_arg_1 == "unused arg 1" && unused_arg_2 == "unused arg 2" && filesystem::exists(abs_path_arg)) |
224 | 109 | { | 113 | { |
225 | 110 | MyScope scope; | 114 | MyScope scope; |
226 | 111 | auto runtime = Runtime::create_scope_runtime("testscopeB", rt_config); | 115 | auto runtime = Runtime::create_scope_runtime("testscopeB", rt_config); |
227 | 112 | 116 | ||
228 | === modified file 'test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.ini.in' | |||
229 | --- test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.ini.in 2014-12-08 15:17:04 +0000 | |||
230 | +++ test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.ini.in 2015-09-15 07:36:31 +0000 | |||
231 | @@ -6,5 +6,5 @@ | |||
232 | 6 | Icon = data/scope-B.Icon | 6 | Icon = data/scope-B.Icon |
233 | 7 | SearchHint = scope-B.SearchHint | 7 | SearchHint = scope-B.SearchHint |
234 | 8 | HotKey = scope-B.HotKey | 8 | HotKey = scope-B.HotKey |
236 | 9 | ScopeRunner = testscopeB "unused arg 1" %S %R unused' 'arg' '2 | 9 | ScopeRunner = testscopeB "unused arg 1" %S %R unused' 'arg' '2 ./testscopeB.ini |
237 | 10 | IdleTimeout = 2 | 10 | IdleTimeout = 2 |
238 | 11 | 11 | ||
239 | === modified file 'test/gtest/scopes/internal/Utils/Utils_test.cpp' | |||
240 | --- test/gtest/scopes/internal/Utils/Utils_test.cpp 2015-04-10 03:55:25 +0000 | |||
241 | +++ test/gtest/scopes/internal/Utils/Utils_test.cpp 2015-09-15 07:36:31 +0000 | |||
242 | @@ -132,3 +132,89 @@ | |||
243 | 132 | 132 | ||
244 | 133 | chmod(child.c_str(), 0777); // Don't leave the dir behind without permissions. | 133 | chmod(child.c_str(), 0777); // Don't leave the dir behind without permissions. |
245 | 134 | } | 134 | } |
246 | 135 | |||
247 | 136 | TEST(Utils, split_exec_args) | ||
248 | 137 | { | ||
249 | 138 | // Test empty executable | ||
250 | 139 | try | ||
251 | 140 | { | ||
252 | 141 | split_exec_args("test", ""); | ||
253 | 142 | FAIL(); | ||
254 | 143 | } | ||
255 | 144 | catch (std::exception const& e) | ||
256 | 145 | { | ||
257 | 146 | EXPECT_STREQ("unity::InvalidArgumentException: Invalid empty executable for scope: 'test'", e.what()); | ||
258 | 147 | } | ||
259 | 148 | |||
260 | 149 | // Test invalid executable | ||
261 | 150 | try | ||
262 | 151 | { | ||
263 | 152 | split_exec_args("test", "\""); | ||
264 | 153 | FAIL(); | ||
265 | 154 | } | ||
266 | 155 | catch (std::exception const& e) | ||
267 | 156 | { | ||
268 | 157 | EXPECT_STREQ("unity::InvalidArgumentException: Invalid executable for scope: 'test'", e.what()); | ||
269 | 158 | } | ||
270 | 159 | |||
271 | 160 | // Test argument splitting | ||
272 | 161 | auto exec_args = split_exec_args("test", "/path\\ to/exec' 'file arg \"arg 2\" arg' '3 arg\\ 4"); | ||
273 | 162 | ASSERT_EQ(5, exec_args.size()); | ||
274 | 163 | EXPECT_STREQ("\"/path to/exec file\"", exec_args[0].c_str()); | ||
275 | 164 | EXPECT_STREQ("arg", exec_args[1].c_str()); | ||
276 | 165 | EXPECT_STREQ("\"arg 2\"", exec_args[2].c_str()); | ||
277 | 166 | EXPECT_STREQ("\"arg 3\"", exec_args[3].c_str()); | ||
278 | 167 | EXPECT_STREQ("\"arg 4\"", exec_args[4].c_str()); | ||
279 | 168 | } | ||
280 | 169 | |||
281 | 170 | TEST(Utils, convert_exec_rel_to_abs) | ||
282 | 171 | { | ||
283 | 172 | // Test empty executable | ||
284 | 173 | try | ||
285 | 174 | { | ||
286 | 175 | convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), ""); | ||
287 | 176 | FAIL(); | ||
288 | 177 | } | ||
289 | 178 | catch (std::exception const& e) | ||
290 | 179 | { | ||
291 | 180 | EXPECT_STREQ("unity::InvalidArgumentException: Invalid empty executable for scope: 'test'", e.what()); | ||
292 | 181 | } | ||
293 | 182 | |||
294 | 183 | // Test invalid executable | ||
295 | 184 | try | ||
296 | 185 | { | ||
297 | 186 | convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), "\""); | ||
298 | 187 | FAIL(); | ||
299 | 188 | } | ||
300 | 189 | catch (std::exception const& e) | ||
301 | 190 | { | ||
302 | 191 | EXPECT_STREQ("unity::InvalidArgumentException: Invalid executable for scope: 'test'", e.what()); | ||
303 | 192 | } | ||
304 | 193 | |||
305 | 194 | // Test nonexistent executable | ||
306 | 195 | try | ||
307 | 196 | { | ||
308 | 197 | convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), "noexec"); | ||
309 | 198 | FAIL(); | ||
310 | 199 | } | ||
311 | 200 | catch (std::exception const& e) | ||
312 | 201 | { | ||
313 | 202 | EXPECT_STREQ("unity::InvalidArgumentException: Nonexistent scope runner executable: 'noexec' for scope: 'test'", e.what()); | ||
314 | 203 | } | ||
315 | 204 | |||
316 | 205 | // Test absolute executable | ||
317 | 206 | auto abs_exec = convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), "/bin/bash"); | ||
318 | 207 | EXPECT_STREQ("/bin/bash", abs_exec.c_str()); | ||
319 | 208 | |||
320 | 209 | // Test absolute executable w/ args | ||
321 | 210 | abs_exec = convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), "/bin/bash arg \"arg 2\" arg' '3 arg\\ 4"); | ||
322 | 211 | EXPECT_STREQ("/bin/bash arg \"arg 2\" \"arg 3\" \"arg 4\"", abs_exec.c_str()); | ||
323 | 212 | |||
324 | 213 | // Test relative executable (w/ ./) | ||
325 | 214 | abs_exec = convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), "./Utils_test"); | ||
326 | 215 | EXPECT_TRUE(boost::filesystem::exists(abs_exec)); | ||
327 | 216 | |||
328 | 217 | // Test relative executable (w/o ./) | ||
329 | 218 | abs_exec = convert_exec_rel_to_abs("test", boost::filesystem::path(TEST_DIR), "Utils_test"); | ||
330 | 219 | EXPECT_TRUE(boost::filesystem::exists(abs_exec)); | ||
331 | 220 | } | ||
332 | 135 | 221 | ||
333 | === modified file 'test/gtest/scopes/internal/zmq_middleware/RegistryI/RegistryI_test.cpp' | |||
334 | --- test/gtest/scopes/internal/zmq_middleware/RegistryI/RegistryI_test.cpp 2015-07-22 03:54:35 +0000 | |||
335 | +++ test/gtest/scopes/internal/zmq_middleware/RegistryI/RegistryI_test.cpp 2015-09-15 07:36:31 +0000 | |||
336 | @@ -28,6 +28,7 @@ | |||
337 | 28 | #include <unity/scopes/internal/ScopeMetadataImpl.h> | 28 | #include <unity/scopes/internal/ScopeMetadataImpl.h> |
338 | 29 | #include <unity/scopes/internal/ScopeImpl.h> | 29 | #include <unity/scopes/internal/ScopeImpl.h> |
339 | 30 | #include <unity/scopes/internal/UniqueID.h> | 30 | #include <unity/scopes/internal/UniqueID.h> |
340 | 31 | #include <unity/scopes/internal/Utils.h> | ||
341 | 31 | #include <unity/scopes/internal/zmq_middleware/ZmqRegistry.h> | 32 | #include <unity/scopes/internal/zmq_middleware/ZmqRegistry.h> |
342 | 32 | #include <unity/scopes/SearchMetadata.h> | 33 | #include <unity/scopes/SearchMetadata.h> |
343 | 33 | #include <unity/scopes/ScopeExceptions.h> | 34 | #include <unity/scopes/ScopeExceptions.h> |
344 | @@ -465,42 +466,6 @@ | |||
345 | 465 | return count_child_procs() - start_process_count; | 466 | return count_child_procs() - start_process_count; |
346 | 466 | } | 467 | } |
347 | 467 | 468 | ||
348 | 468 | string convert_custom_exec(fs::path const& scope_dir, string const& custom_exec) | ||
349 | 469 | { | ||
350 | 470 | string result; | ||
351 | 471 | |||
352 | 472 | vector<string> split; | ||
353 | 473 | boost::split(split, custom_exec, boost::is_space()); | ||
354 | 474 | fs::path program(split.front()); | ||
355 | 475 | if (program.is_relative()) | ||
356 | 476 | { | ||
357 | 477 | // First look inside the arch-specific directory | ||
358 | 478 | if (fs::exists(scope_dir / DEB_HOST_MULTIARCH / program)) | ||
359 | 479 | { | ||
360 | 480 | // Join the full command, not just the program path | ||
361 | 481 | result = (scope_dir / DEB_HOST_MULTIARCH / custom_exec).native(); | ||
362 | 482 | } | ||
363 | 483 | // Next try in the non arch-aware directory | ||
364 | 484 | else if (fs::exists(scope_dir / program)) | ||
365 | 485 | { | ||
366 | 486 | // Join the full command, not just the program path | ||
367 | 487 | result = (scope_dir / custom_exec).native(); | ||
368 | 488 | } | ||
369 | 489 | else | ||
370 | 490 | { | ||
371 | 491 | throw unity::InvalidArgumentException( | ||
372 | 492 | "Nonexistent scope runner '" + custom_exec | ||
373 | 493 | + "' executable for scope"); | ||
374 | 494 | } | ||
375 | 495 | } | ||
376 | 496 | else | ||
377 | 497 | { | ||
378 | 498 | result = custom_exec; | ||
379 | 499 | } | ||
380 | 500 | |||
381 | 501 | return result; | ||
382 | 502 | } | ||
383 | 503 | |||
384 | 504 | ScopeProxy start_testscopeB() | 469 | ScopeProxy start_testscopeB() |
385 | 505 | { | 470 | { |
386 | 506 | std::string test_scope_id = "testscopeB"; | 471 | std::string test_scope_id = "testscopeB"; |
387 | @@ -519,7 +484,7 @@ | |||
388 | 519 | 484 | ||
389 | 520 | RegistryObject::ScopeExecData exec_data; | 485 | RegistryObject::ScopeExecData exec_data; |
390 | 521 | exec_data.scope_id = test_scope_id; | 486 | exec_data.scope_id = test_scope_id; |
392 | 522 | exec_data.custom_exec = convert_custom_exec(REGISTRY_TEST_DIR "/scopes/testscopeB", sc.scope_runner()); | 487 | exec_data.custom_exec = convert_exec_rel_to_abs(test_scope_id, REGISTRY_TEST_DIR "/scopes/testscopeB", sc.scope_runner()); |
393 | 523 | exec_data.runtime_config = runtime_ini; | 488 | exec_data.runtime_config = runtime_ini; |
394 | 524 | exec_data.scope_config = test_scope_config; | 489 | exec_data.scope_config = test_scope_config; |
395 | 525 | exec_data.timeout_ms = process_timeout; | 490 | exec_data.timeout_ms = process_timeout; |
396 | 526 | 491 | ||
397 | === modified file 'unity-scopes.map' | |||
398 | --- unity-scopes.map 2015-04-10 03:55:25 +0000 | |||
399 | +++ unity-scopes.map 2015-09-15 07:36:31 +0000 | |||
400 | @@ -13,6 +13,7 @@ | |||
401 | 13 | virtual?thunk?to?unity::scopes::[!iu]*; | 13 | virtual?thunk?to?unity::scopes::[!iu]*; |
402 | 14 | vtable?for?unity::scopes::utility::[!i]*; | 14 | vtable?for?unity::scopes::utility::[!i]*; |
403 | 15 | vtable?for?unity::scopes::[!iu]*; | 15 | vtable?for?unity::scopes::[!iu]*; |
404 | 16 | unity::scopes::internal::convert_exec_rel_to_abs*; | ||
405 | 16 | unity::scopes::internal::Executor::*; | 17 | unity::scopes::internal::Executor::*; |
406 | 17 | unity::scopes::internal::IniSettingsSchema::*; | 18 | unity::scopes::internal::IniSettingsSchema::*; |
407 | 18 | unity::scopes::internal::JsonSettingsSchema::*; | 19 | unity::scopes::internal::JsonSettingsSchema::*; |
Do you mind adding some tests for convert_ exec_rel_ to_abs (and split_args - would be good to have a check for arguments which contain escaped spaces, e.g. Foo\ Bar.) in the Utils test (test/gtest/ scopes/ internal/ Utils/Utils_ test.cpp) ? That should allow for a much better test than with a test scope.
Other than that looks good and clean!