feat: updated common.mk to include setup and self-update#15
feat: updated common.mk to include setup and self-update#15rebEllieous wants to merge 3 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. 📝 WalkthroughWalkthroughThis PR adds automatic self-update detection and refresh to Changescommon.mk Self-Update Mechanism
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
common.mk (1)
257-261: ⚡ Quick winVerify the hash command on non-GNU developer machines.
sha256sumis not available by default on macOS. Because these calls run during$(shell ...)evaluation underbash -e, a missing binary can abort parsing before the cached fallback path is reached. If Darwin is in the support matrix, add ashasum -a 256oropenssl dgst -sha256fallback here.🤖 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 `@common.mk` around lines 257 - 261, The two sha256sum calls used to compute remote and local_hash (remote=$$(sha256sum .common.mk-download | cut -d' ' -f1); local_hash=$$(sha256sum '$(_COMMON_MK_PATH)' ...)) are not portable to macOS; replace them with a portable hash lookup: detect an available tool (sha256sum || shasum -a 256 || openssl dgst -sha256) into a HASH_CMD variable or small shell helper and normalize its output so the existing cut -d' ' -f1 extraction still works (for example transform shasum or openssl output to "HASH FILENAME" format), then use that HASH_CMD to compute remote and local_hash so parsing under bash -e won't abort on Darwin.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@common.mk`:
- Around line 251-265: When the fetched remote file matches the local hash
(inside the if branch comparing "$$remote" and "$$local_hash"), persist the
current DEV_KIT_VERSION by writing "$(DEV_KIT_VERSION)" into .common.mk-version
in addition to removing .common.mk-download and touching .common.mk-checked;
update the block that handles the hash match so it sets .common.mk-version
(using the same variable as used earlier), so subsequent runs do not treat the
version as stale even when the file content is identical.
- Around line 223-239: The update-common-mk-bootstrap target currently always
reports success even if the awk script never matches the /^common\.mk:$$/
pattern; modify the recipe so it detects whether awk performed a replacement and
fails if not—e.g. have the awk script set an exit status or write a marker when
it rewrites, and then check that marker (or compare Makefile.tmp to Makefile)
before mv; if no change was made, remove Makefile.tmp and exit non-zero so
update-common-mk-bootstrap fails instead of printing "Updated common.mk:
bootstrap in Makefile".
---
Nitpick comments:
In `@common.mk`:
- Around line 257-261: The two sha256sum calls used to compute remote and
local_hash (remote=$$(sha256sum .common.mk-download | cut -d' ' -f1);
local_hash=$$(sha256sum '$(_COMMON_MK_PATH)' ...)) are not portable to macOS;
replace them with a portable hash lookup: detect an available tool (sha256sum ||
shasum -a 256 || openssl dgst -sha256) into a HASH_CMD variable or small shell
helper and normalize its output so the existing cut -d' ' -f1 extraction still
works (for example transform shasum or openssl output to "HASH FILENAME"
format), then use that HASH_CMD to compute remote and local_hash so parsing
under bash -e won't abort on Darwin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
rebEllieous
force-pushed
the
feature/13-update-stale-common.mk
branch
from
f86ee0f to
26067d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@common.mk`:
- Around line 264-265: Replace the hard-coded sha256sum invocation used to
compute remote and local_hash with a portable detection/fallback: detect
available checksum tool (prefer sha256sum, then shasum -a 256, then openssl dgst
-sha256) and use the detected command to compute the digests for remote=$$(...)
and local_hash=$$(...) so the self-update comparison works on macOS and other
systems without sha256sum.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
2 participants
What
Closes #13
Why
common.mk was only re-downloaded when it was missing or explicitly deleted. If dev-kit updated the file, consuming repos would silently continue using a stale version until someone manually removed it. This was especially fragile because the logic had to be duplicated in every consuming repo.
Testing
Manually triggered on solar and: confirmed common.mk is re-fetched when content differs and skipped when content is unchanged
Confirmed the 1h rate-limit works: .common.mk-checked prevents redundant network calls within the window
make update-common-mk-bootstrap ran successfully, rewrote the old two-line recipe in-place
Summary by CodeRabbit
New Features
Chores
Documentation