-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Respect prefix when sysconfdir isn't set in config.toml
#113733
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
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
| // if prefix isn't specified, we want this to be an absolute path, outside `/usr/local`. but if it is specified, make sysconfdir relative. | ||
| let sysconfdir = if builder.config.prefix.is_some() { | ||
| prefix.join(default_path(&builder.config.sysconfdir, "etc")) | ||
| } else { | ||
| "/etc/".into() | ||
| }; |
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.
i am not quite sure if this is the right fix - maybe it should just be changing /etc to etc so that it's always relative to the prefix? that seems more consistent at least (we'd end up with /usr/local/etc by default). but i don't know if people are depending on the existing behavior.
cc @cuviper, what are your thoughts?
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.
See also #113580, where I was suggesting this should just be a diagnostic.
When the prefix is /usr, you almost surely want /etc, not /usr/etc. But for /usr/local prefix, there is often a /usr/local/etc too, so the "local" installations are completely contained. So, I don't think there's a universally great answer, but changing the default feels a little risky.
For my part in distro builds: prefix, sysconfdir, and all the other installation dirs are explicitly set by the distro-default %configure rpm macro, so I shouldn't be affected by changes here either way.
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 a simpler option is to require users to set prefix? I'm not sure how widespread source installs of Rust are, but my very rough guess is that many of those users either are using a non-default prefix today or may want to (e.g., distros probably don't want /usr/local, users installing themselves may want something in $HOME, etc.).
|
@jyn514 any updates on this? thanks |
|
sorry wasn't aware of that, thanks |
fixes #113728