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

fix home function to actually query OS for users home directory #95

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

retrogradeorbit
Copy link

This resolves issue #12. I haven't tested it on windows, but on windows it will return nil, which is not ideal, but it never actually worked properly anyway, so it might be better.

$ lein midje
All checks (108) succeeded.
me.raynes.fs> (home "crispin")
#object[java.io.File 0x15eda628 "/home/crispin"]
me.raynes.fs> (home "www-data")
#object[java.io.File 0x620327b6 "/var/www"]
me.raynes.fs> (home "root")
#object[java.io.File 0x7358d81 "/root"]
me.raynes.fs> (home "missing-user")
nil
me.raynes.fs> (home "root;echo wat")
nil

@Raynes
Copy link
Owner

Raynes commented Jan 14, 2016

Any particular reason this would be better?

@retrogradeorbit
Copy link
Author

It's better because the existing implementation returns the wrong directory for any non standard user, or any system with users laid out in multiple directory hierarchies. This patch returns the correct home directory for those users. For instance, running as a standard user (who has their home directory under /home/user) ask for the home directory of www-data; say (home "www-data") and the existing implementation returns "/home/www-data" which on my system is completely wrong. www-data's home directory is "/var/www". ask for home dir of "root" and you get "/home/root" not "/root". This patch returns the correct home directory for every user by querying the operating system.

Your comment here aludes to this problem:

https://github.com/Raynes/fs/blob/master/src/me/raynes/fs.clj#L27

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.

2 participants