Skip to content

refactor(esp32): take NormalizedPath in Esp32Linker; retire linker-test PathBuf exception (#1512) #1602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zackees merged 1 commit into from
Sep 30, 2026
Merged

refactor(esp32): take NormalizedPath in Esp32Linker; retire linker-test PathBuf exception (#1512) #1602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 15 additions & 14 deletions crates/fbuild-build-esp/src/esp32/esp32_linker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use std::path::{Path, PathBuf};

use fbuild_core::path::NormalizedPath;
use fbuild_core::subprocess::run_command;
use fbuild_core::{BuildProfile, Result, SizeInfo};

Expand Down Expand Up @@ -146,11 +147,11 @@ pub(crate) fn esptool_spawn_failure_message(esptool_bin: Option<&Path>, error: &

/// ESP32-specific linker using RISC-V or Xtensa GCC as the link driver.
pub struct Esp32Linker {
gcc_path: PathBuf,
ar_path: PathBuf,
gcc_path: NormalizedPath,
ar_path: NormalizedPath,
#[allow(dead_code)] // Used later for esptool elf2image
objcopy_path: PathBuf,
size_path: PathBuf,
objcopy_path: NormalizedPath,
size_path: NormalizedPath,
/// MCU config (used for profile-specific flags as fallback).
mcu_config: Esp32McuConfig,
/// SDK linker flags from `flags/ld_flags` (undefined symbols, wrap directives, etc.).
Expand All @@ -169,7 +170,7 @@ pub struct Esp32Linker {
max_ram: Option<u64>,
/// Path to the provisioned standalone esptool binary, if available. `None`
/// falls back to an `esptool` on PATH. See FastLED/fbuild#954.
esptool_bin: Option<PathBuf>,
esptool_bin: Option<NormalizedPath>,
verbose: bool,
/// The CLI caller's PATH, so the bare-`esptool` fallback resolves
/// against the caller's environment instead of the daemon's
Expand All @@ -183,10 +184,10 @@ pub struct Esp32Linker {
impl Esp32Linker {
#[allow(clippy::too_many_arguments)]
pub fn new(
gcc_path: PathBuf,
ar_path: PathBuf,
objcopy_path: PathBuf,
size_path: PathBuf,
gcc_path: NormalizedPath,
ar_path: NormalizedPath,
objcopy_path: NormalizedPath,
size_path: NormalizedPath,
mcu_config: Esp32McuConfig,
sdk_ld_flags: Vec<String>,
sdk_lib_flags: Vec<String>,
Expand All @@ -196,7 +197,7 @@ impl Esp32Linker {
flash_freq: &str,
max_flash: Option<u64>,
max_ram: Option<u64>,
esptool_bin: Option<PathBuf>,
esptool_bin: Option<NormalizedPath>,
verbose: bool,
) -> Self {
let flash_mode = flash_mode.unwrap_or_else(|| mcu_config.default_flash_mode().to_string());
Expand Down Expand Up @@ -574,19 +575,19 @@ impl Linker for Esp32Linker {
}

fn size_tool_path(&self) -> &Path {
&self.size_path
self.size_path.as_path()
}

fn ar_tool_path(&self) -> Option<&Path> {
Some(&self.ar_path)
Some(self.ar_path.as_path())
}

fn objcopy_tool_path(&self) -> Option<&Path> {
Some(&self.objcopy_path)
Some(self.objcopy_path.as_path())
}

fn link_driver_path(&self) -> Option<&Path> {
Some(&self.gcc_path)
Some(self.gcc_path.as_path())
}

async fn report_size(&self, elf_path: &Path) -> Result<SizeInfo> {
Expand Down
43 changes: 23 additions & 20 deletions crates/fbuild-build-esp/src/esp32/esp32_linker_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ fn test_linker(mcu: &str) -> Esp32Linker {
let config = get_mcu_config(mcu).unwrap();
let prefix = config.toolchain_prefix();
Esp32Linker::new(
PathBuf::from(format!("/usr/bin/{}gcc", prefix)),
PathBuf::from(format!("/usr/bin/{}ar", prefix)),
PathBuf::from(format!("/usr/bin/{}objcopy", prefix)),
PathBuf::from(format!("/usr/bin/{}size", prefix)),
NormalizedPath::new(format!("/usr/bin/{}gcc", prefix)),
NormalizedPath::new(format!("/usr/bin/{}ar", prefix)),
NormalizedPath::new(format!("/usr/bin/{}objcopy", prefix)),
NormalizedPath::new(format!("/usr/bin/{}size", prefix)),
config,
vec![
"-nostartfiles".to_string(),
Expand Down Expand Up @@ -70,7 +70,10 @@ fn pinned_esptool_v4_uses_underscore_elf2image_options() {
assert!(v5.iter().any(|arg| arg == "--flash-mode"));
}

fn test_linker_with(esptool_bin: Option<PathBuf>, caller_path: Option<String>) -> Esp32Linker {
fn test_linker_with(
esptool_bin: Option<NormalizedPath>,
caller_path: Option<String>,
) -> Esp32Linker {
let mut linker = test_linker("esp32c6");
linker.esptool_bin = esptool_bin;
linker.caller_path = caller_path;
Expand Down Expand Up @@ -121,7 +124,7 @@ fn absolute_esptool_bin_reuse_ignores_caller_path() {
let elf = tmp.path().join("firmware.elf");
std::fs::write(&elf, b"elf").unwrap();

let esptool = PathBuf::from("C:\\tools\\esptool.exe");
let esptool = NormalizedPath::new("C:\\tools\\esptool.exe");
let linker_a = test_linker_with(Some(esptool.clone()), Some("C:\\venv-a".to_string()));
let flash_size = linker_a.flash_size();

Expand All @@ -145,10 +148,10 @@ fn test_flash_size_uses_board_max_flash_for_elf2image_and_cache() {
let config = get_mcu_config("esp32c6").unwrap();
let prefix = config.toolchain_prefix();
let linker = Esp32Linker::new(
PathBuf::from(format!("/usr/bin/{}gcc", prefix)),
PathBuf::from(format!("/usr/bin/{}ar", prefix)),
PathBuf::from(format!("/usr/bin/{}objcopy", prefix)),
PathBuf::from(format!("/usr/bin/{}size", prefix)),
NormalizedPath::new(format!("/usr/bin/{}gcc", prefix)),
NormalizedPath::new(format!("/usr/bin/{}ar", prefix)),
NormalizedPath::new(format!("/usr/bin/{}objcopy", prefix)),
NormalizedPath::new(format!("/usr/bin/{}size", prefix)),
config,
vec![],
vec![],
Expand Down Expand Up @@ -182,7 +185,7 @@ fn test_esp32_link_command_emits_linker_map_next_to_elf() {
let args = linker.build_link_args(
&[],
&[],
&PathBuf::from("/build/firmware.elf"),
Path::new("/build/firmware.elf"),
&LinkExtraArgs::default(),
);
assert!(
Expand Down Expand Up @@ -239,10 +242,10 @@ fn test_linker_flags_fallback_to_config() {
let prefix = config.toolchain_prefix();
// Empty sdk_ld_flags → falls back to MCU config
let linker = Esp32Linker::new(
PathBuf::from(format!("/usr/bin/{}gcc", prefix)),
PathBuf::from(format!("/usr/bin/{}ar", prefix)),
PathBuf::from(format!("/usr/bin/{}objcopy", prefix)),
PathBuf::from(format!("/usr/bin/{}size", prefix)),
NormalizedPath::new(format!("/usr/bin/{}gcc", prefix)),
NormalizedPath::new(format!("/usr/bin/{}ar", prefix)),
NormalizedPath::new(format!("/usr/bin/{}objcopy", prefix)),
NormalizedPath::new(format!("/usr/bin/{}size", prefix)),
config,
vec![],
vec!["-lfreertos".to_string()],
Expand Down Expand Up @@ -283,10 +286,10 @@ fn test_xtensa_linker_flags() {
let config = get_mcu_config("esp32").unwrap();
let prefix = config.toolchain_prefix();
let linker = Esp32Linker::new(
PathBuf::from(format!("/usr/bin/{}gcc", prefix)),
PathBuf::from(format!("/usr/bin/{}ar", prefix)),
PathBuf::from(format!("/usr/bin/{}objcopy", prefix)),
PathBuf::from(format!("/usr/bin/{}size", prefix)),
NormalizedPath::new(format!("/usr/bin/{}gcc", prefix)),
NormalizedPath::new(format!("/usr/bin/{}ar", prefix)),
NormalizedPath::new(format!("/usr/bin/{}objcopy", prefix)),
NormalizedPath::new(format!("/usr/bin/{}size", prefix)),
config,
vec!["-mlongcalls".to_string()],
vec![],
Expand Down Expand Up @@ -423,7 +426,7 @@ async fn link_runs_the_linker_under_the_c_locale() {
fbuild_core::platform::fs::set_executable(&staging).unwrap();
std::fs::rename(&staging, &fake_gcc).unwrap();
let mut linker = test_linker("esp32s3");
linker.gcc_path = fake_gcc;
linker.gcc_path = fake_gcc.into();

linker
.link(&[], &[], &tmp.path().join("out"), &LinkExtraArgs::default())
Expand Down
10 changes: 5 additions & 5 deletions crates/fbuild-build-esp/src/esp32/orchestrator/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,10 +846,10 @@ impl BuildOrchestrator for Esp32Orchestrator {
all_archives.extend(library_archives);

let linker = Esp32Linker::new(
toolchain.get_gcc_path(),
toolchain.get_ar_path(),
toolchain.get_objcopy_path(),
toolchain.get_size_path(),
toolchain.get_gcc_path().into(),
toolchain.get_ar_path().into(),
toolchain.get_objcopy_path().into(),
toolchain.get_size_path().into(),
mcu_config.clone(),
sdk_ld_flags,
sdk_lib_flags,
Expand All @@ -859,7 +859,7 @@ impl BuildOrchestrator for Esp32Orchestrator {
&flash_freq,
ctx.board.max_flash,
ctx.board.max_ram,
esptool_bin.clone().map(|path| path.into_path_buf()),
esptool_bin.clone(),
params.verbose,
)
.with_caller_path(params.caller_path.clone())
Expand Down
3 changes: 0 additions & 3 deletions dylints/ban_std_pathbuf/src/allowlist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ crates/fbuild-build-engine/src/compiler.rs
crates/fbuild-build-engine/src/compiler_tests.rs
crates/fbuild-build-esp/src/esp32/esp32_compiler.rs
crates/fbuild-build-esp/src/esp32/esp32_linker.rs
# Existing ESP32 linker test fixtures construct raw paths to match the legacy
# production linker's PathBuf API; migrate together with that API (#1501).
crates/fbuild-build-esp/src/esp32/esp32_linker_tests.rs
crates/fbuild-build-esp/src/esp32/orchestrator/build.rs
crates/fbuild-build-esp/src/esp32/orchestrator/embed.rs
crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs
Expand Down
Loading