search

boxen / puppet-nodejs

MIT License
15 stars 47 forks source link

Fix for file ownership issues #67

Closed hugobast closed 7 years ago

hugobast commented 7 years ago

user is an option that gets ignored by puppet. uid is meant to be used in order to set a user to execute the command. I cannot find a point at which it ever was user in recent, not so recent and far distant versions of puppet which leads me to believe this change was introduced by accident.

The effect of using user vs uid are shown below:

▶ ls -l `npm config get prefix`/lib/node_modules
total 0
drwxr-xr-x  12 hugobastien  staff  408 14 Nov 22:37 gulp-cli
drwxr-xr-x  25 hugobastien  staff  850 11 Nov 13:18 npm
drwxr-xr-x   6 nobody       staff  204 14 Nov 22:09 react-native-cli

gulp-cli was installed with the fix that I'm submitting here while react-native-cli was installed current version of puppet-nodejs

jacobbednarz commented 7 years ago

This looks like it was specifically changed in #52 to address some similar issues to what you are facing.

@hugobast would you mind linking to the docs where you have seen this in puppet-land? I also think this is uid (and guid where needed) however I cannot find the docs on it.

cc @hirocaster and @blackjid

blackjid commented 7 years ago

Looks like you are right, and it should be uid. I mistakenly merge that PR. The user param is for the exec method, not for the execution library which is being used.

https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/execution.rb#L119

Maybe @hirocaster had permission issues with we published the PR.

hirocaster commented 7 years ago

OK, I will retry it. Sorry, this PR #52 .

jacobbednarz commented 7 years ago

Thanks everyone! I appreciate it.

blackjid commented 7 years ago

@jacobbednarz Do you think this should be released?? thanks

jacobbednarz commented 7 years ago

great idea! i've cut 5.0.9 to get this fix available.

jacobbednarz commented 7 years ago

got this boxen/our-boxen upstream too via boxen/our-boxen#836