Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove PATH from default env vars for launchctl #1907

Merged
merged 4 commits into from
Feb 5, 2016

Conversation

patrickxb
Copy link
Contributor

@maxtaco
Copy link
Contributor

maxtaco commented Feb 5, 2016

my suggestion is for the client to ship the service its path and for the path to augment its path with the client's path.

@gabriel
Copy link
Contributor

gabriel commented Feb 5, 2016

Actually this does fix it... I'm not sure why I thought the PATH was invalid

We should double check though... maybe we should add a debug log statement which outputs the path (if there isn't already)

Otherwise ship

@gabriel
Copy link
Contributor

gabriel commented Feb 5, 2016

Actually I'm not sure... I might have set my PATH system wide via launchctl:

launchctl getenv PATH
/Users/gabe/.rbenv/shims:/usr/local/bin:/usr/local/sbin:/bin:/Users/gabe/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/lib/node_modules

@gabriel
Copy link
Contributor

gabriel commented Feb 5, 2016

Hold off for a sec, we need to fully understand this more :)

@patrickxb
Copy link
Contributor Author

Based on our testing, it looks like launchctl doesn't use the user's path. It will use a default one, or whatever was set by launchctl setenv ....

We can go forward with @maxtaco suggestion.

@patrickxb
Copy link
Contributor Author

Ok, @maxtaco and @gabriel PTAL. This adds sending of PATH from client to service.

var clientAdditions []string
NextDir:
for _, dir := range strings.Split(arg.Path, ":") {
for _, x := range pathenv {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know this is a rush job, but i'm dying inside! can you use a map so it's not n^2? Thanks!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How long is your path :) ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know i know, but it's less code to use a map!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@maxtaco
Copy link
Contributor

maxtaco commented Feb 5, 2016

Looks good, had a small question !

So service can augment its path with the client's path
and successfully find things like `gpg` that are outside
launchctl default path of /usr/bin:/bin:/usr/sbin:/sbin
@patrickxb patrickxb force-pushed the patrickxb/CORE-2489 branch from 5981c17 to 4e3779d Compare February 5, 2016 21:09
@patrickxb patrickxb merged commit 4e3779d into master Feb 5, 2016
@patrickxb patrickxb deleted the patrickxb/CORE-2489 branch February 25, 2016 15:38
jzila pushed a commit that referenced this pull request Jan 5, 2019
libhttpserver: Get mime types right for even more filetypes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants