HybridCompile: replace move operations with copy #346

Jason2866 · 2025-11-21T12:28:39Z

Description:

dont delete exiting *.a libs to avoid linker errors with unused libs

Related issue (if applicable): fixes #336

Checklist:

The pull request is done against the latest develop branch
Only relevant files were touched
Only one feature/fix was added per PR, more changes are allowed when changing boards.json
I accept the CLA

Summary by CodeRabbit

Bug Fixes
- Made library copying during Arduino + ESP-IDF builds more robust to missing or variant-specific files, avoiding build failures.
- Switched to non-fatal warnings on copy errors and standardized copy behavior across ESP32 variants to reduce integration interruptions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Dont delete exiting *.a libs to avoid linker errors with unused libs

coderabbitai · 2025-11-21T12:28:54Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Replaced move-based file replacement with copy-based operations in the Arduino–ESP-IDF integration; copy failures are now caught and warn instead of raising, and the new handler (_replace_copy) is used for ESP32-S3 and related library files.

Changes

Cohort / File(s)	Summary
Arduino‑IDF library copy handler `builder/frameworks/espidf.py`	Replaced inner helper `_replace_move` with `_replace_copy`; switched from `os.replace`/`os.remove` to `shutil.copy2`; broadened exception handling to catch `OSError`, `IOError`, and generic exceptions and emit warnings instead of raising; updated all invocations to use `_replace_copy` for ESP32-S3 and common Arduino/ESP-IDF prebuilt libs (e.g., `libesp_psram.a`, `libesp_system.a`, `libfreertos.a`, `libbootloader_support.a`, `libesp_hw_support.a`, `libesp_lcd.a`, `libspi_flash.a`, `memory.ld`).

Sequence Diagram(s)

sequenceDiagram
    participant Build as Build Process
    participant Handler as Replace Handler
    participant FS as File System

    rect rgb(240,230,255)
    Note over Handler: Old flow — _replace_move
    Build->>Handler: request replace file
    Handler->>FS: if dest exists -> remove(dest)
    Handler->>FS: move(src, dest)
    FS-->>Handler: File missing / move fails (error)
    Handler-->>Build: raise exception (fail)
    end

    rect rgb(230,255,240)
    Note over Handler: New flow — _replace_copy (current PR)
    Build->>Handler: request replace file
    Handler->>FS: copy2(src, dest)
    FS-->>Handler: success / or error
    alt copy error (missing/IO)
        Handler-->>Build: emit warning, continue
    else success
        Handler-->>Build: success
    end
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Areas to spot-check:
- builder/frameworks/espidf.py — verify all previous _replace_move call sites were updated to _replace_copy.
- Confirm warning messages are informative and not overly noisy.
- Ensure no logic relied on atomic replace semantics (race conditions) where a move was required.

Poem

🐰 I hopped through code with nimble paws,

Copies now dance, no moves, no cause.
PSRAM may hide, yet builds stay bright,
Warnings whisper — all is right. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: replacing move operations with copy operations for handling library files in the Arduino-IDF integration code path.
Linked Issues check	✅ Passed	The PR changes replace move operations with copy operations and add non-fatal error handling, directly addressing issue #336's root cause of missing PSRAM libraries in mixed-environment builds.
Out of Scope Changes check	✅ Passed	All changes in builder/frameworks/espidf.py are focused on the copy/move operation logic and error handling for library files, staying within the scope of fixing the PSRAM library lookup issue.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Jason2866 · 2025-11-21T12:40:03Z

@coderabbitai full review

coderabbitai · 2025-11-21T12:40:09Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5fffba and ad6bacf.

📒 Files selected for processing (1)

builder/frameworks/espidf.py (2 hunks)

🧰 Additional context used

🧠 Learnings (6)

📓 Common learnings

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 285
File: platform.py:670-683
Timestamp: 2025-09-09T18:07:14.583Z
Learning: In the pioarduino/platform-espressif32 project, when tools.json exists in mklittlefs tool directories, it triggers a reinstall via install_tool(). This is intentional behavior to convert tools installed via idf_tools.py method into the platform's tool management system, rather than a redundant check that should be avoided.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.

📚 Learning: 2025-09-23T14:15:00.476Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates and manages its own separate IDF virtual environment (accessed via get_python_exe() from get_idf_venv_dir()). This IDF venv is completely isolated from the penv setup, so different cryptography versions between penv_setup.py and espidf.py don't cause conflicts as they exist in separate virtual environments.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-09-23T14:15:00.476Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates an extra/separate virtual environment from the penv setup, so cryptography version differences between penv_setup.py and espidf.py do not cause conflicts since they operate in isolated environments.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-10-19T17:30:42.403Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-10-19T17:30:42.403Z
Learning: The tool-esp-rom-elfs package must remain in COMMON_IDF_PACKAGES in platform.py because espidf.py requires it to set the ESP_ROM_ELF_DIR environment variable for the ESP-IDF build system, independent of the exception decoder monitor filter usage.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-09-09T18:07:14.583Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 285
File: platform.py:670-683
Timestamp: 2025-09-09T18:07:14.583Z
Learning: In the pioarduino/platform-espressif32 project, when tools.json exists in mklittlefs tool directories, it triggers a reinstall via install_tool(). This is intentional behavior to convert tools installed via idf_tools.py method into the platform's tool management system, rather than a redundant check that should be avoided.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-08-20T14:13:13.842Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.

Applied to files:

builder/frameworks/espidf.py

🪛 Ruff (0.14.5)

builder/frameworks/espidf.py

2595-2595: Do not use bare except

(E722)

2595-2596: try-except-pass detected, consider logging the exception

(S110)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)

GitHub Check: build (macos-15, examples/espidf-ulp-lp)
GitHub Check: build (macos-15, examples/espidf-exceptions)
GitHub Check: build (macos-15, examples/espidf-http-request)
GitHub Check: build (macos-15, examples/espidf-blink)
GitHub Check: build (windows-latest, examples/espidf-hello-world)
GitHub Check: build (windows-latest, examples/espidf-storage-sdcard)
GitHub Check: build (windows-latest, examples/espidf-peripherals-usb)
GitHub Check: build (windows-latest, examples/espidf-arduino-littlefs)
GitHub Check: build (macos-15, examples/arduino-blink)
GitHub Check: build (windows-latest, examples/espidf-arduino-blink)
GitHub Check: build (windows-latest, examples/espidf-exceptions)
GitHub Check: build (windows-latest, examples/espidf-blink)
GitHub Check: build (windows-latest, examples/arduino-rmt-blink)
GitHub Check: build (ubuntu-latest, examples/espidf-arduino-matter-light)
GitHub Check: build (windows-latest, examples/tasmota)
GitHub Check: build (windows-latest, examples/arduino-blink)
GitHub Check: build (windows-latest, examples/arduino-wifiscan)
GitHub Check: build (ubuntu-latest, examples/arduino-blink)

builder/frameworks/espidf.py

Improve error handling for file copy operations.

Jason2866 · 2025-11-21T12:53:49Z

@coderabbitai full review

coderabbitai · 2025-11-21T12:54:04Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

builder/frameworks/espidf.py (1)
2590-2600: Excellent fix! The critical issue has been resolved.

The function now correctly uses shutil.copy2() instead of os.replace(), which copies files while leaving the source intact. This directly addresses issue #336 by ensuring PSRAM libraries remain available for PSRAM-enabled environments while PSRAM-disabled environments can gracefully handle their absence.

The exception handling is well-structured with clear intent. One minor cleanup opportunity:

Apply this diff to remove the unused variable:
-            except (OSError, IOError) as e:
+            except (OSError, IOError):
                 # Gracefully handle missing source files (e.g., PSRAM libs in non-PSRAM builds)
                 # This is expected when copying variant-specific libraries
                 pass

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5fffba and 32d3aff.

📒 Files selected for processing (1)

builder/frameworks/espidf.py (2 hunks)

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 285
File: platform.py:670-683
Timestamp: 2025-09-09T18:07:14.583Z
Learning: In the pioarduino/platform-espressif32 project, when tools.json exists in mklittlefs tool directories, it triggers a reinstall via install_tool(). This is intentional behavior to convert tools installed via idf_tools.py method into the platform's tool management system, rather than a redundant check that should be avoided.

📚 Learning: 2025-09-23T14:15:00.476Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates and manages its own separate IDF virtual environment (accessed via get_python_exe() from get_idf_venv_dir()). This IDF venv is completely isolated from the penv setup, so different cryptography versions between penv_setup.py and espidf.py don't cause conflicts as they exist in separate virtual environments.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-09-23T14:15:00.476Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates an extra/separate virtual environment from the penv setup, so cryptography version differences between penv_setup.py and espidf.py do not cause conflicts since they operate in isolated environments.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-10-19T17:30:42.403Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-10-19T17:30:42.403Z
Learning: The tool-esp-rom-elfs package must remain in COMMON_IDF_PACKAGES in platform.py because espidf.py requires it to set the ESP_ROM_ELF_DIR environment variable for the ESP-IDF build system, independent of the exception decoder monitor filter usage.

Applied to files:

builder/frameworks/espidf.py

📚 Learning: 2025-08-20T14:13:13.842Z

Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.

Applied to files:

builder/frameworks/espidf.py

🪛 Ruff (0.14.5)

builder/frameworks/espidf.py

2595-2595: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)

2599-2599: Do not catch blind exception: Exception

(BLE001)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)

GitHub Check: build (macos-15, examples/arduino-rmt-blink)
GitHub Check: build (windows-latest, examples/espidf-hello-world)
GitHub Check: build (windows-latest, examples/arduino-matter-light)
GitHub Check: build (windows-latest, examples/arduino-blink)
GitHub Check: build (windows-latest, examples/arduino-wifiscan)
GitHub Check: build (ubuntu-latest, examples/espidf-ulp-riscv)
GitHub Check: build (ubuntu-latest, examples/espidf-http-request)
GitHub Check: build (ubuntu-latest, examples/espidf-arduino-littlefs)
GitHub Check: build (ubuntu-latest, examples/espidf-arduino-blink)
GitHub Check: build (ubuntu-latest, examples/espidf-arduino-matter-light)
GitHub Check: build (ubuntu-latest, examples/arduino-NimBLE-SampleScan)
GitHub Check: build (ubuntu-latest, examples/espidf-blink)
GitHub Check: build (ubuntu-latest, examples/arduino-wifiscan)
GitHub Check: build (ubuntu-latest, examples/arduino-blink)

🔇 Additional comments (1)

builder/frameworks/espidf.py (1)

2619-2627: LGTM! Correct usage of the copy-based approach.

All ESP32-S3 variant-specific libraries now use _replace_copy, which copies instead of moving files. This ensures that libraries remain available for all environments in mixed PSRAM/non-PSRAM configurations, directly addressing the checkprogsize failures described in issue #336.

Removed exception variable usage in error handling.

HybridCompile: replace move operations with copy

ad6bacf

Dont delete exiting *.a libs to avoid linker errors with unused libs

coderabbitai bot reviewed Nov 21, 2025

View reviewed changes

builder/frameworks/espidf.py Show resolved Hide resolved

Enhance file copy error handling in espidf.py

32d3aff

Improve error handling for file copy operations.

coderabbitai bot reviewed Nov 21, 2025

View reviewed changes

Jason2866 added 2 commits November 21, 2025 14:07

Simplify exception handling in espidf.py

103b99e

Removed exception variable usage in error handling.

Improve exception handling in espidf.py

095e392

Jason2866 mentioned this pull request Nov 21, 2025

libesp_psram.a: No such file or directory #336

Closed

Jason2866 merged commit d6c0345 into develop Nov 21, 2025
1 check passed

Jason2866 deleted the fix_issue_336 branch November 21, 2025 17:25

coderabbitai bot mentioned this pull request Nov 24, 2025

Make idf compile possible for MCUs which have no precompiled Arduino libraries #353

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HybridCompile: replace move operations with copy #346

HybridCompile: replace move operations with copy #346

Uh oh!

Jason2866 commented Nov 21, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 21, 2025 •

edited

Loading

Review skipped

Uh oh!

Jason2866 commented Nov 21, 2025

Uh oh!

coderabbitai bot commented Nov 21, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Jason2866 commented Nov 21, 2025

Uh oh!

coderabbitai bot commented Nov 21, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HybridCompile: replace move operations with copy #346

HybridCompile: replace move operations with copy #346

Uh oh!

Conversation

Jason2866 commented Nov 21, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Jason2866 commented Nov 21, 2025

Uh oh!

coderabbitai bot commented Nov 21, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jason2866 commented Nov 21, 2025

Uh oh!

coderabbitai bot commented Nov 21, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jason2866 commented Nov 21, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 21, 2025 •

edited

Loading