[Fix] Detect homebrew directory

[ninjaguardian]

No longer assumes homebrew is at /opt/homebrew and attempts to find its prefix. Resorts to the default install directory (/opt/homebrew) if it cannot be found or an error occurs.

[coderabbitai]

📝 Walkthrough

Walkthrough

Dynamic Homebrew prefix detection is added to both the Darwin Python build config and the Meson build. Each attempts to run brew --prefix and falls back to a default when unavailable, replacing previously hardcoded include/lib paths with those derived from the detected prefix.

Changes

Cohort / File(s)	Change Summary
Darwin build config (Python) buildconfig/config_darwin.py	Imports subprocess helpers; runs brew --prefix to compute homebrew_prefix with fallback; replaces hardcoded Homebrew include/lib dirs with {homebrew_prefix}/include and {homebrew_prefix}/lib; preserves other existing include/lib paths.
Meson build system meson.build	On non-Windows, attempts to locate brew and invoke brew --prefix; updates homebrew_prefix if command succeeds and path exists, otherwise falls back to /opt/homebrew; bases list updated to use discovered prefix for include/lib discovery.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant Meson as meson.build
  participant System as shell
  Note over Meson: Configure (non-Windows)
  Dev->>Meson: meson setup
  Meson->>System: which brew
  alt brew found
    Meson->>System: brew --prefix
    alt success & valid path
      System-->>Meson: prefix path
      Note right of Meson: set homebrew_prefix to returned path
    else failure
      Note right of Meson: keep default /opt/homebrew
    end
  else brew not found
    Note right of Meson: keep default /opt/homebrew
  end
  Meson->>Meson: populate bases with include/lib using homebrew_prefix

Loading

sequenceDiagram
  autonumber
  actor Build as Build process
  participant Config as buildconfig/config_darwin.py
  participant System as shell
  Build->>Config: import/execute config
  Config->>System: brew --prefix
  alt success & valid path
    System-->>Config: prefix path
    Config->>Config: set incdirs/libdirs = {prefix}/include,{prefix}/lib
  else failure
    Note right of Config: use existing fallbacks (non-Homebrew/SDL2 paths)
  end
  Build-->>Build: use resolved dirs

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title concisely summarizes the primary purpose of the changes by indicating that the pull request fixes Homebrew directory detection, which aligns with the modifications made to dynamically detect the Homebrew prefix rather than using a static path.
Description Check	✅ Passed	The description clearly outlines the core change of dynamically detecting the Homebrew prefix with a fallback to the default path, which directly corresponds to the modifications in the build configuration files.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f15290f and d9ea8bb.

📒 Files selected for processing (1)

• meson.build (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• meson.build

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

meson.build (1)

285-294: LGTM! Consider adding optional validation for robustness.

The dynamic Homebrew prefix detection logic is sound. The fallback to /opt/homebrew and the use of check: false with returncode verification ensure graceful handling when brew is unavailable or fails.

For added robustness, consider validating that the detected prefix is a valid directory path:

     if brew_cmd.found()
         result = run_command(brew_cmd, '--prefix', check: false)
         if result.returncode() == 0
-            homebrew_prefix = result.stdout().strip()
+            detected_prefix = result.stdout().strip()
+            if detected_prefix != '' and fs.is_dir(detected_prefix)
+                homebrew_prefix = detected_prefix
+            endif
         endif
     endif

This prevents edge cases where brew --prefix might return unexpected output, though the existing fs.exists() checks at lines 304 and 313 already provide safety by skipping invalid paths.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a73c345 and f15290f.

📒 Files selected for processing (2)

• buildconfig/config_darwin.py (2 hunks)
• meson.build (1 hunks)

🧰 Additional context used

🪛 Flake8 (7.3.0)

buildconfig/config_darwin.py

[error] 151-151: continuation line under-indented for hanging indent

(E121)

---

[error] 151-151: block comment should start with '# '

(E265)

---

[error] 155-155: block comment should start with '# '

(E265)

🪛 Ruff (0.14.0)

buildconfig/config_darwin.py

143-143: Starting a process with a partial executable path

(S607)

🔇 Additional comments (4)

buildconfig/config_darwin.py (3)

5-5: LGTM!

The subprocess imports are necessary for dynamic Homebrew prefix detection and are used appropriately in the code below.

---

140-146: LGTM!

The dynamic Homebrew prefix detection is implemented correctly:

• Gracefully handles both missing brew command and execution failures
• Appropriate use of DEVNULL to suppress error output
• Falls back to the conventional /opt/homebrew default

Note: The static analysis S607 warning about using a partial executable path is a false positive. Invoking brew from PATH is the standard and correct approach.

---

147-148: LGTM!

The detected Homebrew prefix is correctly integrated into include and library search paths, while preserving existing fallback paths for robustness.

Also applies to: 156-156

meson.build (1)

296-296: LGTM! Clean integration of the detected prefix.

The bases list now correctly uses the dynamically detected homebrew_prefix instead of the hardcoded value, enabling builds with Homebrew installed at non-standard locations.

[oddbookworm]

Thanks for the PR! 🎉
There is one change I see offhand that I would like to see

[ankith26]

LGTM, thanks for contributing! 🎉

I would have approved this PR even you had left out the buildconfig/config_darwin.py changes. But now that you've already implemented it, it's fine to have it too.

I don't have a mac to test out the changes but the code seems fine, and I'm assuming you already tested it out on a mac.

[oddbookworm]

I'll accept @ankith26's that the changes in config_darwin.py don't need further modification. Thanks for the contribution! 🎉