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

FreeBSD init: Remove unnecessary daemon -u option #1924

Merged
merged 1 commit into from
Oct 31, 2017
Merged

FreeBSD init: Remove unnecessary daemon -u option #1924

merged 1 commit into from
Oct 31, 2017

Conversation

snsinfu
Copy link

@snsinfu snsinfu commented Oct 16, 2017

1. What does this change do, exactly?

Fixes #1923 by removing unnecessary -u ${caddy_user} option for the daemon command in the init script for FreeBSD.

2. Please link to the relevant issues.

#1923

3. Which documentation changes (if any) need to be made because of this PR?

None

4. Checklist

  • I have written tests and verified that they fail without my change
  • I have squashed any insignificant commits
  • This change has comments for package types, values, functions, and non-obvious lines of code
  • I am willing to help maintain this change if there are issues with it later

The rc.subr framework already takes care of substituting user. So, using
daemon's -u option is double user-substitution and fails if $caddy_user
is non-root.
@CLAassistant
Copy link

CLAassistant commented Oct 16, 2017

CLA assistant check
All committers have signed the CLA.

@@ -62,7 +62,7 @@ fi
pidfile="/var/run/${name}.pid"
procname="${caddy_bin_path}" #enabled builtin pid checking for start / stop
command="/usr/sbin/daemon"
command_args="-u ${caddy_user} -p ${pidfile} /usr/bin/env ${caddy_env} ${procname} -cpu ${caddy_cpu} -log stdout -conf ${caddy_config_path} -agree -email ${caddy_cert_email} < /dev/null >> ${caddy_logfile} 2>&1"
command_args="-p ${pidfile} /usr/bin/env ${caddy_env} ${procname} -cpu ${caddy_cpu} -log stdout -conf ${caddy_config_path} -agree -email ${caddy_cert_email} < /dev/null >> ${caddy_logfile} 2>&1"
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to change the caddy_user value? Or remove the unused variable then?

Copy link
Author

Choose a reason for hiding this comment

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

No. The ${name}_user variable (${name} is caddy here) is special and tells the init system to run the command as the specified user. A documentation can be found in the ${name}_user part of man rc.subr. Sorry for being unclear!

@mholt mholt merged commit 34a34c5 into caddyserver:master Oct 31, 2017
@mholt
Copy link
Member

mholt commented Oct 31, 2017

Thank you!

@mholt mholt added this to the 0.10.11 milestone Nov 9, 2017
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