Merge lp:~larryprice/ubuntu-app-launch/recursive-desktop-files into lp:ubuntu-app-launch/16.10

Proposed by Larry Price
Status: Rejected
Rejected by: Ted Gould
Proposed branch: lp:~larryprice/ubuntu-app-launch/recursive-desktop-files
Merge into: lp:ubuntu-app-launch/16.10
Diff against target: 520 lines (+179/-208)
8 files modified
libubuntu-app-launch/app-info.c (+94/-72)
libubuntu-app-launch/application-impl-base.cpp (+26/-0)
libubuntu-app-launch/application-impl-base.h (+2/-0)
libubuntu-app-launch/application-impl-legacy.cpp (+19/-63)
libubuntu-app-launch/application-impl-legacy.h (+0/-4)
libubuntu-app-launch/application-impl-libertine.cpp (+23/-68)
tests/libual-cpp-test.cc (+8/-1)
tests/libual-test.cc (+7/-0)
To merge this branch: bzr merge lp:~larryprice/ubuntu-app-launch/recursive-desktop-files
Reviewer Review Type Date Requested Status
Ted Gould (community) Needs Fixing
Review via email: mp+302029@code.launchpad.net

Commit message

Recursively sweep for desktop files for libertine and legacy applications.

Description of the change

Recursively sweep for desktop files for libertine and legacy applications.

Goes hand-in-hand with an update to liblibertine which finds desktop files recursively https://code.launchpad.net/~larryprice/libertine/recurse-apps-dir.

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

So I think this branch is gonna break when it gets merged with:

https://code.launchpad.net/~ted/ubuntu-app-launch/snappy-backend-no-snap/+merge/302025

As it removes the app-info.h file. The goal generally is to have that C code go away and be replaced with C++ code. I think that will restructure things a bunch. Also you forgot to include test-nested.desktop.

review: Needs Fixing
236. By Larry Price

adding nested

Unmerged revisions

236. By Larry Price

adding nested

235. By Larry Price

Quick tests to verify nested keyfiles are now being accepted

234. By Larry Price

moving keyfileFromPath to parent to reduce code dup

233. By Larry Price

Same treatment for legacy apps

232. By Larry Price

Updating Libertine ctor to find correct desktop file

231. By Larry Price

updating app-info to recursively find legacy apps

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libubuntu-app-launch/app-info.c'
2--- libubuntu-app-launch/app-info.c 2015-08-12 02:52:05 +0000
3+++ libubuntu-app-launch/app-info.c 2016-08-04 14:10:59 +0000
4@@ -200,27 +200,67 @@
5 }
6 }
7
8-/* Look to see if the app id results in a desktop file, if so, fill in the params */
9 static gboolean
10-evaluate_dir (const gchar * dir, const gchar * desktop, gchar ** appdir, gchar ** appdesktop)
11-{
12- char * fulldir = g_build_filename(dir, "applications", desktop, NULL);
13- gboolean found = FALSE;
14-
15- if (g_file_test(fulldir, G_FILE_TEST_EXISTS)) {
16- if (appdir != NULL) {
17- *appdir = g_strdup(dir);
18- }
19-
20- if (appdesktop != NULL) {
21- *appdesktop = g_strdup_printf("applications/%s", desktop);
22- }
23-
24- found = TRUE;
25- }
26-
27- g_free(fulldir);
28- return found;
29+find_desktop_file (const gchar * base_path, const gchar * rel_path, const gchar * desktop_file, gchar ** appdesktop)
30+{
31+ gchar* full_path = g_build_filename(base_path, rel_path, NULL);
32+ gchar* full_filename = g_build_filename(full_path, desktop_file, NULL);
33+
34+ if (g_file_test(full_filename, G_FILE_TEST_IS_REGULAR)) {
35+ if (appdesktop != NULL) {
36+ *appdesktop = g_build_filename(rel_path, desktop_file, NULL);
37+ }
38+
39+ g_free(full_filename);
40+ g_free(full_path);
41+ return TRUE;
42+ }
43+ g_free(full_filename);
44+
45+ GError* error = NULL;
46+ GDir* dir = g_dir_open(full_path, 0, &error);
47+ if (error != NULL) {
48+ g_error_free(error);
49+ g_free(full_path);
50+ return FALSE;
51+ }
52+
53+ const gchar* files;
54+ while ((files = g_dir_read_name(dir)) != NULL)
55+ {
56+ gchar* file = g_build_filename(full_path, files, NULL);
57+ if (g_file_test(file, G_FILE_TEST_IS_DIR))
58+ {
59+ gchar* file_rel_path = g_build_filename(rel_path, files, NULL);
60+ if (find_desktop_file(base_path, file_rel_path, desktop_file, appdesktop))
61+ {
62+ g_free(file_rel_path);
63+ g_free(file);
64+ g_free(dir);
65+ g_free(full_path);
66+ return TRUE;
67+ }
68+ g_free(file_rel_path);
69+ }
70+ g_free(file);
71+ }
72+
73+ g_free(dir);
74+ g_free(full_path);
75+
76+ return FALSE;
77+}
78+
79+gboolean
80+find_app_info (const gchar * base_path, const gchar * desktop_file, gchar ** appdir, gchar ** appdesktop)
81+{
82+ if (find_desktop_file(base_path, "applications", desktop_file, appdesktop)) {
83+ if (appdir != NULL) {
84+ *appdir = g_strdup(base_path);
85+ }
86+ return TRUE;
87+ }
88+ return FALSE;
89 }
90
91 /* Handle the legacy case where we look through the data directories */
92@@ -228,22 +268,20 @@
93 app_info_legacy (const gchar * appid, gchar ** appdir, gchar ** appdesktop)
94 {
95 gchar * desktop = g_strdup_printf("%s.desktop", appid);
96-
97- /* Special case the user's dir */
98- if (evaluate_dir(g_get_user_data_dir(), desktop, appdir, appdesktop)) {
99- g_free(desktop);
100- return TRUE;
101- }
102-
103- const char * const * data_dirs = g_get_system_data_dirs();
104- int i;
105- for (i = 0; data_dirs[i] != NULL; i++) {
106- if (evaluate_dir(data_dirs[i], desktop, appdir, appdesktop)) {
107- g_free(desktop);
108+ if (find_app_info(g_get_user_data_dir(), desktop, appdir, appdesktop)) {
109+ g_free(desktop);
110+ return TRUE;
111+ }
112+
113+ const char * const * data_dirs = g_get_system_data_dirs();
114+ for (int i = 0; data_dirs[i] != NULL; i++) {
115+ if (find_app_info(data_dirs[i], desktop, appdir, appdesktop)) {
116+ g_free(desktop);
117 return TRUE;
118- }
119+ }
120 }
121
122+ g_free(desktop);
123 return FALSE;
124 }
125
126@@ -259,45 +297,29 @@
127 }
128
129 gchar * desktopname = g_strdup_printf("%s.desktop", app);
130-
131- gchar * desktopdir = g_build_filename(g_get_user_cache_dir(), "libertine-container", container, "rootfs", "usr", "share", NULL);
132- gchar * desktopfile = g_build_filename(desktopdir, "applications", desktopname, NULL);
133-
134- if (!g_file_test(desktopfile, G_FILE_TEST_EXISTS)) {
135- g_free(desktopdir);
136- g_free(desktopfile);
137-
138- desktopdir = g_build_filename(g_get_user_data_dir(), "libertine-container", "user-data", container, ".local", "share", NULL);
139- desktopfile = g_build_filename(desktopdir, "applications", desktopname, NULL);
140-
141- if (!g_file_test(desktopfile, G_FILE_TEST_EXISTS)) {
142- g_free(desktopdir);
143- g_free(desktopfile);
144-
145- g_free(desktopname);
146- g_free(container);
147- g_free(app);
148-
149- return FALSE;
150- }
151- }
152-
153- if (appdir != NULL) {
154- *appdir = desktopdir;
155- } else {
156- g_free(desktopdir);
157- }
158-
159- if (appdesktop != NULL) {
160- *appdesktop = g_build_filename("applications", desktopname, NULL);
161- }
162-
163- g_free(desktopfile);
164- g_free(desktopname);
165- g_free(container);
166- g_free(app);
167-
168- return TRUE;
169+ g_free(app);
170+
171+ gchar * system_dir = g_build_filename(g_get_user_cache_dir(), "libertine-container", container, "rootfs", "usr", "share", NULL);
172+ if (find_app_info(system_dir, desktopname, appdir, appdesktop)) {
173+ g_free(system_dir);
174+ g_free(desktopname);
175+ g_free(container);
176+ return TRUE;
177+ }
178+ g_free(system_dir);
179+
180+ gchar * user_dir = g_build_filename(g_get_user_data_dir(), "libertine-container", "user-data", container, ".local", "share", NULL);
181+ if (find_app_info(user_dir, desktopname, appdir, appdesktop)) {
182+ g_free(user_dir);
183+ g_free(desktopname);
184+ g_free(container);
185+ return TRUE;
186+ }
187+ g_free(user_dir);
188+ g_free(desktopname);
189+ g_free(container);
190+
191+ return FALSE;
192 }
193
194 /* Get the information on where the desktop file is from libclick */
195
196=== modified file 'libubuntu-app-launch/application-impl-base.cpp'
197--- libubuntu-app-launch/application-impl-base.cpp 2016-02-09 22:12:32 +0000
198+++ libubuntu-app-launch/application-impl-base.cpp 2016-08-04 14:10:59 +0000
199@@ -157,6 +157,32 @@
200 return std::make_shared<BaseInstance>(appIdStr);
201 }
202
203+std::shared_ptr<GKeyFile> Base::keyfileFromPath(const gchar* pathname)
204+{
205+ if (!g_file_test(pathname, G_FILE_TEST_EXISTS))
206+ {
207+ return {};
208+ }
209+
210+ std::shared_ptr<GKeyFile> keyfile(g_key_file_new(), [](GKeyFile* keyfile) {
211+ if (keyfile != nullptr)
212+ {
213+ g_key_file_free(keyfile);
214+ }
215+ });
216+ GError* error = nullptr;
217+
218+ g_key_file_load_from_file(keyfile.get(), pathname, G_KEY_FILE_NONE, &error);
219+
220+ if (error != nullptr)
221+ {
222+ g_error_free(error);
223+ return {};
224+ }
225+
226+ return keyfile;
227+}
228+
229 }; // namespace app_impls
230 }; // namespace app_launch
231 }; // namespace ubuntu
232
233=== modified file 'libubuntu-app-launch/application-impl-base.h'
234--- libubuntu-app-launch/application-impl-base.h 2016-02-09 22:12:32 +0000
235+++ libubuntu-app-launch/application-impl-base.h 2016-08-04 14:10:59 +0000
236@@ -45,6 +45,8 @@
237
238 protected:
239 std::shared_ptr<Registry> _registry;
240+
241+ static std::shared_ptr<GKeyFile> keyfileFromPath(const gchar* pathname);
242 };
243
244 }; // namespace app_impls
245
246=== modified file 'libubuntu-app-launch/application-impl-legacy.cpp'
247--- libubuntu-app-launch/application-impl-legacy.cpp 2016-05-02 13:59:09 +0000
248+++ libubuntu-app-launch/application-impl-legacy.cpp 2016-08-04 14:10:59 +0000
249@@ -19,6 +19,10 @@
250
251 #include "application-impl-legacy.h"
252 #include "application-info-desktop.h"
253+extern "C"
254+{
255+#include "app-info.h"
256+}
257
258 namespace ubuntu
259 {
260@@ -27,77 +31,29 @@
261 namespace app_impls
262 {
263
264-std::pair<std::string, std::shared_ptr<GKeyFile>> keyfileForApp(const AppID::AppName& name);
265-
266-void clear_keyfile(GKeyFile* keyfile)
267-{
268- if (keyfile != nullptr)
269- {
270- g_key_file_free(keyfile);
271- }
272-}
273-
274-Legacy::Legacy(const AppID::AppName& appname,
275- const std::string& basedir,
276- const std::shared_ptr<GKeyFile>& keyfile,
277- const std::shared_ptr<Registry>& registry)
278- : Base(registry)
279- , _appname(appname)
280- , _basedir(basedir)
281- , _keyfile(keyfile)
282-{
283- if (!_keyfile)
284- throw std::runtime_error{"Unable to find keyfile for legacy application: " + appname.value()};
285-}
286-
287 Legacy::Legacy(const AppID::AppName& appname, const std::shared_ptr<Registry>& registry)
288 : Base(registry)
289 , _appname(appname)
290 {
291- std::tie(_basedir, _keyfile) = keyfileForApp(appname);
292+ gchar* basedir = NULL;
293+ gchar* keyfile = NULL;
294+ if (!app_info_legacy(appname.value().c_str(), &basedir, &keyfile))
295+ {
296+ throw std::runtime_error{"Unable to find keyfile for legacy application: " + appname.value()};
297+ }
298+
299+ _basedir = basedir;
300+ gchar* full_filename = g_build_filename(basedir, keyfile, nullptr);
301+ _keyfile = keyfileFromPath(full_filename);
302+
303+ g_free(full_filename);
304+ g_free(keyfile);
305+ g_free(basedir);
306
307 if (!_keyfile)
308+ {
309 throw std::runtime_error{"Unable to find keyfile for legacy application: " + appname.value()};
310-}
311-
312-std::pair<std::string, std::shared_ptr<GKeyFile>> keyfileForApp(const AppID::AppName& name)
313-{
314- std::string desktopName = name.value() + ".desktop";
315- auto keyfilecheck = [desktopName](const std::string& dir) -> std::shared_ptr<GKeyFile> {
316- auto fullname = g_build_filename(dir.c_str(), "applications", desktopName.c_str(), nullptr);
317- if (!g_file_test(fullname, G_FILE_TEST_EXISTS))
318- {
319- g_free(fullname);
320- return {};
321- }
322-
323- auto keyfile = std::shared_ptr<GKeyFile>(g_key_file_new(), clear_keyfile);
324-
325- GError* error = nullptr;
326- g_key_file_load_from_file(keyfile.get(), fullname, G_KEY_FILE_NONE, &error);
327- g_free(fullname);
328-
329- if (error != nullptr)
330- {
331- g_debug("Unable to load keyfile '%s' becuase: %s", desktopName.c_str(), error->message);
332- g_error_free(error);
333- return {};
334- }
335-
336- return keyfile;
337- };
338-
339- std::string basedir = g_get_user_data_dir();
340- auto retval = keyfilecheck(basedir);
341-
342- auto systemDirs = g_get_system_data_dirs();
343- for (auto i = 0; !retval && systemDirs[i] != nullptr; i++)
344- {
345- basedir = systemDirs[i];
346- retval = keyfilecheck(basedir);
347 }
348-
349- return std::make_pair(basedir, retval);
350 }
351
352 std::shared_ptr<Application::Info> Legacy::info()
353
354=== modified file 'libubuntu-app-launch/application-impl-legacy.h'
355--- libubuntu-app-launch/application-impl-legacy.h 2016-05-02 13:47:11 +0000
356+++ libubuntu-app-launch/application-impl-legacy.h 2016-08-04 14:10:59 +0000
357@@ -34,10 +34,6 @@
358 {
359 public:
360 Legacy(const AppID::AppName& appname, const std::shared_ptr<Registry>& registry);
361- Legacy(const AppID::AppName& appname,
362- const std::string& basedir,
363- const std::shared_ptr<GKeyFile>& keyfile,
364- const std::shared_ptr<Registry>& registry);
365
366 AppID appId() override
367 {
368
369=== modified file 'libubuntu-app-launch/application-impl-libertine.cpp'
370--- libubuntu-app-launch/application-impl-libertine.cpp 2016-05-02 15:16:29 +0000
371+++ libubuntu-app-launch/application-impl-libertine.cpp 2016-08-04 14:10:59 +0000
372@@ -19,7 +19,11 @@
373
374 #include "application-impl-libertine.h"
375 #include "application-info-desktop.h"
376-#include "libertine.h"
377+extern "C"
378+{
379+#include "app-info.h"
380+}
381+#include <libertine.h>
382
383 namespace ubuntu
384 {
385@@ -28,8 +32,6 @@
386 namespace app_impls
387 {
388
389-std::shared_ptr<GKeyFile> keyfileFromPath(const gchar* pathname);
390-
391 Libertine::Libertine(const AppID::Package& container,
392 const AppID::AppName& appname,
393 const std::shared_ptr<Registry>& registry)
394@@ -37,73 +39,26 @@
395 , _container(container)
396 , _appname(appname)
397 {
398- if (!_keyfile)
399- {
400- auto container_path = libertine_container_path(container.value().c_str());
401- auto container_app_path = g_build_filename(container_path, "usr", "share", "applications",
402- (appname.value() + ".desktop").c_str(), nullptr);
403-
404- _keyfile = keyfileFromPath(container_app_path);
405-
406- if (_keyfile)
407- {
408- auto gbasedir = g_build_filename(container_path, "usr", "share", nullptr);
409- _basedir = gbasedir;
410- g_free(gbasedir);
411- }
412-
413- g_free(container_app_path);
414- g_free(container_path);
415- }
416-
417- if (!_keyfile)
418- {
419- auto home_path = libertine_container_home_path(container.value().c_str());
420- auto home_app_path = g_build_filename(home_path, ".local", "share", "applications",
421- (appname.value() + ".desktop").c_str(), NULL);
422-
423- _keyfile = keyfileFromPath(home_app_path);
424-
425- if (_keyfile)
426- {
427- auto gbasedir = g_build_filename(home_path, ".local", "share", nullptr);
428- _basedir = gbasedir;
429- g_free(gbasedir);
430- }
431-
432- g_free(home_app_path);
433- g_free(home_path);
434- }
435-
436- if (!_keyfile)
437+ gchar* basedir = NULL;
438+ gchar* keyfile = NULL;
439+ if (!app_info_libertine(ubuntu_app_launch_triplet_to_app_id(container.value().c_str(), appname.value().c_str(), "0.0"), &basedir, &keyfile))
440+ {
441+ throw std::runtime_error{"Unable to find a keyfile for application '" + appname.value() + "' in container '" +
442+ container.value() + "'"};
443+ }
444+ _basedir = basedir;
445+ gchar* full_filename = g_build_filename(basedir, keyfile, nullptr);
446+ _keyfile = keyfileFromPath(full_filename);
447+
448+ g_free(full_filename);
449+ g_free(keyfile);
450+ g_free(basedir);
451+
452+ if (!_keyfile)
453+ {
454 throw std::runtime_error{"Unable to find a keyfile for application '" + appname.value() + "' in container '" +
455 container.value() + "'"};
456-}
457-
458-std::shared_ptr<GKeyFile> keyfileFromPath(const gchar* pathname)
459-{
460- if (!g_file_test(pathname, G_FILE_TEST_EXISTS))
461- {
462- return {};
463- }
464-
465- std::shared_ptr<GKeyFile> keyfile(g_key_file_new(), [](GKeyFile* keyfile) {
466- if (keyfile != nullptr)
467- {
468- g_key_file_free(keyfile);
469- }
470- });
471- GError* error = nullptr;
472-
473- g_key_file_load_from_file(keyfile.get(), pathname, G_KEY_FILE_NONE, &error);
474-
475- if (error != nullptr)
476- {
477- g_error_free(error);
478- return {};
479- }
480-
481- return keyfile;
482+ }
483 }
484
485 std::list<std::shared_ptr<Application>> Libertine::list(const std::shared_ptr<Registry>& registry)
486
487=== modified file 'tests/libual-cpp-test.cc'
488--- tests/libual-cpp-test.cc 2016-04-27 14:22:00 +0000
489+++ tests/libual-cpp-test.cc 2016-08-04 14:10:59 +0000
490@@ -1596,6 +1596,13 @@
491
492 auto info = libertine->info();
493 EXPECT_TRUE((bool)info);
494-
495 EXPECT_EQ("Test", info->name().value());
496+
497+ /* Correct values for nested libertine */
498+ auto nestedid = ubuntu::app_launch::AppID::parse("container-name_test-nested_0.0");
499+ auto nested = ubuntu::app_launch::Application::create(libertineid, registry);
500+
501+ auto nestedinfo = nested->info();
502+ EXPECT_TRUE((bool)info);
503+ EXPECT_EQ("Test Nested", info->name().value());
504 }
505
506=== modified file 'tests/libual-test.cc'
507--- tests/libual-test.cc 2015-12-18 17:21:20 +0000
508+++ tests/libual-test.cc 2016-08-04 14:10:59 +0000
509@@ -1667,4 +1667,11 @@
510 EXPECT_STREQ("applications/test.desktop", file);
511 g_clear_pointer(&dir, g_free);
512 g_clear_pointer(&file, g_free);
513+
514+ /* Correct values for nested libertine */
515+ EXPECT_TRUE(ubuntu_app_launch_application_info("container-name_test-nested_0.0", &dir, &file));
516+ EXPECT_STREQ(CMAKE_SOURCE_DIR "/libertine-data/libertine-container/container-name/rootfs/usr/share", dir);
517+ EXPECT_STREQ("applications/nested/test-nested.desktop", file);
518+ g_clear_pointer(&dir, g_free);
519+ g_clear_pointer(&file, g_free);
520 }

Subscribers

People subscribed via source and target branches