cmd: add /usr/local/* to PATH #2765

Merged
merged 2 commits into from Feb 3, 2017

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Feb 2, 2017

Surprisingly /usr/local plays a role in the core snap. It is there where
the image build system splices additional files that don't come from a
typical deb. This includes the fake 'apt' script but most notably the
special 'xdg-open' executable that is necessary for snapd-xdg-open to
work.

The patch also expands the existing test that checks environment to
check PATH (and SNAP and HOME while I was a it it). One regression test
also needed changes as it was measuring the stability of PATH across
invocations of snap-confine.

Fixes: https://bugs.launchpad.net/snapcraft/+bug/1661023
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

cmd: add /usr/local/* to PATH
Surprisingly /usr/local plays a role in the core snap. It is there where
the image build system splices additional files that don't come from a
typical deb. This includes the fake 'apt' script but most notably the
special 'xdg-open' executable that is necessary for snapd-xdg-open to
work.

The patch also expands the existing test that checks environment to
check PATH (and SNAP and HOME while I was a it it). One regression test
also needed changes as it was measuring the stability of PATH across
invocations of snap-confine.

Fixes: https://bugs.launchpad.net/snapcraft/+bug/1661023
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Contributor

ogra1 commented Feb 2, 2017

looks good, land it !

cmd/snap-confine/snap-confine.c
@@ -143,7 +143,9 @@ int main(int argc, char **argv)
debug
("resetting PATH to values in sync with core snap");
setenv("PATH",
- "/usr/sbin:/usr/bin:/sbin:/bin:/usr/games", 1);
+ "/usr/local/sbin:" "/usr/sbin:" "/usr/local/bin:"
@mvo5

mvo5 Feb 2, 2017

Collaborator

Hm, not sure this is in sync with core:

$ cat /etc/environment 
PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games"

or am I misreading this?

@zyga

zyga Feb 2, 2017

Contributor

Hmm, good idea to cross-check with core. Let's see...

Are those just in different order?

@ogra1

ogra1 Feb 2, 2017

Contributor

does it matter ? as long as the /usr/local variant is always before the /usr one it should be fine i think

@zyga

zyga Feb 3, 2017

Contributor

Thanks mvo, corrected now.

@zyga zyga referenced this pull request Feb 2, 2017

Merged

tests: improve snap-env test #2766

cmd: ensure that PATH is identical to what is in /etc/environment
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga merged commit 2ea745c into snapcore:master Feb 3, 2017

6 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

@zyga zyga deleted the zyga:fix-lp1661023 branch Feb 3, 2017

@kyrofa kyrofa referenced this pull request in nextcloud/client_theming Mar 3, 2017

Merged

snapcraft.yaml: add support for building a snap package #77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment