Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Improve getent fallback #15

Merged
merged 1 commit into from
May 23, 2018
Merged

Conversation

knu
Copy link
Contributor

@knu knu commented Aug 2, 2016

If HOME is not defined and getent(1) is not found, dirUnix() currently immediately returns an error without even trying the third method (sh -c 'cd && pwd'). It means HOME is the only source for obtaining the home directory on Darwin where getent(1) is missing.

One possible bug here is that the condition to detect if getent(1) is not found might be reversed, which I addressed in the first commit.

The second commit adds a fallback method specially for Darwin. It is to execute dscl(1) via the shell, so no dependency on cgo will be involved.

@knu
Copy link
Contributor Author

knu commented Aug 29, 2016

Feedback, anyone?

@knu
Copy link
Contributor Author

knu commented Sep 1, 2016

I wrote this patch to get puma-dev to work, but seems puma-dev started to include their own fork ("github.com/puma/puma-dev/homedir") which covers the issues I described above.

@hellais
Copy link

hellais commented May 21, 2018

This would be very useful to have merged upstream. I think I am going to use in the meantime an approach similar to puma-dev

@knu knu force-pushed the improve_getent_fallback branch from 9ae2484 to 83b4fcd Compare May 23, 2018 05:58
@knu
Copy link
Contributor Author

knu commented May 23, 2018

Rebased, just in case there's someone interested.

@mitchellh
Copy link
Owner

Thanks, I'll take a look at this again shortly. I'm flying right now but once I get on the ground and in my hotel I'll take a look. Sorry for letting it linger. :)

@mitchellh
Copy link
Owner

Looks good! I'll merge this and then going to nitpick the style a bit. Thank you!

@mitchellh mitchellh merged commit ddd64a0 into mitchellh:master May 23, 2018
@mitchellh
Copy link
Owner

(Actually it looked really good, I think the diff was just messing with my expectations. Thanks!)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants