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

Proposed by Ted Gould
Status: Merged
Approved by: Charles Kerr
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) Approve
Jamie Strandboge Approve
PS Jenkins bot (community) continuous-integration Approve
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.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
147. By Ted Gould

Gotta change the .in file too

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

Detect empty strings in environment variables

149. By Ted Gould

Test to ensure we handle null strings

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Jamie Strandboge (jdstrand) :
review: Approve
Revision history for this message
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
=== modified file 'exec-line-exec.c'
--- exec-line-exec.c 2014-02-07 14:56:27 +0000
+++ exec-line-exec.c 2014-05-01 15:05:19 +0000
@@ -62,10 +62,20 @@
62 /* Protect against app directories that have ':' in them */62 /* Protect against app directories that have ':' in them */
63 if (appdir != NULL && strchr(appdir, ':') == NULL) {63 if (appdir != NULL && strchr(appdir, ':') == NULL) {
64 const gchar * path_path = g_getenv("PATH");64 const gchar * path_path = g_getenv("PATH");
65 if (path_path != NULL && path_path[0] == '\0')
66 path_path = NULL;
65 gchar * path_libpath = NULL;67 gchar * path_libpath = NULL;
66 const gchar * path_joinable[4] = { 0 };68 const gchar * path_joinable[4] = { 0 };
6769
70 const gchar * lib_path = g_getenv("LD_LIBRARY_PATH");
71 if (lib_path != NULL && lib_path[0] == '\0')
72 lib_path = NULL;
73 gchar * lib_libpath = g_build_filename(appdir, "lib", NULL);
74 const gchar * lib_joinable[4] = { 0 };
75
68 const gchar * import_path = g_getenv("QML2_IMPORT_PATH");76 const gchar * import_path = g_getenv("QML2_IMPORT_PATH");
77 if (import_path != NULL && import_path[0] == '\0')
78 import_path = NULL;
69 gchar * import_libpath = NULL;79 gchar * import_libpath = NULL;
70 const gchar * import_joinable[4] = { 0 };80 const gchar * import_joinable[4] = { 0 };
7181
@@ -80,6 +90,10 @@
80 path_joinable[1] = appdir;90 path_joinable[1] = appdir;
81 path_joinable[2] = path_path;91 path_joinable[2] = path_path;
8292
93 lib_joinable[0] = import_libpath;
94 lib_joinable[1] = lib_libpath;
95 lib_joinable[2] = lib_path;
96
83 /* Need to check whether the original is NULL because we're97 /* Need to check whether the original is NULL because we're
84 appending instead of prepending */98 appending instead of prepending */
85 if (import_path == NULL) {99 if (import_path == NULL) {
@@ -92,6 +106,9 @@
92 path_joinable[0] = appdir;106 path_joinable[0] = appdir;
93 path_joinable[1] = path_path;107 path_joinable[1] = path_path;
94108
109 lib_joinable[0] = lib_libpath;
110 lib_joinable[1] = lib_path;
111
95 import_joinable[0] = import_path;112 import_joinable[0] = import_path;
96 }113 }
97114
@@ -100,6 +117,11 @@
100 g_free(path_libpath);117 g_free(path_libpath);
101 g_free(newpath);118 g_free(newpath);
102119
120 gchar * newlib = g_strjoinv(":", (gchar**)lib_joinable);
121 g_setenv("LD_LIBRARY_PATH", newlib, TRUE);
122 g_free(lib_libpath);
123 g_free(newlib);
124
103 if (import_joinable[0] != NULL) {125 if (import_joinable[0] != NULL) {
104 gchar * newimport = g_strjoinv(":", (gchar**)import_joinable);126 gchar * newimport = g_strjoinv(":", (gchar**)import_joinable);
105 g_setenv("QML2_IMPORT_PATH", newimport, TRUE);127 g_setenv("QML2_IMPORT_PATH", newimport, TRUE);
106128
=== modified file 'tests/exec-test-archcolon.sh'
--- tests/exec-test-archcolon.sh 2013-12-20 17:53:34 +0000
+++ tests/exec-test-archcolon.sh 2014-05-01 15:05:19 +0000
@@ -5,6 +5,11 @@
5 exit 15 exit 1
6fi6fi
77
8if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib:/lib" ] ; then
9 echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
10 exit 1
11fi
12
8if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then13if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then
9 echo "Bad QML import path: $QML2_IMPORT_PATH"14 echo "Bad QML import path: $QML2_IMPORT_PATH"
10 exit 115 exit 1
1116
=== modified file 'tests/exec-test-colon.sh'
--- tests/exec-test-colon.sh 2013-12-18 18:04:38 +0000
+++ tests/exec-test-colon.sh 2014-05-01 15:05:19 +0000
@@ -5,6 +5,11 @@
5 exit 15 exit 1
6fi6fi
77
8if [ "$LD_LIBRARY_PATH" != "/lib" ] ; then
9 echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
10 exit 1
11fi
12
8if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then13if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then
9 echo "Bad QML import path: $QML2_IMPORT_PATH"14 echo "Bad QML import path: $QML2_IMPORT_PATH"
10 exit 115 exit 1
1116
=== modified file 'tests/exec-test-full.sh'
--- tests/exec-test-full.sh 2013-12-10 20:59:52 +0000
+++ tests/exec-test-full.sh 2014-05-01 15:05:19 +0000
@@ -5,6 +5,11 @@
5 exit 15 exit 1
6fi6fi
77
8if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib/64bit-amazing:$APP_DIR/lib:/lib" ] ; then
9 echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
10 exit 1
11fi
12
8if [ "$QML2_IMPORT_PATH" != "/bar/qml/import:$APP_DIR/lib/64bit-amazing" ] ; then13if [ "$QML2_IMPORT_PATH" != "/bar/qml/import:$APP_DIR/lib/64bit-amazing" ] ; then
9 echo "Bad QML import path: $QML2_IMPORT_PATH"14 echo "Bad QML import path: $QML2_IMPORT_PATH"
10 exit 115 exit 1
1116
=== modified file 'tests/exec-test-noarch.sh'
--- tests/exec-test-noarch.sh 2013-12-10 21:05:00 +0000
+++ tests/exec-test-noarch.sh 2014-05-01 15:05:19 +0000
@@ -5,6 +5,11 @@
5 exit 15 exit 1
6fi6fi
77
8if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib:/lib" ] ; then
9 echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
10 exit 1
11fi
12
8if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then13if [ "$QML2_IMPORT_PATH" != "/bar/qml/import" ] ; then
9 echo "Bad QML import path: $QML2_IMPORT_PATH"14 echo "Bad QML import path: $QML2_IMPORT_PATH"
10 exit 115 exit 1
1116
=== modified file 'tests/exec-test-noinit.sh'
--- tests/exec-test-noinit.sh 2013-12-10 21:13:28 +0000
+++ tests/exec-test-noinit.sh 2014-05-01 15:05:19 +0000
@@ -5,6 +5,11 @@
5 exit 15 exit 1
6fi6fi
77
8if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib/64bit-amazing:$APP_DIR/lib" ] ; then
9 echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
10 exit 1
11fi
12
8if [ "$QML2_IMPORT_PATH" != "$APP_DIR/lib/64bit-amazing" ] ; then13if [ "$QML2_IMPORT_PATH" != "$APP_DIR/lib/64bit-amazing" ] ; then
9 echo "Bad QML import path: $QML2_IMPORT_PATH"14 echo "Bad QML import path: $QML2_IMPORT_PATH"
10 exit 115 exit 1
1116
=== added file 'tests/exec-test-nullstr.sh'
--- tests/exec-test-nullstr.sh 1970-01-01 00:00:00 +0000
+++ tests/exec-test-nullstr.sh 2014-05-01 15:05:19 +0000
@@ -0,0 +1,18 @@
1#!/bin/bash -e
2
3if [ "$PATH" != "$APP_DIR/lib/64bit-amazing/bin:$APP_DIR" ] ; then
4 echo "Bad PATH: $PATH"
5 exit 1
6fi
7
8if [ "$LD_LIBRARY_PATH" != "$APP_DIR/lib/64bit-amazing:$APP_DIR/lib" ] ; then
9 echo "Bad LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
10 exit 1
11fi
12
13if [ "$QML2_IMPORT_PATH" != "$APP_DIR/lib/64bit-amazing" ] ; then
14 echo "Bad QML import path: $QML2_IMPORT_PATH"
15 exit 1
16fi
17
18exit 0
019
=== modified file 'tests/exec-test.sh.in'
--- tests/exec-test.sh.in 2013-12-20 17:53:34 +0000
+++ tests/exec-test.sh.in 2014-05-01 15:05:19 +0000
@@ -1,6 +1,7 @@
1#!/bin/bash -e1#!/bin/bash -e
22
3export PATH=/path3export PATH=/path
4export LD_LIBRARY_PATH=/lib
4export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-full.sh5export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-full.sh
5export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@6export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
6export QML2_IMPORT_PATH=/bar/qml/import7export QML2_IMPORT_PATH=/bar/qml/import
@@ -10,6 +11,7 @@
10@CMAKE_BINARY_DIR@/exec-line-exec11@CMAKE_BINARY_DIR@/exec-line-exec
1112
12export PATH=/path13export PATH=/path
14export LD_LIBRARY_PATH=/lib
13export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-noarch.sh15export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-noarch.sh
14export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@16export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
15export QML2_IMPORT_PATH=/bar/qml/import17export QML2_IMPORT_PATH=/bar/qml/import
@@ -19,6 +21,7 @@
19@CMAKE_BINARY_DIR@/exec-line-exec21@CMAKE_BINARY_DIR@/exec-line-exec
2022
21unset PATH23unset PATH
24unset LD_LIBRARY_PATH
22export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-noinit.sh25export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-noinit.sh
23export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@26export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
24unset QML2_IMPORT_PATH27unset QML2_IMPORT_PATH
@@ -28,6 +31,7 @@
28@CMAKE_BINARY_DIR@/exec-line-exec31@CMAKE_BINARY_DIR@/exec-line-exec
2932
30export PATH=/path33export PATH=/path
34export LD_LIBRARY_PATH=/lib
31export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-colon.sh35export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-colon.sh
32export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@/foo:bar36export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@/foo:bar
33export QML2_IMPORT_PATH=/bar/qml/import37export QML2_IMPORT_PATH=/bar/qml/import
@@ -37,6 +41,7 @@
37@CMAKE_BINARY_DIR@/exec-line-exec41@CMAKE_BINARY_DIR@/exec-line-exec
3842
39export PATH=/path43export PATH=/path
44export LD_LIBRARY_PATH=/lib
40export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-archcolon.sh45export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-archcolon.sh
41export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@46export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
42export QML2_IMPORT_PATH=/bar/qml/import47export QML2_IMPORT_PATH=/bar/qml/import
@@ -45,3 +50,13 @@
45echo "Testing Arch Colon Test"50echo "Testing Arch Colon Test"
46@CMAKE_BINARY_DIR@/exec-line-exec51@CMAKE_BINARY_DIR@/exec-line-exec
4752
53export PATH=""
54export LD_LIBRARY_PATH=""
55export APP_EXEC=@CMAKE_CURRENT_SOURCE_DIR@/exec-test-nullstr.sh
56export APP_DIR=@CMAKE_CURRENT_BINARY_DIR@
57export QML2_IMPORT_PATH=""
58export UPSTART_APP_LAUNCH_ARCH=64bit-amazing
59
60echo "Testing Null string Test"
61@CMAKE_BINARY_DIR@/exec-line-exec
62

Subscribers

People subscribed via source and target branches