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

Please revert Pull Request #46 #150

Closed
dkerr64 opened this issue Sep 18, 2017 · 1 comment · Fixed by #377
Closed

Please revert Pull Request #46 #150

dkerr64 opened this issue Sep 18, 2017 · 1 comment · Fixed by #377

Comments

@dkerr64
Copy link
Contributor

dkerr64 commented Sep 18, 2017

Pull request #46 makes this change to configure.ac...
-avahi_runtime_dir="${localstatedir}/run"
+avahi_runtime_dir="/run"

Please back out this change. Just because "modern, systemd based systems" now use /run instead of /var/run is no reason to HARDCODE /run. There are other places in the script that use ${localstatedir} that were not updated. The help information was not updated. Modern, systemd based systems symlink /var/run to /run so there was no problems with the way it was.

On the other hand it now creates problems for systems that are NOT using /run and/or were passing in a --localstatedir=xxx as part of their build script... which is now ignored (in that one place you made the change). Now there is no way to pass in a configure parameter to put it back the way it was.

The correct way to do this would have been to have the users that require it to pass in --localstatediir=/. as a configure option. Or, if it is really necessary to change the default for localstatedir then change the default to "${prefix}/." in the configure script (where it is currently set to "${prefix}/var"

Please revert this pull request.
Thanks
David

@smcv
Copy link
Contributor

smcv commented Apr 27, 2018

It looks as though implementing #137 might make everyone happy?

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 a pull request may close this issue.

2 participants