-
-
Notifications
You must be signed in to change notification settings - Fork 881
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
Add FreeBSD Support #376
Add FreeBSD Support #376
Conversation
This adds the necessary changes the params class and associated variables to support FreeBSD.
/(?i-mx:sunos)/ => '/var/log/nginx', | ||
/(?i-mx:linux)/ => '/var/log/nginx', | ||
/(?i-mx:sunos)/ => '/var/log/nginx', | ||
/(?i-mx:freebsd)/ => '/var/log/nginx', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is starting to feel silly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove the selector - it is silly.
The super group on many platforms is known as root, which a UID of 0. However, on other platforms, the UID is still 0, but the group is 'wheel'. Largely historic UNIX jargon, but suffice to say that, to support FreeBSD and others, setting the group of '0' has the same impact while supporting a wider range of platforms.
Regarding changing "root" to "0" for the groups, what do you think of adding a new OS dependent param which is set to "wheel" on FreeBSD and "root" by default? Similar to puppetlabs-apache: https://github.com/puppetlabs/puppetlabs-apache/blob/1.1.1/manifests/params.pp#L152 I know this means having to manually add this for other BSD's (if there is demand for them) but it's clearer which group will be used this way. |
If $root_group makes you happier, I can update the PR. Not sure what to do about the tests for this though. My attention is waning. |
Alright, I've updated the code, but not the tests. I need to give a harder look and see how we test for platform specifics. |
This replaces the code that used to exist and now accommodates FreeBSD using the $kernel fact, but still allows Joyent selection from $kernelversion.
Hows this? |
Looks good to me! Now up to @jfryman to merge. Thanks for making those changes. |
👍 @xaque208 We're going to clean this up. Promise. :) |
@jfryman Excellent, speak up if we can help. We're moving to adopt this as our nginx module. |
This work gets the bare minimum code working on FreeBSD. Catalogs compile,