Skip to content

shims/super/cc: allow paths under HOMEBREW_FORMULA_BUILDPATH - #23971

Merged
woodruffw merged 1 commit into
mainfrom
ww/fix-buildpath
Sep 15, 2026
Merged

woodruffw merged 1 commit into
mainfrom
ww/fix-buildpath

Conversation

@woodruffw

Copy link
Copy Markdown
Member

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

I used GPT Astra (Max) to help me root-cause this bug and iterate on the patch.


This is a fix for the build failures in Homebrew/homebrew-core#311045.

TL;DR: #23940 changes the temporary directory (HOMEBREW_TEMP) used for builds from the common temporary root to a sandbox-specific one. Specifically, this line:

https://github.com/Homebrew/brew/pull/23940/changes#diff-7eeef6e1b769c423e63e92e764feea0fc54944018bcec8c108d9a19eb550c118R584

This then interacted unintentionally with two pre-existing conditions:

  1. Rust bottle builds stage outside of the sandbox-specific HOMEBREW_TEMP;
  2. superenv's cc wrapper only used the sandbox-adjusted HOMEBREW_TEMP to determine which paths to retain, e.g. for -Iinclude arguments.

(2) in particular meant that, even though the sandbox had access to the staged build directory, superenv would incorrectly strip any -Iinclude that referenced it.

@woodruffw
woodruffw requested a review from p-linnane September 15, 2026 04:43
@woodruffw woodruffw self-assigned this Sep 15, 2026
Copilot AI balanced review requested due to automatic review settings September 15, 2026 04:43

@p-linnane p-linnane left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

The bug fix lacks regression coverage in the existing compiler-shim spec.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Allows superenv鈥檚 compiler shim to retain include/library paths within a formula鈥檚 staged build directory.

Changes:

  • Canonicalises HOMEBREW_FORMULA_BUILDPATH.
  • Permits the build path and its descendants in keep?.
File summaries
File Description
Library/Homebrew/shims/super/cc Allows paths beneath the formula build directory.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

馃挕 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread Library/Homebrew/shims/super/cc
@woodruffw
woodruffw added this pull request to the merge queue Sep 15, 2026
Merged via the queue into main with commit 82ea9cb Sep 15, 2026
62 checks passed
@woodruffw
woodruffw deleted the ww/fix-buildpath branch September 15, 2026 05:16

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants