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

Proposed by Ted Gould
Status: Merged
Approved by: Ted Gould
Approved revision: 83
Merged at revision: 37
Proposed branch: lp:~ted/ubuntu-app-launch/click-exec
Merge into: lp:ubuntu-app-launch/13.10
Prerequisite: lp:~ted/ubuntu-app-launch/click-hook
Diff against target: 751 lines (+389/-240)
8 files modified
Makefile (+7/-3)
application-click.conf.in (+13/-8)
application-legacy.conf.in (+4/-3)
application.conf (+4/-4)
click-exec.c (+117/-0)
desktop-exec.c (+5/-222)
helpers.c (+235/-0)
helpers.h (+4/-0)
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
Charles Kerr (community) Approve
Colin Watson Pending
Review via email: mp+179538@code.launchpad.net

This proposal supersedes a proposal from 2013-07-24.

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 : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Colin Watson (cjwatson) wrote : Posted in a previous version of this proposal

+ 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 : Posted in a previous version of this proposal

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.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Colin Watson (cjwatson) wrote : Posted in a previous version of this proposal
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 : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

The code level changes look okay.

Optional fix: in desktop-exec.c, main()'s first dozen or so lines walking through g_get_user_data_dir() and g_get_systeM_data_dirs() look like they could be replaced by a single call to g_key_file_load_from_data_dirs() which does the same thing.

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

On Sat, 2013-08-10 at 14:23 +0000, Charles Kerr wrote:

> The code level changes look okay.

Cool. I added an introduction similar to the one in desktop-hook to
explain it a bit better what was going on. r81.

> Optional fix: in desktop-exec.c, main()'s first dozen or so lines
> walking through g_get_user_data_dir() and g_get_systeM_data_dirs()
> look like they could be replaced by a single call to
> g_key_file_load_from_data_dirs() which does the same thing.

Looked at that, and started making the change. The one thing I didn't
like was that it would just load the first desktop file that it found.
If that one was malformed, it wouldn't fall back to the system one. So
it seems that a broken file in your user directory would haunt you
forever. So I'm going to leave it the way it is for now.

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) :
review: Approve (continuous-integration)
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)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

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-11 19:55:59 +0000
3+++ Makefile 2013-08-11 19:55:59 +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-11 19:55:59 +0000
30+++ application-click.conf.in 2013-08-11 19:55:59 +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-11 19:55:59 +0000
63+++ application-legacy.conf.in 2013-08-11 19:55:59 +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-07-22 17:30:20 +0000
86+++ application.conf 2013-08-11 19:55:59 +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-11 19:55:59 +0000
111@@ -0,0 +1,117 @@
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+/*
135+
136+INTRODUCTION:
137+
138+This is the utility that executes a click package based on the Application ID.
139+Actually it just determines what needs to be executed, and asks Upstart to execute
140+it so that it can be tracked better. This process runs OUTSIDE of the app armor
141+confinement for the application. It also DOES NOT use any files that can be modified
142+by the user. So things like the desktop file in ~/.local/share/applications are
143+all off limits.
144+
145+For information on Click packages and the manifest look at the Click package documentation:
146+
147+https://click-package.readthedocs.org/en/latest/
148+
149+*/
150+
151+int
152+main (int argc, char * argv[])
153+{
154+ if (argc != 2 && argc != 3) {
155+ g_error("Should be called as: %s <app_id> [uri list]", argv[0]);
156+ return 1;
157+ }
158+
159+ GError * error = NULL;
160+ gchar * package = NULL;
161+ /* 'Parse' the App ID */
162+ if (!app_id_to_triplet(argv[1], &package, NULL, NULL)) {
163+ return 1;
164+ }
165+
166+ /* Check click to find out where the files are */
167+ gchar * cmdline = g_strdup_printf("click pkgdir \"%s\"", package);
168+ g_free(package);
169+
170+ gchar * output = NULL;
171+ g_spawn_command_line_sync(cmdline, &output, NULL, NULL, &error);
172+ g_free(cmdline);
173+
174+ /* If we have an extra newline, we can delete it. */
175+ gchar * newline = g_strstr_len(output, -1, "\n");
176+ if (newline != NULL) {
177+ newline[0] = '\0';
178+ }
179+
180+ if (error != NULL) {
181+ g_warning("Unable to get the package directory from click: %s", error->message);
182+ g_error_free(error);
183+ g_free(output); /* Probably not set, but just in case */
184+ return 1;
185+ }
186+
187+ if (!g_file_test(output, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
188+ g_warning("Application directory '%s' doesn't exist", output);
189+ g_free(output);
190+ return 1;
191+ }
192+
193+ g_debug("Setting 'APP_DIR' to '%s'", output);
194+ set_upstart_variable("APP_DIR", output);
195+
196+ gchar * desktopfile = manifest_to_desktop(output, argv[1]);
197+ g_free(output);
198+ if (desktopfile == NULL) {
199+ g_warning("Desktop file unable to be found");
200+ return 1;
201+ }
202+
203+ GKeyFile * keyfile = g_key_file_new();
204+
205+ g_key_file_load_from_file(keyfile, desktopfile, 0, &error);
206+ if (error != NULL) {
207+ g_warning("Unable to load desktop file '%s': %s", desktopfile, error->message);
208+ g_error_free(error);
209+ return 1;
210+ }
211+
212+ gchar * exec = desktop_to_exec(keyfile, desktopfile);
213+ if (exec == NULL) {
214+ return 1;
215+ }
216+
217+ gchar * parsedexec = desktop_exec_parse(exec, argc == 3 ? argv[2] : NULL);
218+
219+ g_debug("Setting 'APP_EXEC' to '%s'", parsedexec);
220+ set_upstart_variable("APP_EXEC", parsedexec);
221+
222+ g_free(parsedexec);
223+ g_free(exec);
224+ g_key_file_unref(keyfile);
225+ g_free(desktopfile);
226+
227+ return 0;
228+}
229
230=== modified file 'desktop-exec.c'
231--- desktop-exec.c 2013-08-11 19:25:19 +0000
232+++ desktop-exec.c 2013-08-11 19:55:59 +0000
233@@ -22,6 +22,8 @@
234 #include <glib.h>
235 #include <gio/gio.h>
236
237+#include "helpers.h"
238+
239 gboolean verify_keyfile (GKeyFile * inkeyfile, const gchar * desktop);
240
241 /* Try to find a desktop file in a particular data directory */
242@@ -69,225 +71,6 @@
243 return TRUE;
244 }
245
246-static gchar *
247-uri2file (const gchar * uri)
248-{
249- GError * error = NULL;
250- gchar * retval = g_filename_from_uri(uri, NULL, &error);
251-
252- if (error != NULL) {
253- g_warning("Unable to resolve '%s' to a filename: %s", uri, error->message);
254- g_error_free(error);
255- }
256-
257- if (retval == NULL) {
258- retval = g_strdup("");
259- }
260-
261- g_debug("Converting URI '%s' to file '%s'", uri, retval);
262- return retval;
263-}
264-
265-static void
266-free_string (gpointer value)
267-{
268- gchar ** str = (gchar **)value;
269- g_free(*str);
270- return;
271-}
272-
273-static gchar *
274-build_file_list (const gchar * uri_list)
275-{
276- gchar ** uri_split = g_strsplit(uri_list, " ", 0);
277-
278- GArray * outarray = g_array_new(TRUE, FALSE, sizeof(gchar *));
279- g_array_set_clear_func(outarray, free_string);
280-
281- int i;
282- for (i = 0; uri_split[i] != NULL; i++) {
283- gchar * path = uri2file(uri_split[i]);
284- g_array_append_val(outarray, path);
285- }
286-
287- gchar * filelist = g_strjoinv(" ", (gchar **)outarray->data);
288- g_array_free(outarray, TRUE);
289-
290- g_strfreev(uri_split);
291-
292- return filelist;
293-}
294-
295-/* Make sure we have the single URI variable */
296-static inline void
297-ensure_singleuri (gchar ** single_uri, const gchar * uri_list)
298-{
299- if (uri_list == NULL) {
300- return;
301- }
302-
303- if (*single_uri != NULL) {
304- return;
305- }
306-
307- *single_uri = g_strdup(uri_list);
308- g_utf8_strchr(*single_uri, -1, ' ')[0] = '\0';
309-
310- return;
311-}
312-
313-/* Make sure we have a single file variable */
314-static inline void
315-ensure_singlefile (gchar ** single_file, gchar ** single_uri, const gchar * uri_list)
316-{
317- if (uri_list == NULL) {
318- return;
319- }
320-
321- if (*single_file != NULL) {
322- return;
323- }
324-
325- ensure_singleuri(single_uri, uri_list);
326-
327- if (single_uri != NULL) {
328- *single_file = uri2file(*single_uri);
329- }
330-
331- return;
332-}
333-
334-
335-static gchar *
336-handle_codes (const gchar * execline, const gchar * uri_list)
337-{
338- gchar ** execsplit = g_strsplit(execline, "%", 0);
339-
340- /* If we didn't have any codes, just exit here */
341- if (execsplit[1] == NULL) {
342- g_strfreev(execsplit);
343- return g_strdup(execline);
344- }
345-
346- int i;
347- gchar * single_uri = NULL;
348- gchar * single_file = NULL;
349- gchar * file_list = NULL;
350- GArray * outarray = g_array_new(TRUE, FALSE, sizeof(const gchar *));
351- g_array_append_val(outarray, execsplit[0]);
352-
353- /* The variables allowed in an exec line from the Freedesktop.org Desktop
354- File specification: http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables */
355- for (i = 1; execsplit[i] != NULL; i++) {
356- const gchar * skipchar = &(execsplit[i][1]);
357-
358- switch (execsplit[i][0]) {
359- case '\0': {
360- const gchar * percent = "%";
361- g_array_append_val(outarray, percent); /* %% is the literal */
362- break;
363- }
364- case 'd':
365- case 'D':
366- case 'n':
367- case 'N':
368- case 'v':
369- case 'm':
370- /* Deprecated */
371- g_array_append_val(outarray, skipchar);
372- break;
373- case 'f':
374- ensure_singlefile(&single_file, &single_uri, uri_list);
375-
376- if (single_file != NULL) {
377- g_array_append_val(outarray, single_file);
378- }
379-
380- g_array_append_val(outarray, skipchar);
381- break;
382- case 'F':
383- if (uri_list != NULL) {
384- if (file_list == NULL) {
385- file_list = build_file_list(uri_list);
386- }
387- g_array_append_val(outarray, file_list);
388- }
389-
390- g_array_append_val(outarray, skipchar);
391- break;
392- case 'i':
393- case 'c':
394- case 'k':
395- /* Perhaps? Not sure anyone uses these */
396- g_array_append_val(outarray, skipchar);
397- break;
398- case 'U':
399- if (uri_list != NULL) {
400- g_array_append_val(outarray, uri_list);
401- }
402- g_array_append_val(outarray, skipchar);
403- break;
404- case 'u':
405- ensure_singleuri(&single_uri, uri_list);
406-
407- if (single_uri != NULL) {
408- g_array_append_val(outarray, single_uri);
409- }
410-
411- g_array_append_val(outarray, skipchar);
412- break;
413- default:
414- g_warning("Desktop Exec line code '%%%c' unknown, skipping.", execsplit[i][0]);
415- g_array_append_val(outarray, skipchar);
416- break;
417- }
418- }
419-
420- gchar * output = g_strjoinv(" ", (gchar **)outarray->data);
421- g_array_free(outarray, TRUE);
422-
423- g_free(single_uri);
424- g_free(single_file);
425- g_free(file_list);
426- g_strfreev(execsplit);
427-
428- return output;
429-}
430-
431-/* Set an environment variable in Upstart */
432-static void
433-set_variable (const gchar * variable, const gchar * value)
434-{
435- GError * error = NULL;
436- gchar * command[4] = {
437- "initctl",
438- "set-env",
439- NULL,
440- NULL
441- };
442-
443- gchar * variablestr = g_strdup_printf("%s=%s", variable, value);
444- command[2] = variablestr;
445-
446- g_spawn_sync(NULL, /* working directory */
447- command,
448- NULL, /* environment */
449- G_SPAWN_SEARCH_PATH,
450- NULL, NULL, /* child setup */
451- NULL, /* stdout */
452- NULL, /* stderr */
453- NULL, /* exit status */
454- &error);
455-
456- if (error != NULL) {
457- g_warning("Unable to set variable '%s' to '%s': %s", variable, value, error->message);
458- g_error_free(error);
459- }
460-
461- g_free(variablestr);
462- return;
463-}
464-
465 int
466 main (int argc, char * argv[])
467 {
468@@ -316,7 +99,7 @@
469 gchar * execline = g_key_file_get_string(keyfile, "Desktop Entry", "Exec", NULL);
470 g_return_val_if_fail(execline != NULL, 1);
471
472- gchar * codeexec = handle_codes(execline, argc == 3 ? argv[2] : NULL);
473+ gchar * codeexec = desktop_exec_parse(execline, argc == 3 ? argv[2] : NULL);
474 if (codeexec != NULL) {
475 g_free(execline);
476 execline = codeexec;
477@@ -324,10 +107,10 @@
478
479 gchar * apparmor = g_key_file_get_string(keyfile, "Desktop Entry", "XCanonicalAppArmorProfile", NULL);
480 if (apparmor != NULL) {
481- set_variable("APP_EXEC_POLICY", apparmor);
482+ set_upstart_variable("APP_EXEC_POLICY", apparmor);
483 }
484
485- set_variable("APP_EXEC", execline);
486+ set_upstart_variable("APP_EXEC", execline);
487
488 g_key_file_free(keyfile);
489 g_free(desktop);
490
491=== modified file 'helpers.c'
492--- helpers.c 2013-08-11 19:55:59 +0000
493+++ helpers.c 2013-08-11 19:55:59 +0000
494@@ -68,6 +68,7 @@
495 gchar * desktoppath = NULL;
496
497 if (!app_id_to_triplet(app_id, &package, &application, &version)) {
498+ g_warning("Unable to parse triplet: %s", app_id);
499 return NULL;
500 }
501
502@@ -214,3 +215,237 @@
503 return exec;
504 }
505
506+/* Sets an upstart variable, currently using initctl */
507+void
508+set_upstart_variable (const gchar * variable, const gchar * value)
509+{
510+ GError * error = NULL;
511+ gchar * command[4] = {
512+ "initctl",
513+ "set-env",
514+ NULL,
515+ NULL
516+ };
517+
518+ gchar * variablestr = g_strdup_printf("%s=%s", variable, value);
519+ command[2] = variablestr;
520+
521+ g_spawn_sync(NULL, /* working directory */
522+ command,
523+ NULL, /* environment */
524+ G_SPAWN_SEARCH_PATH,
525+ NULL, NULL, /* child setup */
526+ NULL, /* stdout */
527+ NULL, /* stderr */
528+ NULL, /* exit status */
529+ &error);
530+
531+ if (error != NULL) {
532+ g_warning("Unable to set variable '%s' to '%s': %s", variable, value, error->message);
533+ g_error_free(error);
534+ }
535+
536+ g_free(variablestr);
537+ return;
538+}
539+
540+/* Convert a URI into a file */
541+static gchar *
542+uri2file (const gchar * uri)
543+{
544+ GError * error = NULL;
545+ gchar * retval = g_filename_from_uri(uri, NULL, &error);
546+
547+ if (error != NULL) {
548+ g_warning("Unable to resolve '%s' to a filename: %s", uri, error->message);
549+ g_error_free(error);
550+ }
551+
552+ if (retval == NULL) {
553+ retval = g_strdup("");
554+ }
555+
556+ g_debug("Converting URI '%s' to file '%s'", uri, retval);
557+ return retval;
558+}
559+
560+/* free a string in an array */
561+static void
562+free_string (gpointer value)
563+{
564+ gchar ** str = (gchar **)value;
565+ g_free(*str);
566+ return;
567+}
568+
569+/* Builds the file list from the URI list */
570+static gchar *
571+build_file_list (const gchar * uri_list)
572+{
573+ gchar ** uri_split = g_strsplit(uri_list, " ", 0);
574+
575+ GArray * outarray = g_array_new(TRUE, FALSE, sizeof(gchar *));
576+ g_array_set_clear_func(outarray, free_string);
577+
578+ int i;
579+ for (i = 0; uri_split[i] != NULL; i++) {
580+ gchar * path = uri2file(uri_split[i]);
581+ g_array_append_val(outarray, path);
582+ }
583+
584+ gchar * filelist = g_strjoinv(" ", (gchar **)outarray->data);
585+ g_array_free(outarray, TRUE);
586+
587+ g_strfreev(uri_split);
588+
589+ return filelist;
590+}
591+
592+/* Make sure we have the single URI variable */
593+static inline void
594+ensure_singleuri (gchar ** single_uri, const gchar * uri_list)
595+{
596+ if (uri_list == NULL) {
597+ return;
598+ }
599+
600+ if (*single_uri != NULL) {
601+ return;
602+ }
603+
604+ *single_uri = g_strdup(uri_list);
605+ g_utf8_strchr(*single_uri, -1, ' ')[0] = '\0';
606+
607+ return;
608+}
609+
610+/* Make sure we have a single file variable */
611+static inline void
612+ensure_singlefile (gchar ** single_file, gchar ** single_uri, const gchar * uri_list)
613+{
614+ if (uri_list == NULL) {
615+ return;
616+ }
617+
618+ if (*single_file != NULL) {
619+ return;
620+ }
621+
622+ ensure_singleuri(single_uri, uri_list);
623+
624+ if (single_uri != NULL) {
625+ *single_file = uri2file(*single_uri);
626+ }
627+
628+ return;
629+}
630+
631+/* Parse a desktop exec line and return the next string */
632+gchar *
633+desktop_exec_parse (const gchar * execline, const gchar * uri_list)
634+{
635+ gchar ** execsplit = g_strsplit(execline, "%", 0);
636+
637+ /* If we didn't have any codes, just exit here */
638+ if (execsplit[1] == NULL) {
639+ g_strfreev(execsplit);
640+ return g_strdup(execline);
641+ }
642+
643+ if (uri_list != NULL && uri_list[0] == '\0') {
644+ uri_list = NULL;
645+ }
646+
647+ int i;
648+ gchar * single_uri = NULL;
649+ gchar * single_file = NULL;
650+ gchar * file_list = NULL;
651+ gboolean previous_percent = FALSE;
652+ GArray * outarray = g_array_new(TRUE, FALSE, sizeof(const gchar *));
653+ g_array_append_val(outarray, execsplit[0]);
654+
655+ /* The variables allowed in an exec line from the Freedesktop.org Desktop
656+ File specification: http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables */
657+ for (i = 1; execsplit[i] != NULL; i++) {
658+ const gchar * skipchar = &(execsplit[i][1]);
659+
660+ /* Handle the case of %%F printing "%F" */
661+ if (previous_percent) {
662+ g_array_append_val(outarray, execsplit[i]);
663+ previous_percent = FALSE;
664+ continue;
665+ }
666+
667+ switch (execsplit[i][0]) {
668+ case '\0': {
669+ const gchar * percent = "%";
670+ g_array_append_val(outarray, percent); /* %% is the literal */
671+ previous_percent = TRUE;
672+ break;
673+ }
674+ case 'd':
675+ case 'D':
676+ case 'n':
677+ case 'N':
678+ case 'v':
679+ case 'm':
680+ /* Deprecated */
681+ g_array_append_val(outarray, skipchar);
682+ break;
683+ case 'f':
684+ ensure_singlefile(&single_file, &single_uri, uri_list);
685+
686+ if (single_file != NULL) {
687+ g_array_append_val(outarray, single_file);
688+ }
689+
690+ g_array_append_val(outarray, skipchar);
691+ break;
692+ case 'F':
693+ if (uri_list != NULL) {
694+ if (file_list == NULL) {
695+ file_list = build_file_list(uri_list);
696+ }
697+ g_array_append_val(outarray, file_list);
698+ }
699+
700+ g_array_append_val(outarray, skipchar);
701+ break;
702+ case 'i':
703+ case 'c':
704+ case 'k':
705+ /* Perhaps? Not sure anyone uses these */
706+ g_array_append_val(outarray, skipchar);
707+ break;
708+ case 'U':
709+ if (uri_list != NULL) {
710+ g_array_append_val(outarray, uri_list);
711+ }
712+ g_array_append_val(outarray, skipchar);
713+ break;
714+ case 'u':
715+ ensure_singleuri(&single_uri, uri_list);
716+
717+ if (single_uri != NULL) {
718+ g_array_append_val(outarray, single_uri);
719+ }
720+
721+ g_array_append_val(outarray, skipchar);
722+ break;
723+ default:
724+ g_warning("Desktop Exec line code '%%%c' unknown, skipping.", execsplit[i][0]);
725+ g_array_append_val(outarray, skipchar);
726+ break;
727+ }
728+ }
729+
730+ gchar * output = g_strjoinv(" ", (gchar **)outarray->data);
731+ g_array_free(outarray, TRUE);
732+
733+ g_free(single_uri);
734+ g_free(single_file);
735+ g_free(file_list);
736+ g_strfreev(execsplit);
737+
738+ return output;
739+}
740
741=== modified file 'helpers.h'
742--- helpers.h 2013-08-11 19:55:59 +0000
743+++ helpers.h 2013-08-11 19:55:59 +0000
744@@ -27,4 +27,8 @@
745 const gchar * app_id);
746 gchar * desktop_to_exec (GKeyFile * desktop_file,
747 const gchar * from);
748+void set_upstart_variable (const gchar * variable,
749+ const gchar * value);
750+gchar * desktop_exec_parse (const gchar * execline,
751+ const gchar * uri_list);
752

Subscribers

People subscribed via source and target branches