-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Respect prefix
when sysconfdir
isn't set in config.toml
#113733
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