Merge lp:~ted/ubuntu-app-launch/click-exec into lp:ubuntu-app-launch/13.10

Proposed by Ted Gould
Status: Superseded
Proposed branch: lp:~ted/ubuntu-app-launch/click-exec
Merge into: lp:ubuntu-app-launch/13.10
Prerequisite: lp:~ted/ubuntu-app-launch/click-exec-pre
Diff against target: 965 lines (+458/-272)
10 files modified
Makefile (+7/-3)
application-click.conf.in (+13/-8)
application-legacy.conf.in (+4/-3)
application.conf (+4/-4)
click-exec.c (+100/-0)
desktop-exec.c (+5/-222)
desktop-hook.c (+76/-18)
helpers.c (+243/-13)
helpers.h (+4/-0)
upstart-app-launch-desktop.click-hook.in (+2/-1)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/click-exec
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Colin Watson Needs Fixing
Indicator Applet Developers Pending
Review via email: mp+176570@code.launchpad.net

This proposal has been superseded by a proposal from 2013-08-09.

Commit message

Flesh out the click package execution

Description of the change

This is the implementation of the click exec code. One of the things that makes this look bigger than it is, is that we're moving some code from desktop-exec into helper.c, and using that. So basically desktop-exec and click-exec can use that same code.

Also, because of LP limitations I created one branch that includes all the other proposed merge and set it up as a prerequisite branch. This one shouldn't be merged until the others would.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Colin Watson (cjwatson) wrote :

+ cd ${HOME}/.cache/upstart-app-launch/desktop/${APP_ID}

${HOME}/.cache/upstart-app-launch/desktop/${APP_ID} is a symlink to the .desktop file, not a directory. You need to get the package directory instead and cd to that. Something like:

  CLICK_DIR="$(click pkgdir "${HOME}/.cache/upstart-app-launch/desktop/${APP_ID}")"
  cd "$CLICK_DIR"

+ CLICK_DIR=`click pkgdir ${APP_ID}`

This looks wrong; click pkgdir doesn't take app IDs (perhaps it should).

Please quote app IDs for safety.

+pre-start exec @libexecdir@/desktop-exec ${APP_ID} "${APP_URIS}"

It is odd to pass a list of URIs as a single argument separated by spaces rather than just letting the shell's word-splitting sort it out. Did you really mean to quote ${APP_URIS}? (The C code would need to be adjusted to take URIs as more than one element of argv, but that would seem much more normal anyway.) On the other hand, ${APP_ID} should be quoted.

My reading of desktop_exec_parse is that it will do the wrong thing when presented with e.g. "%%F".

review: Needs Fixing
Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2013-07-24 at 07:22 +0000, Colin Watson wrote:

> + cd ${HOME}/.cache/upstart-app-launch/desktop/${APP_ID}
>
> ${HOME}/.cache/upstart-app-launch/desktop/${APP_ID} is a symlink to the .desktop file, not a directory. You need to get the package directory instead and cd to that. Something like:
>
> CLICK_DIR="$(click pkgdir "${HOME}/.cache/upstart-app-launch/desktop/${APP_ID}")"
> cd "$CLICK_DIR"

That's really confusing. Doesn't that mean that the click packaging
system knows about desktop files then? Why would it be dependent on the
representation of the data? It seems like the packaging system should
only know about the manifest file, so it wouldn't have enough
information to link to a particular file there.

> + CLICK_DIR=`click pkgdir ${APP_ID}`
>
> This looks wrong; click pkgdir doesn't take app IDs (perhaps it should).

Ah, okay. I got confused there. I actually had it with package name in
an earlier revision and I thought you changed it. Undoing that commit
in r70.

> Please quote app IDs for safety.

Done in r71

> +pre-start exec @libexecdir@/desktop-exec ${APP_ID} "${APP_URIS}"
>
> It is odd to pass a list of URIs as a single argument separated by spaces rather than just letting the shell's word-splitting sort it out. Did you really mean to quote ${APP_URIS}? (The C code would need to be adjusted to take URIs as more than one element of argv, but that would seem much more normal anyway.) On the other hand, ${APP_ID} should be quoted.

Yes, I've struggled with how that should be done. Since it is always
passed through the Upstart jobs as a single variable it seemed weird to
have the shell expand it in just that case. For instance, you can't
start a job without making it a single variable. Consistency between
what was passed into the job and what was passed to the exec script
seemed logical.

> My reading of desktop_exec_parse is that it will do the wrong thing when presented with e.g. "%%F".

Yes! Fixed in r72.

lp:~ted/ubuntu-app-launch/click-exec updated
70. By Ted Gould

Revert to passing the click package to pkgdir

71. By Ted Gould

Quoting APP_ID variable

72. By Ted Gould

Handling the case of %%F

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~ted/ubuntu-app-launch/click-exec updated
73. By Ted Gould

Updating for latest click-hook

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Colin Watson (cjwatson) wrote :
Download full text (3.5 KiB)

On Wed, Jul 24, 2013 at 04:22:37PM -0000, Ted Gould wrote:
> On Wed, 2013-07-24 at 07:22 +0000, Colin Watson wrote:
> > + cd ${HOME}/.cache/upstart-app-launch/desktop/${APP_ID}
> >
> > ${HOME}/.cache/upstart-app-launch/desktop/${APP_ID} is a symlink to the .desktop file, not a directory. You need to get the package directory instead and cd to that. Something like:
> >
> > CLICK_DIR="$(click pkgdir "${HOME}/.cache/upstart-app-launch/desktop/${APP_ID}")"
> > cd "$CLICK_DIR"
>
> That's really confusing. Doesn't that mean that the click packaging
> system knows about desktop files then? Why would it be dependent on the
> representation of the data?

No, it doesn't know about desktop files and is not dependent on the
representation of the data. With the exception of the temporary "click
desktophook" command, there is absolutely nothing in click with any
knowledge of desktop files; that was a design requirement I was very
clear on from the beginning, and you can verify the isolation for
yourself with grep.

You have to declare the location of the desktop file in the manifest one
way or another, and that information clearly has to be passed to the
hook somehow. It's simplest to pass that by way of linking to the
desktop file whose location is declared in the manifest. This means
that in the simple case where you don't need any postprocessing but only
need to drop the file that's the object of the hook somewhere, you can
write a hook that doesn't need an Exec line at all because it can just
tell click to symlink the object into the right place. It also
simplifies hooks that just need to do fairly simple sed-style
postprocessing because all they need to do is walk two directories,
apply substitutions to any files that are missing in the target
directory or that are newer in the source than the target, and remove
any files from the target that aren't in the source.

If you need to get the directory from the
hook-object-file-within-directory, you can always use "click pkgdir" to
get it; but I think it's likely that quite a few simple hooks won't need
to care about this, and in any case linking to the
hook-object-file-within-directory is more informative in that it's
easier to go from that path to the containing package directory than it
is to take the containing directory, parse the manifest to find the
entry for the hook in question, walk all its applications, and find each
of their hook-object-files. The latter approach makes particularly
little sense to me given that click has just had to do much the same
parsing itself to figure out which hooks to execute.

> > +pre-start exec @libexecdir@/desktop-exec ${APP_ID} "${APP_URIS}"
> >
> > It is odd to pass a list of URIs as a single argument separated by spaces rather than just letting the shell's word-splitting sort it out. Did you really mean to quote ${APP_URIS}? (The C code would need to be adjusted to take URIs as more than one element of argv, but that would seem much more normal anyway.) On the other hand, ${APP_ID} should be quoted.
>
>
> Yes, I've struggled with how that should be done. Since it is always
> passed through the Upstart jobs as a single variable it seemed wei...

Read more...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ted/ubuntu-app-launch/click-exec updated
74. By Ted Gould

Grabbing the click-hook updates

75. By Ted Gould

Switch to getting the directory from click

76. By Ted Gould

Find the directory from click

77. By Ted Gould

Switching to have the click-exec program set the directory

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ted/ubuntu-app-launch/click-exec updated
78. By Ted Gould

Fix a lagging pkglibexecdir

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ted/ubuntu-app-launch/click-exec updated
79. By Ted Gould

Grabbing changes dropping the click- prefix

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~ted/ubuntu-app-launch/click-exec updated
80. By Ted Gould

Update to latest click hook

81. By Ted Gould

Adding an introduction to explain the code a bit.

82. By Ted Gould

Merging trunk to resolve conflicts

83. By Ted Gould

Merging updated click-hook

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2013-08-07 19:14:25 +0000
3+++ Makefile 2013-08-07 19:14:25 +0000
4@@ -1,8 +1,11 @@
5-default: desktop-exec desktop-hook lsapp zg-report-app application.conf application-click.conf application-legacy.conf debian/upstart-app-launch-desktop.click-hook
6+default: desktop-exec desktop-hook click-exec lsapp zg-report-app application.conf application-click.conf application-legacy.conf debian/upstart-app-launch-desktop.click-hook
7 @echo "Building"
8
9-desktop-exec: desktop-exec.c
10- gcc -o desktop-exec desktop-exec.c `pkg-config --cflags --libs glib-2.0 gio-2.0` -Wall -Werror
11+desktop-exec: desktop-exec.c helpers.h helpers.c
12+ gcc -o desktop-exec desktop-exec.c helpers.c `pkg-config --cflags --libs glib-2.0 gio-2.0 json-glib-1.0` -Wall -Werror
13+
14+click-exec: click-exec.c helpers.h helpers.c
15+ gcc -o click-exec click-exec.c helpers.c `pkg-config --cflags --libs glib-2.0 gio-2.0 json-glib-1.0` -Wall -Werror
16
17 desktop-hook: desktop-hook.c helpers.c helpers.h
18 gcc -o desktop-hook desktop-hook.c helpers.c `pkg-config --cflags --libs glib-2.0 gio-2.0 json-glib-1.0` -Wall -Werror
19@@ -29,6 +32,7 @@
20 install -m 644 application-click.conf $(DESTDIR)/usr/share/upstart/sessions/
21 mkdir -p $(DESTDIR)/usr/lib/$(DEB_BUILD_MULTIARCH)/upstart-app-launch/
22 install -m 755 desktop-exec $(DESTDIR)/usr/lib/$(DEB_BUILD_MULTIARCH)/upstart-app-launch/
23+ install -m 755 click-exec $(DESTDIR)/usr/lib/$(DEB_BUILD_MULTIARCH)/upstart-app-launch/
24 install -m 755 desktop-hook $(DESTDIR)/usr/lib/$(DEB_BUILD_MULTIARCH)/upstart-app-launch/
25 install -m 755 zg-report-app $(DESTDIR)/usr/lib/$(DEB_BUILD_MULTIARCH)/upstart-app-launch/
26 mkdir -p $(DESTDIR)/usr/bin/
27
28=== modified file 'application-click.conf.in'
29--- application-click.conf.in 2013-08-07 19:14:25 +0000
30+++ application-click.conf.in 2013-08-07 19:14:25 +0000
31@@ -9,15 +9,20 @@
32 env APP_ID
33 export APP_ID
34
35-# TODO: This doesn't do anything really...
36+env APP_URIS=""
37+env APP_DIR="/"
38+
39+apparmor switch ${APP_ID}
40+
41+env APP_EXEC="echo Error"
42+
43+pre-start exec @pkglibexecdir@/click-exec "${APP_ID}" "${APP_URIS}"
44
45 script
46- if [ -d /opt/com.canonical.click/${APP_ID} ] ; then
47- exit 0
48- else
49- exit 1
50- fi
51+ cd ${APP_DIR}
52+ PATH=.:${PATH}
53+ exec ${APP_EXEC}
54 end script
55
56-post-start exec @pkglibexecdir@/zg-report-app open application://${APP_ID}.desktop
57-post-stop exec @pkglibexecdir@/zg-report-app close application://${APP_ID}.desktop
58+post-start exec @pkglibexecdir@/zg-report-app open "application://${APP_ID}.desktop"
59+post-stop exec @pkglibexecdir@/zg-report-app close "application://${APP_ID}.desktop"
60
61=== modified file 'application-legacy.conf.in'
62--- application-legacy.conf.in 2013-08-07 19:14:25 +0000
63+++ application-legacy.conf.in 2013-08-07 19:14:25 +0000
64@@ -8,8 +8,9 @@
65
66 env APP_EXEC="echo Error"
67 env APP_EXEC_POLICY
68+env APP_URIS
69
70-pre-start exec @pkglibexecdir@/desktop-exec ${APP_ID}
71+pre-start exec @pkglibexecdir@/desktop-exec "${APP_ID}" "${APP_URIS}"
72
73 script
74 if [ -z $APP_EXEC_POLICY ]; then
75@@ -19,5 +20,5 @@
76 fi
77 end script
78
79-post-start exec @pkglibexecdir@/zg-report-app open application://${APP_ID}.desktop
80-post-stop exec @pkglibexecdir@/zg-report-app close application://${APP_ID}.desktop
81+post-start exec @pkglibexecdir@/zg-report-app open "application://${APP_ID}.desktop"
82+post-stop exec @pkglibexecdir@/zg-report-app close "application://${APP_ID}.desktop"
83
84=== modified file 'application.conf'
85--- application.conf 2013-08-07 19:14:25 +0000
86+++ application.conf 2013-08-07 19:14:25 +0000
87@@ -13,15 +13,15 @@
88 export APP_URIS
89
90 script
91- CLICK_PKG=`echo ${APP_ID} | cut -d _ -f 1`
92+ CLICK_PKG=`echo "${APP_ID}" | cut -d _ -f 1`
93
94 if [ ! -z $CLICK_PKG ] ; then
95- CLICK_DIR=`click pkgdir ${CLICK_PKG}`
96+ CLICK_DIR=`click pkgdir "${CLICK_PKG}"`
97 fi
98
99 if [ ! -z $CLICK_DIR ] && [ -d $CLICK_DIR ] ; then
100- initctl emit application-click-start APP_ID=${APP_ID} APP_URIS="${APP_URIS}"
101+ initctl emit application-click-start APP_ID="${APP_ID}" APP_URIS="${APP_URIS}"
102 else
103- initctl emit application-legacy-start APP_ID=${APP_ID} INSTANCE_ID=`date -u +%s` APP_URIS="${APP_URIS}"
104+ initctl emit application-legacy-start APP_ID="${APP_ID}" INSTANCE_ID=`date -u +%s` APP_URIS="${APP_URIS}"
105 fi
106 end script
107
108=== added file 'click-exec.c'
109--- click-exec.c 1970-01-01 00:00:00 +0000
110+++ click-exec.c 2013-08-07 19:14:25 +0000
111@@ -0,0 +1,100 @@
112+/*
113+ * Copyright 2013 Canonical Ltd.
114+ *
115+ * This program is free software: you can redistribute it and/or modify it
116+ * under the terms of the GNU General Public License version 3, as published
117+ * by the Free Software Foundation.
118+ *
119+ * This program is distributed in the hope that it will be useful, but
120+ * WITHOUT ANY WARRANTY; without even the implied warranties of
121+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
122+ * PURPOSE. See the GNU General Public License for more details.
123+ *
124+ * You should have received a copy of the GNU General Public License along
125+ * with this program. If not, see <http://www.gnu.org/licenses/>.
126+ *
127+ * Authors:
128+ * Ted Gould <ted.gould@canonical.com>
129+ */
130+
131+#include <glib.h>
132+#include "helpers.h"
133+
134+int
135+main (int argc, char * argv[])
136+{
137+ if (argc != 2 && argc != 3) {
138+ g_error("Should be called as: %s <app_id> [uri list]", argv[0]);
139+ return 1;
140+ }
141+
142+ GError * error = NULL;
143+ gchar * package = NULL;
144+ /* 'Parse' the App ID */
145+ if (!app_id_to_triplet(argv[1], &package, NULL, NULL)) {
146+ return 1;
147+ }
148+
149+ /* Check click to find out where the files are */
150+ gchar * cmdline = g_strdup_printf("click pkgdir \"%s\"", package);
151+ g_free(package);
152+
153+ gchar * output = NULL;
154+ g_spawn_command_line_sync(cmdline, &output, NULL, NULL, &error);
155+ g_free(cmdline);
156+
157+ /* If we have an extra newline, we can delete it. */
158+ gchar * newline = g_strstr_len(output, -1, "\n");
159+ if (newline != NULL) {
160+ newline[0] = '\0';
161+ }
162+
163+ if (error != NULL) {
164+ g_warning("Unable to get the package directory from click: %s", error->message);
165+ g_error_free(error);
166+ g_free(output); /* Probably not set, but just in case */
167+ return 1;
168+ }
169+
170+ if (!g_file_test(output, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
171+ g_warning("Application directory '%s' doesn't exist", output);
172+ g_free(output);
173+ return 1;
174+ }
175+
176+ g_debug("Setting 'APP_DIR' to '%s'", output);
177+ set_upstart_variable("APP_DIR", output);
178+
179+ gchar * desktopfile = manifest_to_desktop(output, argv[1]);
180+ g_free(output);
181+ if (desktopfile == NULL) {
182+ g_warning("Desktop file unable to be found");
183+ return 1;
184+ }
185+
186+ GKeyFile * keyfile = g_key_file_new();
187+
188+ g_key_file_load_from_file(keyfile, desktopfile, 0, &error);
189+ if (error != NULL) {
190+ g_warning("Unable to load desktop file '%s': %s", desktopfile, error->message);
191+ g_error_free(error);
192+ return 1;
193+ }
194+
195+ gchar * exec = desktop_to_exec(keyfile, desktopfile);
196+ if (exec == NULL) {
197+ return 1;
198+ }
199+
200+ gchar * parsedexec = desktop_exec_parse(exec, argc == 3 ? argv[2] : NULL);
201+
202+ g_debug("Setting 'APP_EXEC' to '%s'", parsedexec);
203+ set_upstart_variable("APP_EXEC", parsedexec);
204+
205+ g_free(parsedexec);
206+ g_free(exec);
207+ g_key_file_unref(keyfile);
208+ g_free(desktopfile);
209+
210+ return 0;
211+}
212
213=== modified file 'desktop-exec.c'
214--- desktop-exec.c 2013-08-07 19:14:25 +0000
215+++ desktop-exec.c 2013-08-07 19:14:25 +0000
216@@ -22,6 +22,8 @@
217 #include <glib.h>
218 #include <gio/gio.h>
219
220+#include "helpers.h"
221+
222 gboolean verify_keyfile (GKeyFile * inkeyfile, const gchar * desktop);
223
224 /* Try to find a desktop file in a particular data directory */
225@@ -69,225 +71,6 @@
226 return TRUE;
227 }
228
229-static gchar *
230-uri2file (const gchar * uri)
231-{
232- GError * error = NULL;
233- gchar * retval = g_filename_from_uri(uri, NULL, &error);
234-
235- if (error != NULL) {
236- g_warning("Unable to resolve '%s' to a filename: %s", uri, error->message);
237- g_error_free(error);
238- }
239-
240- if (retval == NULL) {
241- retval = g_strdup("");
242- }
243-
244- g_debug("Converting URI '%s' to file '%s'", uri, retval);
245- return retval;
246-}
247-
248-static void
249-free_string (gpointer value)
250-{
251- gchar ** str = (gchar **)value;
252- g_free(*str);
253- return;
254-}
255-
256-static gchar *
257-build_file_list (const gchar * uri_list)
258-{
259- gchar ** uri_split = g_strsplit(uri_list, " ", 0);
260-
261- GArray * outarray = g_array_new(TRUE, FALSE, sizeof(gchar *));
262- g_array_set_clear_func(outarray, free_string);
263-
264- int i;
265- for (i = 0; uri_split[i] != NULL; i++) {
266- gchar * path = uri2file(uri_split[i]);
267- g_array_append_val(outarray, path);
268- }
269-
270- gchar * filelist = g_strjoinv(" ", (gchar **)outarray->data);
271- g_array_free(outarray, TRUE);
272-
273- g_strfreev(uri_split);
274-
275- return filelist;
276-}
277-
278-/* Make sure we have the single URI variable */
279-static inline void
280-ensure_singleuri (gchar ** single_uri, const gchar * uri_list)
281-{
282- if (uri_list == NULL) {
283- return;
284- }
285-
286- if (*single_uri != NULL) {
287- return;
288- }
289-
290- *single_uri = g_strdup(uri_list);
291- g_utf8_strchr(*single_uri, -1, ' ')[0] = '\0';
292-
293- return;
294-}
295-
296-/* Make sure we have a single file variable */
297-static inline void
298-ensure_singlefile (gchar ** single_file, gchar ** single_uri, const gchar * uri_list)
299-{
300- if (uri_list == NULL) {
301- return;
302- }
303-
304- if (*single_file != NULL) {
305- return;
306- }
307-
308- ensure_singleuri(single_uri, uri_list);
309-
310- if (single_uri != NULL) {
311- *single_file = uri2file(*single_uri);
312- }
313-
314- return;
315-}
316-
317-
318-static gchar *
319-handle_codes (const gchar * execline, const gchar * uri_list)
320-{
321- gchar ** execsplit = g_strsplit(execline, "%", 0);
322-
323- /* If we didn't have any codes, just exit here */
324- if (execsplit[1] == NULL) {
325- g_strfreev(execsplit);
326- return g_strdup(execline);
327- }
328-
329- int i;
330- gchar * single_uri = NULL;
331- gchar * single_file = NULL;
332- gchar * file_list = NULL;
333- GArray * outarray = g_array_new(TRUE, FALSE, sizeof(const gchar *));
334- g_array_append_val(outarray, execsplit[0]);
335-
336- /* The variables allowed in an exec line from the Freedesktop.org Desktop
337- File specification: http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables */
338- for (i = 1; execsplit[i] != NULL; i++) {
339- const gchar * skipchar = &(execsplit[i][1]);
340-
341- switch (execsplit[i][0]) {
342- case '\0': {
343- const gchar * percent = "%";
344- g_array_append_val(outarray, percent); /* %% is the literal */
345- break;
346- }
347- case 'd':
348- case 'D':
349- case 'n':
350- case 'N':
351- case 'v':
352- case 'm':
353- /* Deprecated */
354- g_array_append_val(outarray, skipchar);
355- break;
356- case 'f':
357- ensure_singlefile(&single_file, &single_uri, uri_list);
358-
359- if (single_file != NULL) {
360- g_array_append_val(outarray, single_file);
361- }
362-
363- g_array_append_val(outarray, skipchar);
364- break;
365- case 'F':
366- if (uri_list != NULL) {
367- if (file_list == NULL) {
368- file_list = build_file_list(uri_list);
369- }
370- g_array_append_val(outarray, file_list);
371- }
372-
373- g_array_append_val(outarray, skipchar);
374- break;
375- case 'i':
376- case 'c':
377- case 'k':
378- /* Perhaps? Not sure anyone uses these */
379- g_array_append_val(outarray, skipchar);
380- break;
381- case 'U':
382- if (uri_list != NULL) {
383- g_array_append_val(outarray, uri_list);
384- }
385- g_array_append_val(outarray, skipchar);
386- break;
387- case 'u':
388- ensure_singleuri(&single_uri, uri_list);
389-
390- if (single_uri != NULL) {
391- g_array_append_val(outarray, single_uri);
392- }
393-
394- g_array_append_val(outarray, skipchar);
395- break;
396- default:
397- g_warning("Desktop Exec line code '%%%c' unknown, skipping.", execsplit[i][0]);
398- g_array_append_val(outarray, skipchar);
399- break;
400- }
401- }
402-
403- gchar * output = g_strjoinv(" ", (gchar **)outarray->data);
404- g_array_free(outarray, TRUE);
405-
406- g_free(single_uri);
407- g_free(single_file);
408- g_free(file_list);
409- g_strfreev(execsplit);
410-
411- return output;
412-}
413-
414-/* Set an environment variable in Upstart */
415-static void
416-set_variable (const gchar * variable, const gchar * value)
417-{
418- GError * error = NULL;
419- gchar * command[4] = {
420- "initctl",
421- "set-env",
422- NULL,
423- NULL
424- };
425-
426- gchar * variablestr = g_strdup_printf("%s=%s", variable, value);
427- command[2] = variablestr;
428-
429- g_spawn_sync(NULL, /* working directory */
430- command,
431- NULL, /* environment */
432- G_SPAWN_SEARCH_PATH,
433- NULL, NULL, /* child setup */
434- NULL, /* stdout */
435- NULL, /* stderr */
436- NULL, /* exit status */
437- &error);
438-
439- if (error != NULL) {
440- g_warning("Unable to set variable '%s' to '%s': %s", variable, value, error->message);
441- g_error_free(error);
442- }
443-
444- g_free(variablestr);
445- return;
446-}
447-
448 int
449 main (int argc, char * argv[])
450 {
451@@ -316,7 +99,7 @@
452 gchar * execline = g_key_file_get_string(keyfile, "Desktop Entry", "Exec", NULL);
453 g_return_val_if_fail(execline != NULL, 1);
454
455- gchar * codeexec = handle_codes(execline, argc == 3 ? argv[2] : NULL);
456+ gchar * codeexec = desktop_exec_parse(execline, argc == 3 ? argv[2] : NULL);
457 if (codeexec != NULL) {
458 g_free(execline);
459 execline = codeexec;
460@@ -324,10 +107,10 @@
461
462 gchar * apparmor = g_key_file_get_string(keyfile, "Desktop Entry", "XCanonicalAppArmorProfile", NULL);
463 if (apparmor != NULL) {
464- set_variable("APP_EXEC_POLICY", apparmor);
465+ set_upstart_variable("APP_EXEC_POLICY", apparmor);
466 }
467
468- set_variable("APP_EXEC", execline);
469+ set_upstart_variable("APP_EXEC", execline);
470
471 g_key_file_free(keyfile);
472 g_free(desktop);
473
474=== modified file 'desktop-hook.c'
475--- desktop-hook.c 2013-08-07 19:14:25 +0000
476+++ desktop-hook.c 2013-08-07 19:14:25 +0000
477@@ -82,10 +82,19 @@
478 void
479 add_click_package (const gchar * dir, const gchar * name, GArray * app_array)
480 {
481- app_state_t * state = find_app_entry(name, app_array);
482+ if (!g_str_has_suffix(name, ".desktop")) {
483+ return;
484+ }
485+
486+ gchar * appid = g_strdup(name);
487+ g_strstr_len(appid, -1, ".desktop")[0] = '\0';
488+
489+ app_state_t * state = find_app_entry(appid, app_array);
490 state->has_click = TRUE;
491 state->click_created = creation_time(dir, name);
492
493+ g_free(appid);
494+
495 return;
496 }
497
498@@ -97,13 +106,15 @@
499 return;
500 }
501
502- if (!g_str_has_prefix(name, "click-")) {
503- return;
504- }
505-
506- gchar * appid = g_strdup(name + strlen("click-"));
507+ gchar * appid = g_strdup(name);
508 g_strstr_len(appid, -1, ".desktop")[0] = '\0';
509
510+ /* We only want valid APP IDs as desktop files */
511+ if (!app_id_to_triplet(appid, NULL, NULL, NULL)) {
512+ g_free(appid);
513+ return;
514+ }
515+
516 app_state_t * state = find_app_entry(appid, app_array);
517 state->has_desktop = TRUE;
518 state->desktop_created = creation_time(dir, name);
519@@ -161,14 +172,14 @@
520
521 if (g_key_file_has_key(keyfile, "Desktop Entry", "Path", NULL)) {
522 gchar * oldpath = g_key_file_get_string(keyfile, "Desktop Entry", "Path", NULL);
523- g_debug("Desktop file '%s' has a Path set to '%s'. Setting as XCanonicalOldPath.", from, oldpath);
524+ g_debug("Desktop file '%s' has a Path set to '%s'. Setting as X-Ubuntu-Old-Path.", from, oldpath);
525
526- g_key_file_set_string(keyfile, "Desktop Entry", "XCanonicalOldPath", oldpath);
527+ g_key_file_set_string(keyfile, "Desktop Entry", "X-Ubuntu-Old-Path", oldpath);
528
529 g_free(oldpath);
530 }
531
532- gchar * path = g_build_filename(appdir, app_id, NULL);
533+ gchar * path = g_build_filename(appdir, NULL);
534 g_key_file_set_string(keyfile, "Desktop Entry", "Path", path);
535 g_free(path);
536
537@@ -177,6 +188,8 @@
538 g_free(newexec);
539 g_free(oldexec);
540
541+ g_key_file_set_string(keyfile, "Desktop Entry", "X-Ubuntu-Application-ID", app_id);
542+
543 gsize datalen = 0;
544 gchar * data = g_key_file_to_data(keyfile, &datalen, &error);
545 g_key_file_unref(keyfile);
546@@ -203,25 +216,56 @@
547 static void
548 build_desktop_file (app_state_t * state, const gchar * symlinkdir, const gchar * desktopdir)
549 {
550+ GError * error = NULL;
551+ gchar * package = NULL;
552 /* 'Parse' the App ID */
553- if (!app_id_to_triplet(state->app_id, NULL, NULL, NULL)) {
554- return;
555- }
556-
557- gchar * indesktop = manifest_to_desktop(symlinkdir, state->app_id);
558+ if (!app_id_to_triplet(state->app_id, &package, NULL, NULL)) {
559+ return;
560+ }
561+
562+ /* Check click to find out where the files are */
563+ gchar * cmdline = g_strdup_printf("click pkgdir \"%s\"", package);
564+ g_free(package);
565+
566+ gchar * output = NULL;
567+ g_spawn_command_line_sync(cmdline, &output, NULL, NULL, &error);
568+ g_free(cmdline);
569+
570+ /* If we have an extra newline, we can delete it. */
571+ gchar * newline = g_strstr_len(output, -1, "\n");
572+ if (newline != NULL) {
573+ newline[0] = '\0';
574+ }
575+
576+ if (error != NULL) {
577+ g_warning("Unable to get the package directory from click: %s", error->message);
578+ g_error_free(error);
579+ g_free(output); /* Probably not set, but just in case */
580+ return;
581+ }
582+
583+ if (!g_file_test(output, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
584+ g_warning("Dirctory returned by click '%s' couldn't be found", output);
585+ g_free(output);
586+ return;
587+ }
588+
589+ gchar * indesktop = manifest_to_desktop(output, state->app_id);
590 if (indesktop == NULL) {
591+ g_free(output);
592 return;
593 }
594
595 /* Determine the desktop file name */
596- gchar * desktopfile = g_strdup_printf("click-%s.desktop", state->app_id);
597+ gchar * desktopfile = g_strdup_printf("%s.desktop", state->app_id);
598 gchar * desktoppath = g_build_filename(desktopdir, desktopfile, NULL);
599 g_free(desktopfile);
600
601- copy_desktop_file(indesktop, desktoppath, symlinkdir, state->app_id);
602+ copy_desktop_file(indesktop, desktoppath, output, state->app_id);
603
604 g_free(desktoppath);
605 g_free(indesktop);
606+ g_free(output);
607
608 return;
609 }
610@@ -230,10 +274,24 @@
611 static void
612 remove_desktop_file (app_state_t * state, const gchar * desktopdir)
613 {
614- gchar * desktopfile = g_strdup_printf("click-%s.desktop", state->app_id);
615+ gchar * desktopfile = g_strdup_printf("%s.desktop", state->app_id);
616 gchar * desktoppath = g_build_filename(desktopdir, desktopfile, NULL);
617 g_free(desktopfile);
618
619+ GKeyFile * keyfile = g_key_file_new();
620+ g_key_file_load_from_file(keyfile,
621+ desktoppath,
622+ G_KEY_FILE_NONE,
623+ NULL);
624+
625+ if (!g_key_file_has_key(keyfile, "Desktop Entry", "X-Ubuntu-Application-ID", NULL)) {
626+ g_debug("Desktop file '%s' is not one created by us.", desktoppath);
627+ g_key_file_unref(keyfile);
628+ g_free(desktoppath);
629+ return;
630+ }
631+ g_key_file_unref(keyfile);
632+
633 if (g_unlink(desktoppath) != 0) {
634 g_warning("Unable to delete desktop file: %s", desktoppath);
635 }
636@@ -254,7 +312,7 @@
637
638 GArray * apparray = g_array_new(FALSE, FALSE, sizeof(app_state_t));
639
640- /* Find all the symlinks of apps */
641+ /* Find all the symlinks of desktop files */
642 gchar * symlinkdir = g_build_filename(g_get_user_cache_dir(), "upstart-app-launch", "desktop", NULL);
643 if (!g_file_test(symlinkdir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
644 g_warning("No installed click packages");
645
646=== modified file 'helpers.c'
647--- helpers.c 2013-08-07 19:14:25 +0000
648+++ helpers.c 2013-08-07 19:14:25 +0000
649@@ -68,11 +68,12 @@
650 gchar * desktoppath = NULL;
651
652 if (!app_id_to_triplet(app_id, &package, &application, &version)) {
653+ g_warning("Unable to parse triplet: %s", app_id);
654 return NULL;
655 }
656
657 gchar * manifestfile = g_strdup_printf("%s.manifest", package);
658- gchar * manifestpath = g_build_filename(app_dir, app_id, ".click", "info", manifestfile, NULL);
659+ gchar * manifestpath = g_build_filename(app_dir, ".click", "info", manifestfile, NULL);
660 g_free(manifestfile);
661
662 if (!g_file_test(manifestpath, G_FILE_TEST_EXISTS)) {
663@@ -106,14 +107,14 @@
664 goto manifest_out;
665 }
666
667- if (!json_object_has_member(rootobj, "applications")) {
668- g_warning("Manifest '%s' doesn't have an applications section", manifestpath);
669+ if (!json_object_has_member(rootobj, "hooks")) {
670+ g_warning("Manifest '%s' doesn't have an hooks section", manifestpath);
671 goto manifest_out;
672 }
673
674- JsonObject * appsobj = json_object_get_object_member(rootobj, "applications");
675+ JsonObject * appsobj = json_object_get_object_member(rootobj, "hooks");
676 if (appsobj == NULL) {
677- g_warning("Manifest '%s' has an applications section that is not a JSON object", manifestpath);
678+ g_warning("Manifest '%s' has an hooks section that is not a JSON object", manifestpath);
679 goto manifest_out;
680 }
681
682@@ -128,19 +129,14 @@
683 goto manifest_out;
684 }
685
686- if (json_object_has_member(appobj, "type") && g_strcmp0(json_object_get_string_member(appobj, "type"), "desktop") != 0) {
687- g_warning("Manifest '%s' has a definition for application '%s' whose type is not 'desktop'", manifestpath, application);
688- goto manifest_out;
689- }
690-
691 gchar * filename = NULL;
692- if (json_object_has_member(appobj, "file")) {
693- filename = g_strdup(json_object_get_string_member(appobj, "file"));
694+ if (json_object_has_member(appobj, "desktop")) {
695+ filename = g_strdup(json_object_get_string_member(appobj, "desktop"));
696 } else {
697 filename = g_strdup_printf("%s.desktop", application);
698 }
699
700- desktoppath = g_build_filename(app_dir, app_id, filename, NULL);
701+ desktoppath = g_build_filename(app_dir, filename, NULL);
702 g_free(filename);
703
704 if (!g_file_test(desktoppath, G_FILE_TEST_EXISTS)) {
705@@ -219,3 +215,237 @@
706 return exec;
707 }
708
709+/* Sets an upstart variable, currently using initctl */
710+void
711+set_upstart_variable (const gchar * variable, const gchar * value)
712+{
713+ GError * error = NULL;
714+ gchar * command[4] = {
715+ "initctl",
716+ "set-env",
717+ NULL,
718+ NULL
719+ };
720+
721+ gchar * variablestr = g_strdup_printf("%s=%s", variable, value);
722+ command[2] = variablestr;
723+
724+ g_spawn_sync(NULL, /* working directory */
725+ command,
726+ NULL, /* environment */
727+ G_SPAWN_SEARCH_PATH,
728+ NULL, NULL, /* child setup */
729+ NULL, /* stdout */
730+ NULL, /* stderr */
731+ NULL, /* exit status */
732+ &error);
733+
734+ if (error != NULL) {
735+ g_warning("Unable to set variable '%s' to '%s': %s", variable, value, error->message);
736+ g_error_free(error);
737+ }
738+
739+ g_free(variablestr);
740+ return;
741+}
742+
743+/* Convert a URI into a file */
744+static gchar *
745+uri2file (const gchar * uri)
746+{
747+ GError * error = NULL;
748+ gchar * retval = g_filename_from_uri(uri, NULL, &error);
749+
750+ if (error != NULL) {
751+ g_warning("Unable to resolve '%s' to a filename: %s", uri, error->message);
752+ g_error_free(error);
753+ }
754+
755+ if (retval == NULL) {
756+ retval = g_strdup("");
757+ }
758+
759+ g_debug("Converting URI '%s' to file '%s'", uri, retval);
760+ return retval;
761+}
762+
763+/* free a string in an array */
764+static void
765+free_string (gpointer value)
766+{
767+ gchar ** str = (gchar **)value;
768+ g_free(*str);
769+ return;
770+}
771+
772+/* Builds the file list from the URI list */
773+static gchar *
774+build_file_list (const gchar * uri_list)
775+{
776+ gchar ** uri_split = g_strsplit(uri_list, " ", 0);
777+
778+ GArray * outarray = g_array_new(TRUE, FALSE, sizeof(gchar *));
779+ g_array_set_clear_func(outarray, free_string);
780+
781+ int i;
782+ for (i = 0; uri_split[i] != NULL; i++) {
783+ gchar * path = uri2file(uri_split[i]);
784+ g_array_append_val(outarray, path);
785+ }
786+
787+ gchar * filelist = g_strjoinv(" ", (gchar **)outarray->data);
788+ g_array_free(outarray, TRUE);
789+
790+ g_strfreev(uri_split);
791+
792+ return filelist;
793+}
794+
795+/* Make sure we have the single URI variable */
796+static inline void
797+ensure_singleuri (gchar ** single_uri, const gchar * uri_list)
798+{
799+ if (uri_list == NULL) {
800+ return;
801+ }
802+
803+ if (*single_uri != NULL) {
804+ return;
805+ }
806+
807+ *single_uri = g_strdup(uri_list);
808+ g_utf8_strchr(*single_uri, -1, ' ')[0] = '\0';
809+
810+ return;
811+}
812+
813+/* Make sure we have a single file variable */
814+static inline void
815+ensure_singlefile (gchar ** single_file, gchar ** single_uri, const gchar * uri_list)
816+{
817+ if (uri_list == NULL) {
818+ return;
819+ }
820+
821+ if (*single_file != NULL) {
822+ return;
823+ }
824+
825+ ensure_singleuri(single_uri, uri_list);
826+
827+ if (single_uri != NULL) {
828+ *single_file = uri2file(*single_uri);
829+ }
830+
831+ return;
832+}
833+
834+/* Parse a desktop exec line and return the next string */
835+gchar *
836+desktop_exec_parse (const gchar * execline, const gchar * uri_list)
837+{
838+ gchar ** execsplit = g_strsplit(execline, "%", 0);
839+
840+ /* If we didn't have any codes, just exit here */
841+ if (execsplit[1] == NULL) {
842+ g_strfreev(execsplit);
843+ return g_strdup(execline);
844+ }
845+
846+ if (uri_list != NULL && uri_list[0] == '\0') {
847+ uri_list = NULL;
848+ }
849+
850+ int i;
851+ gchar * single_uri = NULL;
852+ gchar * single_file = NULL;
853+ gchar * file_list = NULL;
854+ gboolean previous_percent = FALSE;
855+ GArray * outarray = g_array_new(TRUE, FALSE, sizeof(const gchar *));
856+ g_array_append_val(outarray, execsplit[0]);
857+
858+ /* The variables allowed in an exec line from the Freedesktop.org Desktop
859+ File specification: http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables */
860+ for (i = 1; execsplit[i] != NULL; i++) {
861+ const gchar * skipchar = &(execsplit[i][1]);
862+
863+ /* Handle the case of %%F printing "%F" */
864+ if (previous_percent) {
865+ g_array_append_val(outarray, execsplit[i]);
866+ previous_percent = FALSE;
867+ continue;
868+ }
869+
870+ switch (execsplit[i][0]) {
871+ case '\0': {
872+ const gchar * percent = "%";
873+ g_array_append_val(outarray, percent); /* %% is the literal */
874+ previous_percent = TRUE;
875+ break;
876+ }
877+ case 'd':
878+ case 'D':
879+ case 'n':
880+ case 'N':
881+ case 'v':
882+ case 'm':
883+ /* Deprecated */
884+ g_array_append_val(outarray, skipchar);
885+ break;
886+ case 'f':
887+ ensure_singlefile(&single_file, &single_uri, uri_list);
888+
889+ if (single_file != NULL) {
890+ g_array_append_val(outarray, single_file);
891+ }
892+
893+ g_array_append_val(outarray, skipchar);
894+ break;
895+ case 'F':
896+ if (uri_list != NULL) {
897+ if (file_list == NULL) {
898+ file_list = build_file_list(uri_list);
899+ }
900+ g_array_append_val(outarray, file_list);
901+ }
902+
903+ g_array_append_val(outarray, skipchar);
904+ break;
905+ case 'i':
906+ case 'c':
907+ case 'k':
908+ /* Perhaps? Not sure anyone uses these */
909+ g_array_append_val(outarray, skipchar);
910+ break;
911+ case 'U':
912+ if (uri_list != NULL) {
913+ g_array_append_val(outarray, uri_list);
914+ }
915+ g_array_append_val(outarray, skipchar);
916+ break;
917+ case 'u':
918+ ensure_singleuri(&single_uri, uri_list);
919+
920+ if (single_uri != NULL) {
921+ g_array_append_val(outarray, single_uri);
922+ }
923+
924+ g_array_append_val(outarray, skipchar);
925+ break;
926+ default:
927+ g_warning("Desktop Exec line code '%%%c' unknown, skipping.", execsplit[i][0]);
928+ g_array_append_val(outarray, skipchar);
929+ break;
930+ }
931+ }
932+
933+ gchar * output = g_strjoinv(" ", (gchar **)outarray->data);
934+ g_array_free(outarray, TRUE);
935+
936+ g_free(single_uri);
937+ g_free(single_file);
938+ g_free(file_list);
939+ g_strfreev(execsplit);
940+
941+ return output;
942+}
943
944=== modified file 'helpers.h'
945--- helpers.h 2013-08-07 19:14:25 +0000
946+++ helpers.h 2013-08-07 19:14:25 +0000
947@@ -27,4 +27,8 @@
948 const gchar * app_id);
949 gchar * desktop_to_exec (GKeyFile * desktop_file,
950 const gchar * from);
951+void set_upstart_variable (const gchar * variable,
952+ const gchar * value);
953+gchar * desktop_exec_parse (const gchar * execline,
954+ const gchar * uri_list);
955
956
957=== modified file 'upstart-app-launch-desktop.click-hook.in'
958--- upstart-app-launch-desktop.click-hook.in 2013-08-07 19:14:25 +0000
959+++ upstart-app-launch-desktop.click-hook.in 2013-08-07 19:14:25 +0000
960@@ -1,3 +1,4 @@
961-Pattern: ${home}/.cache/upstart-app-launch/desktop/${id}
962+Pattern: ${home}/.cache/upstart-app-launch/desktop/${id}.desktop
963 Exec: @pkglibexecdir@/desktop-hook ${id}
964 User-Level: yes
965+Hook-Name: desktop

Subscribers

People subscribed via source and target branches