fix(build): always recompile version.o with explicit 7-char revision - #1125
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe Makefile has been modified to adjust REVISION computation to use a deterministic short commit hash when the working tree has no diff, and to change the version.o target dependency to a phony prerequisite to ensure regeneration on each incremental build. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Line 110: The REVISION assignment uses git rev-parse --short=7 which
guarantees at least 7 chars, so enforce an exact 7-character hash by truncating
the output: update the Makefile's REVISION assignment (the REVISION variable) to
pipe git rev-parse --short=7 HEAD into a fixed-width truncation (e.g., cut -c1-7
or similar) so the value stored in REVISION is exactly seven characters to match
GIT_SHORT_REVISION_LENGTH and MSP protocol expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Two Makefile bugs that caused firmware to report uncommitted version even on a clean tree: 1. version.o depended on $(SRC) — only rebuilt when a source file changed. A dirty-tree build (REVISION=uncommitted_...) followed by a clean-tree build with no source changes left a stale object, so the firmware reported "uncommitted" even though the filename correctly embedded the commit hash. Fix: replace $(SRC) with FORCE so version.o always recompiles, matching the comment's stated intent. 2. git rev-parse --short=7 guarantees a minimum of 7 chars but auto-extends for uniqueness, so the length can still exceed GIT_SHORT_REVISION_LENGTH (7). The MSP protocol field sends exactly GIT_SHORT_REVISION_LENGTH bytes, silently truncating any excess and producing an inconsistent hash between filename and configurator as the repo grows. Fix: git rev-parse HEAD | cut -c1-7 hard-truncates the full SHA to exactly 7 chars regardless of repo size or git configuration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d649d0a to
3fa7ef2
Compare
1 participant
Summary
Two Makefile bugs that caused firmware to report
uncommittedversion even on a clean tree:Stale
version.o: The rule$(OBJECT_DIR)/.../version.o : $(SRC)only triggered recompilation when a source file changed. A dirty-tree build (REVISION=uncommitted_YYYYMMDD) followed by a clean-tree build with no source changes left a stale object — firmware embeddeduncommitted_...while the filename correctly showed the commit hash. Fixed by replacing$(SRC)with aFORCEphony target soversion.oalways recompiles (matching the comment's stated intent).Non-deterministic hash length:
git log --format="%h"auto-detects the minimum unique hash length and grows as the repo gets larger (currently 10 chars).GIT_SHORT_REVISION_LENGTHis defined as 7, so the MSP protocol was silently dropping 3 chars. Changed togit rev-parse --short=7 HEADto pin exactly 7 chars — matching the defined constant and keeping source, filename, and protocol wire format consistent.Test plan
..._Build_d649d0a.hex)uncommittedmakefollowed bymake(no changes) still produces correct hash (FORCE rebuildsversion.oevery time)🤖 Generated with Claude Code
Summary by CodeRabbit