Merge lp:~ted/ubuntu-app-launch/ld-library-path into lp:ubuntu-app-launch/14.04

Proposed by Ted Gould on 2014-04-30
Status: Merged
Approved by: Charles Kerr on 2014-05-21
Approved revision: 149
Merged at revision: 148
Proposed branch: lp:~ted/ubuntu-app-launch/ld-library-path
Merge into: lp:ubuntu-app-launch/14.04
Diff against target: 218 lines (+80/-0)
8 files modified
exec-line-exec.c (+22/-0)
tests/exec-test-archcolon.sh (+5/-0)
tests/exec-test-colon.sh (+5/-0)
tests/exec-test-full.sh (+5/-0)
tests/exec-test-noarch.sh (+5/-0)
tests/exec-test-noinit.sh (+5/-0)
tests/exec-test-nullstr.sh (+18/-0)
tests/exec-test.sh.in (+15/-0)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/ld-library-path
Reviewer Review Type Date Requested Status
Charles Kerr (community) 2014-04-30 Approve on 2014-05-21
Jamie Strandboge 2014-04-30 Approve on 2014-05-01
PS Jenkins bot (community) continuous-integration Approve on 2014-05-01
Review via email: mp+217832@code.launchpad.net

Commit message

Set LD_LIBRARY_PATH to include the application directory

Description of the change

Setting the LD_LIBARAY_PATH to include the application directory including the architecture specific one if that's appropriate.

To post a comment you must log in.
147. By Ted Gould on 2014-05-01

Gotta change the .in file too

Jamie Strandboge (jdstrand) wrote :

I tested this with my package in http://bazaar.launchpad.net/~jdstrand/+junk/test-click-env/files (on desktop):

$ sudo click install --user=$USER com.ubuntu.developer.jdstrand.click-env_0.1_all.click

Test with path unset:
$ initctl unset-env -g LD_LIBRARY_PATH
$ initctl list-env|grep LD_LIBRARY_PATH
$ start application APP_ID=com.ubuntu.developer.jdstrand.click-env_click-env_0.1 && sleep 5 && grep LD_LIBRARY_PATH ~/.cache/upstart/application-click-com.ubuntu.developer.jdstrand.click-env_click-env_0.1.log | tail -1
application stop/waiting
LD_LIBRARY_PATH=/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib/x86_64-linux-gnu:/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib
(good)

Test with path set:
$ initctl set-env -g LD_LIBRARY_PATH=/foo:/
$ initctl list-env|grep LD_LIBRARY_PATH
LD_LIBRARY_PATH=/foo:/
$ start application APP_ID=com.ubuntu.developer.jdstrand.click-env_click-env_0.1 && sleep 5 && grep LD_LIBRARY_PATH ~/.cache/upstart/application-click-com.ubuntu.developer.jdstrand.click-env_click-env_0.1.log | tail -1
application stop/waiting
LD_LIBRARY_PATH=/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib/x86_64-linux-gnu:/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib:/foo:/
(good)

Test with path set, but empty:
$ initctl set-env -g LD_LIBRARY_PATH=
$ initctl list-env|grep LD_LIBRARY_PATHLD_LIBRARY_PATH=
$ start application APP_ID=com.ubuntu.developer.jdstrand.click-env_click-env_0.1 && sleep 5 && grep LD_LIBRARY_PATH ~/.cache/upstart/application-click-com.ubuntu.developer.jdstrand.click-env_click-env_0.1.log | tail -1
application stop/waiting
LD_LIBRARY_PATH=/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib/x86_64-linux-gnu:/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib:

When LD_LIBRARY_PATH is set but empty, upstart-app-launch leaves a trailing ':'. I think we should treat empty the same as unset.

Note, the same thing affects QML2_IMPORT_PATH and PATH:
$ initctl set-env -g QML2_IMPORT_PATH=
$ start application APP_ID=com.ubuntu.developer.jdstrand.click-env_click-env_0.1 && sleep 5 && grep QML2_IMPORT_PATH ~/.cache/upstart/application-click-com.ubuntu.developer.jdstrand.click-env_click-env_0.1.log | tail -1
application stop/waiting
QML2_IMPORT_PATH=:/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib/x86_64-linux-gnu
(notice ':' is prepended)

$ initctl set-env -g PATH=
$ start application APP_ID=com.ubuntu.developer.jdstrand.click-env_click-env_0.1 && sleep 5 && grep '^PATH=' ~/.cache/upstart/application-click-com.ubuntu.developer.jdstrand.click-env_click-env_0.1.log | tail -1
application stop/waiting
PATH=/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env/lib/x86_64-linux-gnu/bin:/opt/click.ubuntu.com/.click/users/jamie/com.ubuntu.developer.jdstrand.click-env:
(notice ':' is appended)

review: Needs Fixing
Ted Gould (ted) wrote :

On Thu, 2014-05-01 at 13:54 +0000, Jamie Strandboge wrote:

> When LD_LIBRARY_PATH is set but empty, upstart-app-launch leaves a trailing ':'. I think we should treat empty the same as unset.
>
> Note, the same thing affects QML2_IMPORT_PATH and PATH:

Makes sense. Fixed in r148. Test in r149.

148. By Ted Gould on 2014-05-01

Detect empty strings in environment variables

149. By Ted Gould on 2014-05-01

Test to ensure we handle null strings

review: Approve
Charles Kerr (charlesk) wrote :

Approving by proxy for Jamie since he's not in the indicator-applet-devel group.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'exec-line-exec.c'
2--- exec-line-exec.c 2014-02-07 14:56:27 +0000
3+++ exec-line-exec.c 2014-05-01 15:05:19 +0000
4@@ -62,10 +62,20 @@
5 /* Protect against app directories that have ':' in them */
6 if (appdir != NULL && strchr(appdir, ':') == NULL) {
7 const gchar * path_path = g_getenv("PATH");
8+ if (path_path != NULL && path_path[0] == '\0')
9+ path_path = NULL;
10 gchar * path_libpath = NULL;
11 const gchar * path_joinable[4] = { 0 };
12
13+ const gchar * lib_path = g_getenv("LD_LIBRARY_PATH");
14+ if (lib_path != NULL && lib_path[0] == '\0')
15+ lib_path = NULL;
16+ gchar * lib_libpath = g_build_filename(appdir, "lib", NULL);
17+ const gchar * lib_joinable[4] = { 0 };
18+
19 const gchar * import_path = g_getenv("QML2_IMPORT_PATH");
20+ if (import_path != NULL && import_path[0] == '\0')
21+ import_path = NULL;
22 gchar * import_libpath = NULL;
23 const gchar * import_joinable[4] = { 0 };
24
25@@ -80,6 +90,10 @@
26 path_joinable[1] = appdir;
27 path_joinable[2] = path_path;
28
29+ lib_joinable[0] = import_libpath;
30+ lib_joinable[1] = lib_libpath;
31+ lib_joinable[2] = lib_path;
32+
33 /* Need to check whether the original is NULL because we're
34 appending instead of prepending */
35 if (import_path == NULL) {
36@@ -92,6 +106,9 @@
37 path_joinable[0] = appdir;
38 path_joinable[1] = path_path;
39
40+ lib_joinable[0] = lib_libpath;
41+ lib_joinable[1] = lib_path;
42+
43 import_joinable[0] = import_path;
44 }
45
46@@ -100,6 +117,11 @@
47 g_free(path_libpath);
48 g_free(newpath);
49
50+ gchar * newlib = g_strjoinv(":", (gchar**)lib_joinable);
51+ g_setenv("LD_LIBRARY_PATH", newlib, TRUE);
52+ g_free(lib_libpath);
53+ g_free(newlib);
54+
55 if (import_joinable[0] != NULL) {
56 gchar * newimport = g_strjoinv(":", (gchar**)import_joinable);
57 g_setenv("QML2_IMPORT_PATH", newimport, TRUE);
58
59=== modified file 'tests/exec-test-archcolon.sh'
60--- tests/exec-test-archcolon.sh 2013-12-20 17:53:34 +0000
61+++ tests/exec-test-archcolon.sh 2014-05-01 15:05:19 +0000
62@@ -5,6 +5,11 @@
63 exit 1
64 fi
65
66+if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib:/lib" ] ; then
67+ echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
68+ exit 1
69+fi
70+
71 if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then
72 echo "Bad QML import path: $QML2_IMPORT_PATH"
73 exit 1
74
75=== modified file 'tests/exec-test-colon.sh'
76--- tests/exec-test-colon.sh 2013-12-18 18:04:38 +0000
77+++ tests/exec-test-colon.sh 2014-05-01 15:05:19 +0000
78@@ -5,6 +5,11 @@
79 exit 1
80 fi
81
82+if [ "$LD_LIBRARY_PATH" != "/lib" ] ; then
83+ echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
84+ exit 1
85+fi
86+
87 if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then
88 echo "Bad QML import path: $QML2_IMPORT_PATH"
89 exit 1
90
91=== modified file 'tests/exec-test-full.sh'
92--- tests/exec-test-full.sh 2013-12-10 20:59:52 +0000
93+++ tests/exec-test-full.sh 2014-05-01 15:05:19 +0000
94@@ -5,6 +5,11 @@
95 exit 1
96 fi
97
98+if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib/64bit-amazing:$APP_DIR/lib:/lib" ] ; then
99+ echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
100+ exit 1
101+fi
102+
103 if [ "$QML2_IMPORT_PATH" != "/bar/qml/import:$APP_DIR/lib/64bit-amazing" ] ; then
104 echo "Bad QML import path: $QML2_IMPORT_PATH"
105 exit 1
106
107=== modified file 'tests/exec-test-noarch.sh'
108--- tests/exec-test-noarch.sh 2013-12-10 21:05:00 +0000
109+++ tests/exec-test-noarch.sh 2014-05-01 15:05:19 +0000
110@@ -5,6 +5,11 @@
111 exit 1
112 fi
113
114+if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib:/lib" ] ; then
115+ echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
116+ exit 1
117+fi
118+
119 if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then
120 echo "Bad QML import path: $QML2_IMPORT_PATH"
121 exit 1
122
123=== modified file 'tests/exec-test-noinit.sh'
124--- tests/exec-test-noinit.sh 2013-12-10 21:13:28 +0000
125+++ tests/exec-test-noinit.sh 2014-05-01 15:05:19 +0000
126@@ -5,6 +5,11 @@
127 exit 1
128 fi
129
130+if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib/64bit-amazing:$APP_DIR/lib" ] ; then
131+ echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
132+ exit 1
133+fi
134+
135 if [ "$QML2_IMPORT_PATH" != "$APP_DIR/lib/64bit-amazing" ] ; then
136 echo "Bad QML import path: $QML2_IMPORT_PATH"
137 exit 1
138
139=== added file 'tests/exec-test-nullstr.sh'
140--- tests/exec-test-nullstr.sh 1970-01-01 00:00:00 +0000
141+++ tests/exec-test-nullstr.sh 2014-05-01 15:05:19 +0000
142@@ -0,0 +1,18 @@
143+#!/bin/bash -e
144+
145+if [ "$PATH" != "$APP_DIR/lib/64bit-amazing/bin:$APP_DIR" ] ; then
146+ echo "Bad PATH: $PATH"
147+ exit 1
148+fi
149+
150+if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib/64bit-amazing:$APP_DIR/lib" ] ; then
151+ echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
152+ exit 1
153+fi
154+
155+if [ "$QML2_IMPORT_PATH" != "$APP_DIR/lib/64bit-amazing" ] ; then
156+ echo "Bad QML import path: $QML2_IMPORT_PATH"
157+ exit 1
158+fi
159+
160+exit 0
161
162=== modified file 'tests/exec-test.sh.in'
163--- tests/exec-test.sh.in 2013-12-20 17:53:34 +0000
164+++ tests/exec-test.sh.in 2014-05-01 15:05:19 +0000
165@@ -1,6 +1,7 @@
166 #!/bin/bash -e
167
168 export PATH=/path
169+export LD_LIBRARY_PATH=/lib
170 export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-full.sh
171 export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
172 export QML2_IMPORT_PATH=/bar/qml/import
173@@ -10,6 +11,7 @@
174 @CMAKE_BINARY_DIR@/exec-line-exec
175
176 export PATH=/path
177+export LD_LIBRARY_PATH=/lib
178 export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-noarch.sh
179 export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
180 export QML2_IMPORT_PATH=/bar/qml/import
181@@ -19,6 +21,7 @@
182 @CMAKE_BINARY_DIR@/exec-line-exec
183
184 unset PATH
185+unset LD_LIBRARY_PATH
186 export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-noinit.sh
187 export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
188 unset QML2_IMPORT_PATH
189@@ -28,6 +31,7 @@
190 @CMAKE_BINARY_DIR@/exec-line-exec
191
192 export PATH=/path
193+export LD_LIBRARY_PATH=/lib
194 export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-colon.sh
195 export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@/foo:bar
196 export QML2_IMPORT_PATH=/bar/qml/import
197@@ -37,6 +41,7 @@
198 @CMAKE_BINARY_DIR@/exec-line-exec
199
200 export PATH=/path
201+export LD_LIBRARY_PATH=/lib
202 export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-archcolon.sh
203 export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
204 export QML2_IMPORT_PATH=/bar/qml/import
205@@ -45,3 +50,13 @@
206 echo "Testing Arch Colon Test"
207 @CMAKE_BINARY_DIR@/exec-line-exec
208
209+export PATH=""
210+export LD_LIBRARY_PATH=""
211+export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-nullstr.sh
212+export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
213+export QML2_IMPORT_PATH=""
214+export UPSTART_APP_LAUNCH_ARCH=64bit-amazing
215+
216+echo "Testing Null string Test"
217+@CMAKE_BINARY_DIR@/exec-line-exec
218+

Subscribers

People subscribed via source and target branches