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 4 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 38 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy 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
olzemal
left a comment
olzemal
left a comment
There was a problem hiding this comment.
some small nitpicks but lgtm otherwise
3 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