fix(nixos): auto-detect matic host - #2100
Conversation
|
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces automatic host detection for the "matic" host (a Framework 13 AMD Ryzen AI 300 laptop) by reading DMI system vendor and product name values in the Makefile, removing the need to explicitly pass HOST=matic during builds and switches. It also updates the documentation and adds corresponding tests. The feedback suggests using printf instead of echo when processing DMI_PRODUCT_NAME to avoid unexpected behavior with arbitrary system strings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| elif [ "$(DMI_SYS_VENDOR)" = "Framework" ] \ | ||
| && echo "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then \ | ||
| echo "matic"; \ |
There was a problem hiding this comment.
Using echo to print variables that can contain arbitrary system or user input (such as DMI_PRODUCT_NAME) can lead to unexpected behavior if the value starts with a hyphen (interpreted as an option like -n) or contains backslashes. It is safer and more robust to use printf '%s\n' instead of echo.
elif [ "$(DMI_SYS_VENDOR)" = "Framework" ] \
&& printf '%s\n' "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then \
echo "matic"; \
| elif [ "$$hostname" = "matic" ]; then \ | ||
| echo "matic"; \ | ||
| elif [ "$(DMI_SYS_VENDOR)" = "Framework" ] \ | ||
| && echo "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then \ |
There was a problem hiding this comment.
DMI product-name match is a moving target: grep -q "Laptop 13.*AMD Ryzen AI 300" matches the current Framework AMD 300 DMI string (Laptop 13 (AMD Ryzen AI 300 Series)), but a future Framework BIOS or SKU rename would silently disable auto-detection — make build would still work with HOST=matic, but the auto-detect UX advertised in the README would quietly regress. Consider pinning the exact expected value (or leaving a comment pointing at the source of truth in nixos-hardware) so a future mismatch surfaces as an obvious diff rather than a silent behavior change.
| elif [ "$$hostname" = "matic" ]; then \ | ||
| echo "matic"; \ | ||
| elif [ "$(DMI_SYS_VENDOR)" = "Framework" ] \ | ||
| && echo "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then \ |
There was a problem hiding this comment.
Prefer printf over echo for arbitrary values: echo builtin behavior differs across shells when the argument starts with -n/-e or contains backslashes. DMI_PRODUCT_NAME is kernel-provided so this is unlikely in practice, but printf '%s\n' "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300" is the standard, portable form and matches the rest of the shell in this Makefile.
| && echo "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then \ | |
| elif [ "$(DMI_SYS_VENDOR)" = "Framework" ] \ | |
| && printf '%s\n' "$(DMI_PRODUCT_NAME)" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then \ |
Mesa DescriptionTL;DRAutomatically detects Framework Laptop 13 (AMD Ryzen AI 300) as the What changed?
Description generated by Mesa. Update settings |
The CI branches of nix-build and nix-switch hardcoded the 'runner' host for nixosConfigurations, ignoring both HOST and DETECTED_HOST. Framework 13 DMI detection added in #2100 therefore never took effect under CI, failing the make_build_host_resolution shellspec cases on main. Both branches now resolve HOST, then DETECTED_HOST, and fall back to the existing runner path when neither is set.
Summary
maticfrom DMI data when its current hostname is still genericnix-switchfully generic and feed the detected host through the existing host-resolution pathmake buildandmake switchfor matic while preservingHOST=maticas an overrideValidation
shellspec spec/make_build_host_resolution_spec.sh(4 examples, 0 failures)make shell-test(1,635 ShellSpec examples and 414 fish tests, 0 failures)git diff --checkNote
make nix-format-checkcould not reach the host Nix daemon from the Codex sandbox. No Nix files are changed by this PR.Summary by cubic
Auto-detect the
matichost on Framework Laptop 13 (AMD Ryzen AI 300 Series) using DMI so machines with a generic hostname pick the right config.make buildandmake switchnow work withoutHOST=matic; the generic host-resolution andnix-switchflow is unchanged, and the override still works.Written for commit afe22d8. Summary will update on new commits.