mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
fix(dev): avoid repeated cargo dev rebuilds
Scrub Cargo build-script environment from nested cargo commands so cargo dev does not poison fingerprints, and preserve unchanged SPA asset files while refreshing embedded assets.
This commit is contained in:
Bryan Helmkamp
parent
67ad1f520f
commit
45bebb1fd8
2 changed files with 243 additions and 19 deletions
|
|
@ -116,6 +116,9 @@ impl PlannedCommand {
|
|||
pub(crate) fn command(planned: &PlannedCommand) -> Command {
|
||||
let mut command = Command::new(&planned.program);
|
||||
command.args(&planned.args);
|
||||
if planned.program == "cargo" {
|
||||
scrub_nested_cargo_env(&mut command);
|
||||
}
|
||||
for key in &planned.unset_env {
|
||||
command.env_remove(key);
|
||||
}
|
||||
|
|
@ -144,6 +147,46 @@ pub(crate) fn capture_command(cwd: &Path, planned: &PlannedCommand) -> Result<Ou
|
|||
.with_context(|| format!("running {}", planned.to_shell_line()))
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "dev CLI sanitizes inherited Cargo build-script env before spawning nested cargo"
|
||||
)]
|
||||
fn scrub_nested_cargo_env(command: &mut Command) {
|
||||
for (key, _) in std::env::vars_os() {
|
||||
if is_cargo_build_env(&key) {
|
||||
command.env_remove(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_cargo_build_env(key: &std::ffi::OsStr) -> bool {
|
||||
let Some(key) = key.to_str() else {
|
||||
return false;
|
||||
};
|
||||
|
||||
matches!(
|
||||
key,
|
||||
"CARGO_BIN_NAME"
|
||||
| "CARGO_CRATE_NAME"
|
||||
| "CARGO_MANIFEST_DIR"
|
||||
| "CARGO_MANIFEST_PATH"
|
||||
| "CARGO_PRIMARY_PACKAGE"
|
||||
| "DEBUG"
|
||||
| "HOST"
|
||||
| "NUM_JOBS"
|
||||
| "OPT_LEVEL"
|
||||
| "OUT_DIR"
|
||||
| "PROFILE"
|
||||
| "RUSTC"
|
||||
| "RUSTDOC"
|
||||
| "TARGET"
|
||||
) || key.starts_with("CARGO_BIN_EXE_")
|
||||
|| key.starts_with("CARGO_CFG_")
|
||||
|| key.starts_with("CARGO_FEATURE_")
|
||||
|| key.starts_with("CARGO_PKG_")
|
||||
|| key.starts_with("DEP_")
|
||||
}
|
||||
|
||||
pub(crate) fn shell_arg(arg: impl AsRef<str>) -> String {
|
||||
let arg = arg.as_ref();
|
||||
shlex::try_quote(arg).map_or_else(
|
||||
|
|
@ -151,3 +194,22 @@ pub(crate) fn shell_arg(arg: impl AsRef<str>) -> String {
|
|||
std::borrow::Cow::into_owned,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::ffi::OsStr;
|
||||
|
||||
use super::{PlannedCommand, command};
|
||||
|
||||
#[test]
|
||||
fn cargo_commands_do_not_inherit_outer_manifest_dir() {
|
||||
let prepared = command(&PlannedCommand::new("cargo").arg("build"));
|
||||
|
||||
assert!(
|
||||
prepared
|
||||
.get_envs()
|
||||
.any(|(key, value)| { key == OsStr::new("CARGO_MANIFEST_DIR") && value.is_none() }),
|
||||
"nested cargo commands should scrub CARGO_MANIFEST_DIR from cargo run"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
use std::collections::BTreeSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
|
||||
|
|
@ -97,22 +98,45 @@ pub(super) fn run_bun_build(web_dir: &Path) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "dev spa refresh mirrors build output with synchronous filesystem operations"
|
||||
)]
|
||||
pub(super) fn mirror_dist(dist_dir: &Path, asset_dir: &Path) -> Result<()> {
|
||||
if !dist_dir.is_dir() {
|
||||
bail!("apps/fabro-web/dist is missing; run `bun run build` before mirroring SPA assets");
|
||||
}
|
||||
|
||||
if asset_dir.exists() {
|
||||
std::fs::remove_dir_all(asset_dir)
|
||||
.with_context(|| format!("removing {}", asset_dir.display()))?;
|
||||
}
|
||||
let plan = mirror_plan(dist_dir)?;
|
||||
remove_stale_entries(asset_dir, &plan)?;
|
||||
std::fs::create_dir_all(asset_dir)
|
||||
.with_context(|| format!("creating {}", asset_dir.display()))?;
|
||||
|
||||
for relative_dir in &plan.dirs {
|
||||
let destination = asset_dir.join(relative_dir);
|
||||
std::fs::create_dir_all(&destination)
|
||||
.with_context(|| format!("creating {}", destination.display()))?;
|
||||
}
|
||||
|
||||
for source_file in &plan.files {
|
||||
copy_if_changed(&source_file.source, &asset_dir.join(&source_file.relative))?;
|
||||
}
|
||||
|
||||
write_if_changed(&asset_dir.join(".gitkeep"), b"")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
struct MirrorPlan {
|
||||
dirs: BTreeSet<PathBuf>,
|
||||
files: Vec<SourceFile>,
|
||||
}
|
||||
|
||||
struct SourceFile {
|
||||
source: PathBuf,
|
||||
relative: PathBuf,
|
||||
}
|
||||
|
||||
fn mirror_plan(dist_dir: &Path) -> Result<MirrorPlan> {
|
||||
let mut dirs = BTreeSet::new();
|
||||
let mut files = Vec::new();
|
||||
|
||||
for entry in WalkDir::new(dist_dir) {
|
||||
let entry = entry.context("walking apps/fabro-web/dist")?;
|
||||
let source = entry.path();
|
||||
|
|
@ -123,10 +147,8 @@ pub(super) fn mirror_dist(dist_dir: &Path, asset_dir: &Path) -> Result<()> {
|
|||
continue;
|
||||
}
|
||||
|
||||
let destination = asset_dir.join(relative);
|
||||
if entry.file_type().is_dir() {
|
||||
std::fs::create_dir_all(&destination)
|
||||
.with_context(|| format!("creating {}", destination.display()))?;
|
||||
dirs.insert(relative.to_path_buf());
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -134,21 +156,135 @@ pub(super) fn mirror_dist(dist_dir: &Path, asset_dir: &Path) -> Result<()> {
|
|||
continue;
|
||||
}
|
||||
|
||||
if let Some(parent) = destination.parent() {
|
||||
std::fs::create_dir_all(parent)
|
||||
.with_context(|| format!("creating {}", parent.display()))?;
|
||||
for ancestor in relative.ancestors().skip(1) {
|
||||
if ancestor.as_os_str().is_empty() {
|
||||
break;
|
||||
}
|
||||
dirs.insert(ancestor.to_path_buf());
|
||||
}
|
||||
std::fs::copy(source, &destination).with_context(|| {
|
||||
format!("copying {} to {}", source.display(), destination.display())
|
||||
})?;
|
||||
|
||||
files.push(SourceFile {
|
||||
source: source.to_path_buf(),
|
||||
relative: relative.to_path_buf(),
|
||||
});
|
||||
}
|
||||
|
||||
std::fs::write(asset_dir.join(".gitkeep"), b"")
|
||||
.with_context(|| format!("writing {}", asset_dir.join(".gitkeep").display()))?;
|
||||
files.sort_by(|left, right| left.relative.cmp(&right.relative));
|
||||
|
||||
Ok(MirrorPlan { dirs, files })
|
||||
}
|
||||
|
||||
fn remove_stale_entries(asset_dir: &Path, plan: &MirrorPlan) -> Result<()> {
|
||||
if !asset_dir.exists() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let desired_files = plan
|
||||
.files
|
||||
.iter()
|
||||
.map(|source| source.relative.clone())
|
||||
.chain([PathBuf::from(".gitkeep")])
|
||||
.collect::<BTreeSet<_>>();
|
||||
|
||||
for entry in WalkDir::new(asset_dir).contents_first(true) {
|
||||
let entry = entry.context("walking fabro-spa assets")?;
|
||||
let path = entry.path();
|
||||
let relative = path
|
||||
.strip_prefix(asset_dir)
|
||||
.with_context(|| format!("{} is not under {}", path.display(), asset_dir.display()))?;
|
||||
if relative.as_os_str().is_empty() {
|
||||
continue;
|
||||
}
|
||||
|
||||
if entry.file_type().is_dir() {
|
||||
if !plan.dirs.contains(relative) {
|
||||
std::fs::remove_dir(path)
|
||||
.with_context(|| format!("removing {}", path.display()))?;
|
||||
}
|
||||
} else if !desired_files.contains(relative) {
|
||||
std::fs::remove_file(path).with_context(|| format!("removing {}", path.display()))?;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "dev spa refresh mirrors build output with synchronous filesystem operations"
|
||||
)]
|
||||
fn copy_if_changed(source: &Path, destination: &Path) -> Result<()> {
|
||||
if destination.is_file() && files_match(source, destination)? {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if destination.exists() {
|
||||
remove_destination(destination)?;
|
||||
}
|
||||
if let Some(parent) = destination.parent() {
|
||||
std::fs::create_dir_all(parent)
|
||||
.with_context(|| format!("creating {}", parent.display()))?;
|
||||
}
|
||||
std::fs::copy(source, destination)
|
||||
.with_context(|| format!("copying {} to {}", source.display(), destination.display()))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "dev spa refresh writes marker files synchronously while mirroring build output"
|
||||
)]
|
||||
fn write_if_changed(destination: &Path, contents: &[u8]) -> Result<()> {
|
||||
if destination.is_file()
|
||||
&& std::fs::read(destination)
|
||||
.with_context(|| format!("reading {}", destination.display()))?
|
||||
== contents
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if destination.exists() {
|
||||
remove_destination(destination)?;
|
||||
}
|
||||
if let Some(parent) = destination.parent() {
|
||||
std::fs::create_dir_all(parent)
|
||||
.with_context(|| format!("creating {}", parent.display()))?;
|
||||
}
|
||||
std::fs::write(destination, contents)
|
||||
.with_context(|| format!("writing {}", destination.display()))
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "dev spa refresh compares generated asset bytes synchronously before mirroring"
|
||||
)]
|
||||
fn files_match(left: &Path, right: &Path) -> Result<bool> {
|
||||
let left_len = left
|
||||
.metadata()
|
||||
.with_context(|| format!("reading metadata for {}", left.display()))?
|
||||
.len();
|
||||
let right_len = right
|
||||
.metadata()
|
||||
.with_context(|| format!("reading metadata for {}", right.display()))?
|
||||
.len();
|
||||
if left_len != right_len {
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
Ok(
|
||||
std::fs::read(left).with_context(|| format!("reading {}", left.display()))?
|
||||
== std::fs::read(right).with_context(|| format!("reading {}", right.display()))?,
|
||||
)
|
||||
}
|
||||
|
||||
fn remove_destination(path: &Path) -> Result<()> {
|
||||
if path.is_dir() {
|
||||
std::fs::remove_dir_all(path).with_context(|| format!("removing {}", path.display()))
|
||||
} else {
|
||||
std::fs::remove_file(path).with_context(|| format!("removing {}", path.display()))
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) struct TempDir {
|
||||
path: PathBuf,
|
||||
}
|
||||
|
|
@ -210,6 +346,32 @@ mod tests {
|
|||
std::fs::read(root.join(path)).expect("reading fixture file")
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn mirror_dist_preserves_unchanged_assets() {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
|
||||
let fixture = tempfile::tempdir().expect("creating fixture");
|
||||
write_file(fixture.path(), "dist/index.html", b"index");
|
||||
write_file(fixture.path(), "assets/index.html", b"index");
|
||||
|
||||
let before_inode = std::fs::metadata(fixture.path().join("assets/index.html"))
|
||||
.expect("reading initial metadata")
|
||||
.ino();
|
||||
|
||||
mirror_dist(&fixture.path().join("dist"), &fixture.path().join("assets"))
|
||||
.expect("mirroring dist");
|
||||
|
||||
let after_inode = std::fs::metadata(fixture.path().join("assets/index.html"))
|
||||
.expect("reading mirrored metadata")
|
||||
.ino();
|
||||
|
||||
assert_eq!(
|
||||
before_inode, after_inode,
|
||||
"unchanged asset files should not be deleted and recreated"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mirror_dist_removes_stale_files_source_maps_and_keeps_directory_tracked() {
|
||||
let fixture = tempfile::tempdir().expect("creating fixture");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue