Skip to content

Commit 11393b6

Browse files
committed
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 <work@onurozkan.dev>
1 parent 7314873 commit 11393b6

File tree

1 file changed

+27
-22
lines changed

1 file changed

+27
-22
lines changed

src/bootstrap/src/core/build_steps/install.rs

+27-22
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,24 @@ fn install_sh(
7171

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

75-
// Sanity check for the user write access on prefix and sysconfdir
76-
assert!(
77-
is_dir_writable_for_user(&prefix),
78-
"User doesn't have write access on `install.prefix` path in the `config.toml`.",
79-
);
80-
assert!(
81-
is_dir_writable_for_user(&sysconfdir),
82-
"User doesn't have write access on `install.sysconfdir` path in `config.toml`."
83-
);
76+
// If `DESTDIR` environment variable is present, there is no point to
77+
// check the write access for `prefix` and `sysconfdir`, as they combined
78+
// with the path from `DESTDIR` environment variable.
79+
if let Some(destdir) = &destdir_env {
80+
assert!(is_dir_writable_for_user(destdir), "User doesn't have write access on DESTDIR.");
81+
} else {
82+
// Sanity check for the user write access on prefix and sysconfdir.
83+
assert!(
84+
is_dir_writable_for_user(&prefix),
85+
"User doesn't have write access on `install.prefix` path in the `config.toml`.",
86+
);
87+
assert!(
88+
is_dir_writable_for_user(&sysconfdir),
89+
"User doesn't have write access on `install.sysconfdir` path in `config.toml`."
90+
);
91+
}
8492

8593
let datadir = prefix.join(default_path(&builder.config.datadir, "share"));
8694
let docdir = prefix.join(default_path(&builder.config.docdir, "share/doc/rust"));
@@ -94,13 +102,13 @@ fn install_sh(
94102
let mut cmd = Command::new(SHELL);
95103
cmd.current_dir(&empty_dir)
96104
.arg(sanitize_sh(&tarball.decompressed_output().join("install.sh")))
97-
.arg(format!("--prefix={}", prepare_dir(prefix)))
98-
.arg(format!("--sysconfdir={}", prepare_dir(sysconfdir)))
99-
.arg(format!("--datadir={}", prepare_dir(datadir)))
100-
.arg(format!("--docdir={}", prepare_dir(docdir)))
101-
.arg(format!("--bindir={}", prepare_dir(bindir)))
102-
.arg(format!("--libdir={}", prepare_dir(libdir)))
103-
.arg(format!("--mandir={}", prepare_dir(mandir)))
105+
.arg(format!("--prefix={}", prepare_dir(&destdir_env, prefix)))
106+
.arg(format!("--sysconfdir={}", prepare_dir(&destdir_env, sysconfdir)))
107+
.arg(format!("--datadir={}", prepare_dir(&destdir_env, datadir)))
108+
.arg(format!("--docdir={}", prepare_dir(&destdir_env, docdir)))
109+
.arg(format!("--bindir={}", prepare_dir(&destdir_env, bindir)))
110+
.arg(format!("--libdir={}", prepare_dir(&destdir_env, libdir)))
111+
.arg(format!("--mandir={}", prepare_dir(&destdir_env, mandir)))
104112
.arg("--disable-ldconfig");
105113
builder.run(&mut cmd);
106114
t!(fs::remove_dir_all(&empty_dir));
@@ -110,19 +118,16 @@ fn default_path(config: &Option<PathBuf>, default: &str) -> PathBuf {
110118
config.as_ref().cloned().unwrap_or_else(|| PathBuf::from(default))
111119
}
112120

113-
fn prepare_dir(mut path: PathBuf) -> String {
121+
fn prepare_dir(destdir_env: &Option<PathBuf>, mut path: PathBuf) -> String {
114122
// The DESTDIR environment variable is a standard way to install software in a subdirectory
115123
// while keeping the original directory structure, even if the prefix or other directories
116124
// contain absolute paths.
117125
//
118126
// More information on the environment variable is available here:
119127
// https://www.gnu.org/prep/standards/html_node/DESTDIR.html
120-
if let Some(destdir) = env::var_os("DESTDIR").map(PathBuf::from) {
121-
// Sanity check for the user write access on DESTDIR
122-
assert!(is_dir_writable_for_user(&destdir), "User doesn't have write access on DESTDIR.");
123-
128+
if let Some(destdir) = destdir_env {
124129
let without_destdir = path.clone();
125-
path = destdir;
130+
path = destdir.clone();
126131
// Custom .join() which ignores disk roots.
127132
for part in without_destdir.components() {
128133
if let Component::Normal(s) = part {

0 commit comments

Comments
 (0)