Skip to content

Commit

Permalink
improve and fix x install
Browse files Browse the repository at this point in the history
Fix: Write access check of `prefix` and `sysconfdir`
when DESTDIR is present.

Improvement: Instead of repeatedly reading `DESTDIR` within
each `fn prepare_dir` usage, read it once and pass it to
the `fn prepare_dir`.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
  • Loading branch information
onur-ozkan committed Oct 30, 2023
1 parent 7314873 commit 4b14048
Showing 1 changed file with 30 additions and 22 deletions.
52 changes: 30 additions & 22 deletions src/bootstrap/src/core/build_steps/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,27 @@ fn install_sh(

let prefix = default_path(&builder.config.prefix, "/usr/local");
let sysconfdir = prefix.join(default_path(&builder.config.sysconfdir, "/etc"));
let destdir_env = env::var_os("DESTDIR").map(PathBuf::from);

// Sanity check for the user write access on prefix and sysconfdir
assert!(
is_dir_writable_for_user(&prefix),
"User doesn't have write access on `install.prefix` path in the `config.toml`.",
);
assert!(
is_dir_writable_for_user(&sysconfdir),
"User doesn't have write access on `install.sysconfdir` path in `config.toml`."
);
// Sanity checks on the write access of user.
//
// When the `DESTDIR` environment variable is present, there is no point to
// check write access for `prefix` and `sysconfdir` individually, as they
// are combined with the path from the `DESTDIR` environment variable. In
// this case, we only need to check the `DESTDIR` path, disregarding the
// `prefix` and `sysconfdir` paths.
if let Some(destdir) = &destdir_env {
assert!(is_dir_writable_for_user(destdir), "User doesn't have write access on DESTDIR.");
} else {
assert!(
is_dir_writable_for_user(&prefix),
"User doesn't have write access on `install.prefix` path in the `config.toml`.",
);
assert!(
is_dir_writable_for_user(&sysconfdir),
"User doesn't have write access on `install.sysconfdir` path in `config.toml`."
);
}

let datadir = prefix.join(default_path(&builder.config.datadir, "share"));
let docdir = prefix.join(default_path(&builder.config.docdir, "share/doc/rust"));
Expand All @@ -94,13 +105,13 @@ fn install_sh(
let mut cmd = Command::new(SHELL);
cmd.current_dir(&empty_dir)
.arg(sanitize_sh(&tarball.decompressed_output().join("install.sh")))
.arg(format!("--prefix={}", prepare_dir(prefix)))
.arg(format!("--sysconfdir={}", prepare_dir(sysconfdir)))
.arg(format!("--datadir={}", prepare_dir(datadir)))
.arg(format!("--docdir={}", prepare_dir(docdir)))
.arg(format!("--bindir={}", prepare_dir(bindir)))
.arg(format!("--libdir={}", prepare_dir(libdir)))
.arg(format!("--mandir={}", prepare_dir(mandir)))
.arg(format!("--prefix={}", prepare_dir(&destdir_env, prefix)))
.arg(format!("--sysconfdir={}", prepare_dir(&destdir_env, sysconfdir)))
.arg(format!("--datadir={}", prepare_dir(&destdir_env, datadir)))
.arg(format!("--docdir={}", prepare_dir(&destdir_env, docdir)))
.arg(format!("--bindir={}", prepare_dir(&destdir_env, bindir)))
.arg(format!("--libdir={}", prepare_dir(&destdir_env, libdir)))
.arg(format!("--mandir={}", prepare_dir(&destdir_env, mandir)))
.arg("--disable-ldconfig");
builder.run(&mut cmd);
t!(fs::remove_dir_all(&empty_dir));
Expand All @@ -110,19 +121,16 @@ fn default_path(config: &Option<PathBuf>, default: &str) -> PathBuf {
config.as_ref().cloned().unwrap_or_else(|| PathBuf::from(default))
}

fn prepare_dir(mut path: PathBuf) -> String {
fn prepare_dir(destdir_env: &Option<PathBuf>, mut path: PathBuf) -> String {
// The DESTDIR environment variable is a standard way to install software in a subdirectory
// while keeping the original directory structure, even if the prefix or other directories
// contain absolute paths.
//
// More information on the environment variable is available here:
// https://www.gnu.org/prep/standards/html_node/DESTDIR.html
if let Some(destdir) = env::var_os("DESTDIR").map(PathBuf::from) {
// Sanity check for the user write access on DESTDIR
assert!(is_dir_writable_for_user(&destdir), "User doesn't have write access on DESTDIR.");

if let Some(destdir) = destdir_env {
let without_destdir = path.clone();
path = destdir;
path = destdir.clone();
// Custom .join() which ignores disk roots.
for part in without_destdir.components() {
if let Component::Normal(s) = part {
Expand Down

0 comments on commit 4b14048

Please sign in to comment.