add HybridCompile sdkconfig file to hash#446
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughBoth Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
builder/frameworks/arduino.py (1)
76-80:⚠️ Potential issue | 🟠 Major
PathCache.sdk_dircan still crash when framework libs are missing.At Line 76-Line 80,
Path(self.framework_lib_dir)is evaluated even whenframework_lib_dirisNone, which raises before downstream guards run.Suggested fix
`@property` def sdk_dir(self): if self._sdk_dir is None: + if not self.framework_lib_dir: + return None self._sdk_dir = fs.to_unix_path( str(Path(self.framework_lib_dir) / self.chip_variant / "include") ) return self._sdk_dir🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/arduino.py` around lines 76 - 80, The sdk_dir property (def sdk_dir(self)) can throw when self.framework_lib_dir is None because Path(self.framework_lib_dir) is evaluated unguarded; update sdk_dir to first check if self.framework_lib_dir is truthy before constructing Path/self.chip_variant (e.g., if self._sdk_dir is None: if not self.framework_lib_dir: return None or leave self._sdk_dir as None; otherwise compute fs.to_unix_path(str(Path(self.framework_lib_dir) / self.chip_variant / "include"))). Ensure you only call Path(...) and fs.to_unix_path when framework_lib_dir is present and preserve existing memoization using self._sdk_dir.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@builder/frameworks/arduino.py`:
- Around line 280-293: The current loop over entry_custom_sdkconfig only handles
the first "file://" entry and prepends its mtime, which misses additional
references and doesn't use file content for robust invalidation; update the loop
that processes entry_custom_sdkconfig (the block referencing
entry_custom_sdkconfig, project_dir, isabs, file_ref, file_path) to iterate all
lines, for every line starting with "file://" resolve file_path, attempt to open
and read the file content (fall back to mtime if reading fails), and incorporate
each file's content (or its mtime) into the value used for hash computation
(e.g., prepend/append each file's data to entry_custom_sdkconfig) while removing
the stray break and preserving OSError handling so missing/unreadable files
don’t crash the flow.
In `@builder/frameworks/espidf.py`:
- Around line 782-799: The checksum input currently only captures the first
file:// entry mtime; update the logic around
env.GetProjectOption("custom_sdkconfig") and the loop over raw.splitlines() so
that you resolve every file:// reference (not just the first), read each
referenced file’s contents and/or mtime, and include those resolved
contents/mtimes in custom_sdk_config_flags before appending the raw option;
ensure you iterate all entries, resolve relative paths against PROJECT_DIR,
handle OSError for missing files (skip but continue), and concatenate all
resolved file information plus the original raw text (using the existing
raw.rstrip("\n") + "\n" pattern) so changes to any referenced file invalidate
the cache.
---
Outside diff comments:
In `@builder/frameworks/arduino.py`:
- Around line 76-80: The sdk_dir property (def sdk_dir(self)) can throw when
self.framework_lib_dir is None because Path(self.framework_lib_dir) is evaluated
unguarded; update sdk_dir to first check if self.framework_lib_dir is truthy
before constructing Path/self.chip_variant (e.g., if self._sdk_dir is None: if
not self.framework_lib_dir: return None or leave self._sdk_dir as None;
otherwise compute fs.to_unix_path(str(Path(self.framework_lib_dir) /
self.chip_variant / "include"))). Ensure you only call Path(...) and
fs.to_unix_path when framework_lib_dir is present and preserve existing
memoization using self._sdk_dir.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e616bbec-aaf5-481b-b93d-b30bfba59557
📒 Files selected for processing (2)
builder/frameworks/arduino.pybuilder/frameworks/espidf.py
1 participant
Description:
Related issue (if applicable): fixes #441
Checklist:
Summary by CodeRabbit