From 4b14048d6037a2da5601952a2dd36fe67d04218b Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 30 Oct 2023 11:22:32 +0300 Subject: [PATCH] improve and fix x install 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 --- src/bootstrap/src/core/build_steps/install.rs | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/install.rs b/src/bootstrap/src/core/build_steps/install.rs index 391995b7c3b84..ed6b1c5154c8c 100644 --- a/src/bootstrap/src/core/build_steps/install.rs +++ b/src/bootstrap/src/core/build_steps/install.rs @@ -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")); @@ -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)); @@ -110,19 +121,16 @@ fn default_path(config: &Option, 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, 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 {