refactor(makefile): extract host detection into a script #2107
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,60 @@ | ||||||||||
| #!/usr/bin/env bash | ||||||||||
| # Resolve the named host configuration for the current machine. | ||||||||||
| # | ||||||||||
| # Prints the host name on stdout, or nothing when the machine does not map to | ||||||||||
| # a named host. Reads its inputs from the environment so callers (and specs) | ||||||||||
| # can drive detection without touching the real machine: | ||||||||||
| # | ||||||||||
| # OS, ARCH uname-derived platform | ||||||||||
| # DMI_SYS_VENDOR, DMI_PRODUCT_NAME /sys/class/dmi/id values on Linux | ||||||||||
| # RUNPOD_POD_ID set inside RunPod containers | ||||||||||
|
|
||||||||||
| set -euo pipefail | ||||||||||
|
|
||||||||||
| OS="${OS:-}" | ||||||||||
| ARCH="${ARCH:-}" | ||||||||||
| DMI_SYS_VENDOR="${DMI_SYS_VENDOR:-}" | ||||||||||
| DMI_PRODUCT_NAME="${DMI_PRODUCT_NAME:-}" | ||||||||||
| RUNPOD_POD_ID="${RUNPOD_POD_ID:-}" | ||||||||||
|
|
||||||||||
| detect_darwin_host() { | ||||||||||
| if [ "$(whoami 2>/dev/null || true)" != "shunkakinoki" ] || [ "$ARCH" != "arm64" ]; then | ||||||||||
| return 0 | ||||||||||
| fi | ||||||||||
|
|
||||||||||
| local computer_name | ||||||||||
| computer_name="$(scutil --get ComputerName 2>/dev/null || true)" | ||||||||||
| if echo "$computer_name" | grep -q "Shun's MacBook M4"; then | ||||||||||
|
Contributor
There was a problem hiding this comment. Using
Suggested change
|
||||||||||
| echo "galactica" | ||||||||||
| fi | ||||||||||
| return 0 | ||||||||||
| } | ||||||||||
|
Comment on lines
+20
to
+31
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Make the machine probes injectable for deterministic detection tests. The script still reads Accept environment overrides for these values, falling back to the commands only when the overrides are unset; then explicitly neutralize/set them in Also applies to: 39-46 🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| detect_linux_host() { | ||||||||||
| if [ -n "$RUNPOD_POD_ID" ]; then | ||||||||||
| echo "pod" | ||||||||||
| return 0 | ||||||||||
| fi | ||||||||||
|
|
||||||||||
| local hostname | ||||||||||
| hostname="$(hostname 2>/dev/null || true)" | ||||||||||
| case "$hostname" in | ||||||||||
| kyber | matic) | ||||||||||
| echo "$hostname" | ||||||||||
| return 0 | ||||||||||
| ;; | ||||||||||
| esac | ||||||||||
|
|
||||||||||
| # Framework 13 (AMD Ryzen AI 300) ships with a generic hostname, so fall back | ||||||||||
| # to the DMI identity burned into the board. | ||||||||||
| if [ "$DMI_SYS_VENDOR" = "Framework" ] && | ||||||||||
| echo "$DMI_PRODUCT_NAME" | grep -q "Laptop 13.*AMD Ryzen AI 300"; then | ||||||||||
|
Comment on lines
+50
to
+51
Contributor
There was a problem hiding this comment. Using
Suggested change
|
||||||||||
| echo "matic" | ||||||||||
| fi | ||||||||||
| return 0 | ||||||||||
| } | ||||||||||
|
|
||||||||||
| case "$OS" in | ||||||||||
| Darwin) detect_darwin_host ;; | ||||||||||
| Linux) detect_linux_host ;; | ||||||||||
| esac | ||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| #!/usr/bin/env bash | ||
| # shellcheck disable=SC2329 | ||
|
|
||
| Describe 'scripts/detect-host.sh' | ||
| SCRIPT="$PWD/scripts/detect-host.sh" | ||
|
|
||
| Describe 'script properties' | ||
| It 'uses bash shebang' | ||
| When run bash -c "head -1 '$SCRIPT'" | ||
| The output should include '#!/usr/bin/env bash' | ||
| End | ||
|
|
||
| It 'uses strict mode' | ||
| When run bash -c "grep 'set -euo pipefail' '$SCRIPT'" | ||
| The output should include 'set -euo pipefail' | ||
| End | ||
| End | ||
|
|
||
| Describe 'Linux detection' | ||
| It 'maps a Framework 13 AMD Ryzen AI 300 board to matic' | ||
| When run env OS=Linux ARCH=x86_64 DMI_SYS_VENDOR=Framework \ | ||
| DMI_PRODUCT_NAME='Laptop 13 (AMD Ryzen AI 300 Series)' \ | ||
| RUNPOD_POD_ID= bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal 'matic' | ||
| End | ||
|
|
||
| It 'prefers a RunPod pod over DMI data' | ||
| When run env OS=Linux ARCH=x86_64 DMI_SYS_VENDOR=Framework \ | ||
| DMI_PRODUCT_NAME='Laptop 13 (AMD Ryzen AI 300 Series)' \ | ||
| RUNPOD_POD_ID=abc123 bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal 'pod' | ||
| End | ||
|
|
||
| It 'ignores a Framework board that is not the AMD Ryzen AI 300 model' | ||
| When run env OS=Linux ARCH=x86_64 DMI_SYS_VENDOR=Framework \ | ||
| DMI_PRODUCT_NAME='Laptop 13 (12th Gen Intel Core)' \ | ||
| RUNPOD_POD_ID= bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal '' | ||
| End | ||
|
|
||
| It 'ignores matching product data from another vendor' | ||
| When run env OS=Linux ARCH=x86_64 DMI_SYS_VENDOR=Acme \ | ||
| DMI_PRODUCT_NAME='Laptop 13 (AMD Ryzen AI 300 Series)' \ | ||
| RUNPOD_POD_ID= bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal '' | ||
| End | ||
|
|
||
| It 'reports no host when there is no DMI data' | ||
| When run env OS=Linux ARCH=x86_64 DMI_SYS_VENDOR= DMI_PRODUCT_NAME= \ | ||
| RUNPOD_POD_ID= bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal '' | ||
| End | ||
| End | ||
|
|
||
| Describe 'Darwin detection' | ||
| It 'reports no host on a non-arm64 Mac' | ||
| When run env OS=Darwin ARCH=x86_64 bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal '' | ||
| End | ||
| End | ||
|
|
||
| Describe 'unknown platforms' | ||
| It 'reports no host' | ||
| When run env OS=Plan9 ARCH=x86_64 bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal '' | ||
| End | ||
|
|
||
| It 'reports no host when OS is unset' | ||
| When run env -u OS ARCH=x86_64 bash "$SCRIPT" | ||
| The status should be success | ||
| The output should equal '' | ||
| End | ||
| End | ||
| End |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing variables inline to
$(shell ...)using single quotes can break the shell syntax if any of the variables (especiallyDMI_PRODUCT_NAMEorDMI_SYS_VENDORwhich are read from system files) contain single quotes (e.g.,Shun's Laptop).Instead of passing them inline, you can use the
exportdirective in the Makefile. This makes the variables automatically available in the environment of the$(shell ...)subshell, eliminating any quoting or escaping issues entirely.