Skip to content

Commit 60ad61b

Browse files
authored
feat(cargo-php): Atomic extension installation and smoke testing
Refs: #619 Fixes: #518
1 parent 32b0dd6 commit 60ad61b

File tree

1 file changed

+44
-1
lines changed

1 file changed

+44
-1
lines changed

crates/cli/src/lib.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ struct Install {
121121
/// Whether to bypass the install prompt.
122122
#[clap(long)]
123123
yes: bool,
124+
/// Skip the smoke test that verifies the extension loads correctly.
125+
#[clap(long)]
126+
no_smoke_test: bool,
124127
}
125128

126129
#[derive(Parser)]
@@ -221,10 +224,50 @@ impl Install {
221224
ext_dir.push(ext_name);
222225
}
223226

224-
std::fs::copy(&ext_path, &ext_dir).with_context(
227+
// Use atomic copy: copy to temp file in same directory, then rename.
228+
// This prevents race conditions where a partially-written extension could be loaded.
229+
let temp_ext_path = ext_dir.with_extension(format!(
230+
"{}.tmp.{}",
231+
ext_dir
232+
.extension()
233+
.map(|e| e.to_string_lossy())
234+
.unwrap_or_default(),
235+
std::process::id()
236+
));
237+
238+
std::fs::copy(&ext_path, &temp_ext_path).with_context(
225239
|| "Failed to copy extension from target directory to extension directory",
226240
)?;
227241

242+
// Rename is atomic on POSIX when source and destination are on the same filesystem
243+
if let Err(e) = std::fs::rename(&temp_ext_path, &ext_dir) {
244+
// Clean up temp file on failure
245+
let _ = std::fs::remove_file(&temp_ext_path);
246+
return Err(e).with_context(|| "Failed to rename extension to final destination");
247+
}
248+
249+
// Smoke test: verify the extension loads correctly before enabling it in php.ini.
250+
// This prevents broken extensions from crashing PHP on startup.
251+
if !self.no_smoke_test {
252+
let smoke_test = Command::new("php")
253+
.arg("-d")
254+
.arg(format!("extension={}", ext_dir.display()))
255+
.arg("-r")
256+
.arg("")
257+
.output()
258+
.context("Failed to run PHP for smoke test")?;
259+
260+
if !smoke_test.status.success() {
261+
// Extension failed to load - remove it and report the error
262+
let _ = std::fs::remove_file(&ext_dir);
263+
let stderr = String::from_utf8_lossy(&smoke_test.stderr);
264+
bail!(
265+
"Extension failed to load during smoke test. The extension file has been removed.\n\
266+
PHP output:\n{stderr}"
267+
);
268+
}
269+
}
270+
228271
if let Some(php_ini) = php_ini {
229272
let mut file = OpenOptions::new()
230273
.read(true)

0 commit comments

Comments
 (0)