Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions agents/docs/commands-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ help text).
| Command | Use this when | See more |
|---|---|---|
| `fbuild build` | You want to compile firmware for the env specified by `-e <env>` and `<project_dir>`. The default path; cache via daemon. | `fbuild help build` |
| `fbuild clean sketch|all` | You want to remove one environment/profile's project outputs, optionally including its exact reusable framework-cache entries, without compiling or deploying. | `fbuild help clean`, FastLED/fbuild#1089 |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Escape the pipe in the command name.

sketch|all creates an extra table cell, so the “See more” value is dropped.

Proposed fix
-| `fbuild clean sketch|all` | You want to remove one environment/profile's project outputs, optionally including its exact reusable framework-cache entries, without compiling or deploying. | `fbuild help clean`, FastLED/fbuild#1089 |
+| `fbuild clean sketch\|all` | You want to remove one environment/profile's project outputs, optionally including its exact reusable framework-cache entries, without compiling or deploying. | `fbuild help clean`, FastLED/fbuild#1089 |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| `fbuild clean sketch|all` | You want to remove one environment/profile's project outputs, optionally including its exact reusable framework-cache entries, without compiling or deploying. | `fbuild help clean`, FastLED/fbuild#1089 |
| `fbuild clean sketch\|all` | You want to remove one environment/profile's project outputs, optionally including its exact reusable framework-cache entries, without compiling or deploying. | `fbuild help clean`, FastLED/fbuild#1089 |
🧰 Tools
🪛 LanguageTool

[style] ~15-~15: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ld| |fbuild clean sketch|all` | You want to remove one environment/profile's projec...

(REP_WANT_TO_VB)

🪛 markdownlint-cli2 (0.23.0)

[warning] 15-15: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agents/docs/commands-reference.md` at line 15, Escape the pipe character in
the command name within the Markdown table row so “sketch|all” remains in a
single cell and the “See more” value is preserved. Update only the affected
command-name entry in the documentation table.

Source: Linters/SAST tools

| `fbuild deploy` | You want to build AND flash to a connected board. Pass `--monitor` to attach the monitor after flash. | `fbuild help deploy` |
| `fbuild monitor` | You want to attach the serial monitor to an already-running board without re-flashing. | `fbuild help monitor` |
| `fbuild reset` | You want to reset the device without re-flashing. | `fbuild help reset` |
Expand Down
60 changes: 60 additions & 0 deletions crates/fbuild-build-engine/src/framework_core_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ impl FrameworkCoreCache {
std::fs::create_dir_all(&self.path)?;
Ok(copy_artifacts(core_build_dir, &self.path, true, true)?.stats)
}

/// Remove only this content-addressed cache entry.
///
/// The parent cache root may contain entries for other projects,
/// environments, profiles, or compiler signatures and must remain intact.
pub fn remove(&self) -> std::io::Result<()> {
match std::fs::remove_dir_all(&self.path) {
Ok(()) => Ok(()),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()),
Err(error) => Err(error),
}
}
}

fn core_cache_key(
Expand Down Expand Up @@ -537,4 +549,52 @@ mod tests {
assert_eq!(outcome.stats.skipped, 1);
assert_eq!(std::fs::read(dst.join("main.cpp.o")).unwrap(), b"project");
}

#[test]
fn remove_deletes_only_the_selected_cache_entry() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path().join("project");
std::fs::create_dir_all(&project).unwrap();
let compiler = FakeCompiler::new();
let flags = LanguageExtraFlags {
common: Vec::new(),
c: Vec::new(),
cxx: Vec::new(),
asm: Vec::new(),
};
let source = project.join("src/main.cpp");
std::fs::create_dir_all(source.parent().unwrap()).unwrap();
std::fs::write(&source, b"same").unwrap();
let selected = FrameworkCoreCache::new(
&project,
"avr",
"uno",
BuildProfile::Release,
&compiler,
std::slice::from_ref(&source),
&flags,
);
let sibling = FrameworkCoreCache::new(
&project,
"avr",
"mega",
BuildProfile::Release,
&compiler,
std::slice::from_ref(&source),
&flags,
);
std::fs::create_dir_all(selected.path()).unwrap();
std::fs::create_dir_all(sibling.path()).unwrap();
std::fs::write(selected.path().join("marker"), b"selected").unwrap();
std::fs::write(sibling.path().join("marker"), b"sibling").unwrap();

selected.remove().unwrap();

assert!(!selected.path().exists());
assert_eq!(
std::fs::read(sibling.path().join("marker")).unwrap(),
b"sibling"
);
selected.remove().unwrap();
}
}
2 changes: 2 additions & 0 deletions crates/fbuild-build-engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ pub struct BuildParams {
/// Remove the matching reusable framework caches before building. Implies
/// `clean` at the CLI boundary; normal `clean` only removes project output.
pub clean_all: bool,
/// When true, remove outputs/cache entries without compiling or linking.
pub clean_only: bool,
pub clean: bool,
pub profile: BuildProfile,
pub build_dir: PathBuf,
Expand Down
65 changes: 49 additions & 16 deletions crates/fbuild-build-engine/src/pipeline/sequential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,54 @@ pub async fn run_sequential_build_with_libs(
// libs → link), but each phase fans out file compilation across `jobs`
// threads via `compile_sources_parallel`.
let jobs = crate::parallel::effective_jobs(params.jobs);
let core_cache = crate::framework_core_cache::FrameworkCoreCache::new(
&params.project_dir,
platform_label,
&params.env_name,
params.profile,
compiler,
&core_and_variant,
&user_overlay,
);
if params.clean_all {
let _g = perf.phase("core-cache-remove");
match core_cache.remove() {
Ok(()) => tracing::info!(
"removed framework core cache key={} at {}",
core_cache.key(),
core_cache.path().display()
),
Err(error) => tracing::warn!(
"failed to remove framework core cache key={} at {}: {}",
core_cache.key(),
core_cache.path().display(),
error
),
}
}
Comment on lines +107 to +122

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fail clean all when exact cache eviction fails. These paths only warn on removal errors and then report a successful cleanup, leaving reusable cache entries intact.

  • crates/fbuild-build-engine/src/pipeline/sequential.rs#L107-L122: propagate core_cache.remove() failures instead of continuing to the successful clean-only result.
  • crates/fbuild-build-esp/src/esp32/orchestrator/build.rs#L613-L627: propagate ESP32 core-cache removal failures.
  • crates/fbuild-build-esp/src/esp32/orchestrator/framework_libs.rs#L105-L112: propagate ESP32 framework-library cache removal failures.
📍 Affects 3 files
  • crates/fbuild-build-engine/src/pipeline/sequential.rs#L107-L122 (this comment)
  • crates/fbuild-build-esp/src/esp32/orchestrator/build.rs#L613-L627
  • crates/fbuild-build-esp/src/esp32/orchestrator/framework_libs.rs#L105-L112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-build-engine/src/pipeline/sequential.rs` around lines 107 -
122, Make clean-all fail when exact cache eviction fails instead of logging a
warning and returning success. In sequential.rs, propagate errors from
core_cache.remove() in the clean_all branch; apply the same propagation to the
ESP32 core-cache removal in build.rs and framework-library cache removal in
framework_libs.rs, preserving successful removal handling.

if params.clean_only {
if params.build_dir.exists() {
std::fs::remove_dir_all(&params.build_dir)?;
}
return Ok(BuildResult {
success: true,
firmware_path: None,
elf_path: None,
size_info: None,
symbol_map: None,
build_time_secs: start.elapsed().as_secs_f64(),
message: format!(
"cleaned {} ({})",
params.env_name,
params.profile.as_dir_name()
),
compile_database_path: None,
build_log: ctx.build_log,
});
}
let build_log_mutex = std::sync::Mutex::new(ctx.build_log);

let core_cache = if params.clean {
None
} else {
Some(crate::framework_core_cache::FrameworkCoreCache::new(
&params.project_dir,
platform_label,
&params.env_name,
params.profile,
compiler,
&core_and_variant,
&user_overlay,
))
};
if let Some(cache) = core_cache.as_ref() {
{
let cache = &core_cache;
let _g = perf.phase("core-cache-hydrate");
match cache.hydrate(
&ctx.core_build_dir,
Expand Down Expand Up @@ -165,7 +197,8 @@ pub async fn run_sequential_build_with_libs(
.await?
};
core_objects.extend(variant_objects);
if let Some(cache) = core_cache.as_ref() {
{
let cache = &core_cache;
let _g = perf.phase("core-cache-store");
match cache.store(&ctx.core_build_dir) {
Ok(stats) if stats.copied > 0 => tracing::info!(
Expand Down
24 changes: 22 additions & 2 deletions crates/fbuild-build-esp/src/esp32/orchestrator/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,6 @@ impl BuildOrchestrator for Esp32Orchestrator {
}

// Compile core + variant sources in parallel
let build_log_mutex = std::sync::Mutex::new(ctx.build_log);
// `--clean` resets only the project build directory. Framework core
// objects are content-addressed global artifacts, so hydrate them even
// for a clean build; `--clean-all` is the explicit cache eviction path.
Expand All @@ -613,7 +612,7 @@ impl BuildOrchestrator for Esp32Orchestrator {
);
if params.clean_all {
let _g = perf.phase("core-cache-remove");
match std::fs::remove_dir_all(core_cache.path()) {
match core_cache.remove() {
Ok(()) => tracing::info!(
"removed framework core cache {}",
core_cache.path().display()
Expand All @@ -626,6 +625,27 @@ impl BuildOrchestrator for Esp32Orchestrator {
),
}
}
if params.clean_only {
if params.build_dir.exists() {
std::fs::remove_dir_all(&params.build_dir)?;
}
return Ok(BuildResult {
success: true,
firmware_path: None,
elf_path: None,
size_info: None,
symbol_map: None,
build_time_secs: start.elapsed().as_secs_f64(),
message: format!(
"cleaned {} ({})",
params.env_name,
params.profile.as_dir_name()
),
compile_database_path: None,
build_log: ctx.build_log,
});
}
let build_log_mutex = std::sync::Mutex::new(ctx.build_log);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{
let _g = perf.phase("core-cache-hydrate");
match core_cache.hydrate(core_build_dir, &compiler, &all_core_sources, &user_overlay) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ pub(super) async fn compile_framework_builtin_libs(
}
}
}
if params.clean_only {
return Ok(());
}
match framework_cache.hydrate(&fw_libs_build_dir) {
Ok(copied) if copied > 0 => tracing::info!(
"hydrated {} cached ESP32 framework library archives",
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-build/src/compile_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ async fn build_one_sketch(inputs: SketchBuildInputs) -> SketchResult {
project_dir: sketch.clone(),
env_name: env_name.clone(),
clean_all: false,
clean_only: false,
clean: false,
profile,
build_dir,
Expand Down
4 changes: 4 additions & 0 deletions crates/fbuild-build/tests/avr_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ async fn build_uno_minimal() {
project_dir: project_dir.clone(),
env_name: "uno".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down Expand Up @@ -209,6 +210,7 @@ async fn compare_with_python_output() {
env_name: "uno".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: false,
Expand Down Expand Up @@ -300,6 +302,7 @@ void loop() {
env_name: "uno".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -367,6 +370,7 @@ fn uno_build_params(project_dir: &Path, build_dir: PathBuf, clean: bool) -> Buil
clean,
clean_all: false,
profile: BuildProfile::Release,
clean_only: false,
build_dir,
verbose: false,
jobs: None,
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-build/tests/eh_frame_strip_esp32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ fn make_params(project_dir: &Path) -> BuildParams {
project_dir: project_dir.to_path_buf(),
env_name: "esp32dev".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down
8 changes: 8 additions & 0 deletions crates/fbuild-build/tests/esp32_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ void loop() {
project_dir: project_dir.to_path_buf(),
env_name: "esp32dev".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down Expand Up @@ -173,6 +174,7 @@ void loop() {
env_name: "esp32c6".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -257,6 +259,7 @@ void loop() {
env_name: "esp32c3".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -342,6 +345,7 @@ void loop() {
env_name: "esp32s3".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -417,6 +421,7 @@ async fn build_esp32s3_fixture() {
env_name: "esp32s3".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir: build_dir.clone(),
verbose: true,
Expand Down Expand Up @@ -484,6 +489,7 @@ async fn build_nightdriverstrip_demo() {
env_name: "demo".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -577,6 +583,7 @@ async fn incremental_build_at(project_dir: &std::path::Path, env_name: &str) {
env_name: env_name.to_string(),
clean_all: false,
clean: false,
clean_only: false,
profile: BuildProfile::Release,
build_dir: fbuild_paths::BuildLayout::new(
project_dir.to_path_buf(),
Expand Down Expand Up @@ -675,6 +682,7 @@ async fn incremental_nightdriverstrip_one_file_changed() {
env_name: env_name.to_string(),
clean_all: false,
clean: false,
clean_only: false,
profile: BuildProfile::Release,
build_dir: fbuild_paths::BuildLayout::new(
project_dir.clone(),
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-build/tests/nxplpc_build_flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ async fn lpc845brk_propagates_build_flags_to_library_compile_587() {
project_dir: fixture.clone(),
env_name: "lpc845brk".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-build/tests/nxplpc_core_compile_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ async fn build_core_repo(repo: &Path, env_name: &str) -> tempfile::TempDir {
project_dir: repo.to_path_buf(),
env_name: env_name.to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-build/tests/stm32_acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ async fn stm32f103c8_blink_with_spi_auto_discovers_library_205_ac4() {
project_dir: project_dir.to_path_buf(),
env_name: "stm32f103c8".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir: build_dir.clone(),
Expand Down
1 change: 1 addition & 0 deletions crates/fbuild-build/tests/teensy30_acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ async fn teensy30_analog_output_meets_205_ac2() {
// as #220 / #221.
env_name: "teensy30".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down
4 changes: 4 additions & 0 deletions crates/fbuild-build/tests/teensy_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ void loop() {
project_dir: project_dir.to_path_buf(),
env_name: "teensy41".to_string(),
clean_all: false,
clean_only: false,
clean: true,
profile: BuildProfile::Release,
build_dir,
Expand Down Expand Up @@ -154,6 +155,7 @@ void loop() {}
env_name: "teensy41".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -206,6 +208,7 @@ async fn build_teensy41_fixture() {
env_name: "teensy41".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir,
verbose: true,
Expand Down Expand Up @@ -317,6 +320,7 @@ void loop() {
env_name: "teensy30".to_string(),
clean_all: false,
clean: true,
clean_only: false,
profile: BuildProfile::Release,
build_dir: tmp.path().join(".fbuild/build/teensy30/release"),
verbose: true,
Expand Down
Loading
Loading