Skip to content

Null Git global config for local repository inspections#22797

Merged
MikeMcQuaid merged 1 commit into
mainfrom
missing-head-on-install
Jun 18, 2026
Merged

Null Git global config for local repository inspections#22797
MikeMcQuaid merged 1 commit into
mainfrom
missing-head-on-install

Conversation

@carlocab

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 Hyperfine benchmarks.
  • 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?

  • AI was used to generate or assist with generating this PR.

I used Claude to diagnose the bug and write the fix. I reviewed the resulting code and manually ran brew lgtm and also manually checked that the bug has been fixed.


last_commit and current_revision run git rev-parse against the
cached clone while staging inside the build sandbox, before the stage
environment nulls the global Git config. There, reading the user's
global Git config is denied and makes Git exit, so last_commit returns
an empty string. For HEAD builds this makes update_commit("") produce a
bare HEAD- version, the build installs to Cellar/<name>/HEAD-, and
the install then fails with Error: Empty installation once the real
short commit is computed outside the sandbox.

These are local, read-only inspections of an already-cloned repository
that never need credential helpers, so null the global config for them
via a shared local_git_env helper, matching the existing
source_modified_time fix. The download-time commands keep reading the
user config for private-repository credentials.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com


To reproduce, run:

brew install --HEAD neovim

`last_commit` and `current_revision` run `git rev-parse` against the
cached clone while staging inside the build sandbox, before the stage
environment nulls the global Git config. There, reading the user's
global Git config is denied and makes Git exit, so `last_commit` returns
an empty string. For HEAD builds this makes `update_commit("")` produce a
bare `HEAD-` version, the build installs to `Cellar/<name>/HEAD-`, and
the install then fails with `Error: Empty installation` once the real
short commit is computed outside the sandbox.

These are local, read-only inspections of an already-cloned repository
that never need credential helpers, so null the global config for them
via a shared `local_git_env` helper, matching the existing
`source_modified_time` fix. The download-time commands keep reading the
user config for private-repository credentials.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 18, 2026 12:59

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.

Pull request overview

This PR fixes sandboxed staging failures for --HEAD installs by ensuring read-only Git inspections against an already-cached clone do not read the user’s global Git config (which can be denied in the sandbox), preventing empty last_commit/current_revision results and downstream versioning/install path issues.

Changes:

  • Introduces a shared local_git_env helper that merges GIT_TERMINAL_PROMPT=0 with Utils::Git.no_global_config_env.
  • Routes local inspection commands (git ... show / git ... rev-parse) in source_modified_time, last_commit, and current_revision through local_git_env.
  • Updates the Git download strategy spec to assert #last_commit uses the null-global-config env.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Library/Homebrew/download_strategy/git_download_strategy.rb Adds local_git_env and applies it to local Git inspection calls to avoid sandbox global-config reads.
Library/Homebrew/test/download_strategies/git_spec.rb Refactors/extends #last_commit tests to assert the correct env is passed to system_command.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/test/download_strategies/git_spec.rb

@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 for fix here @carlocab and apologies for the regressions 🙈

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Jun 18, 2026
Merged via the queue into main with commit 8208306 Jun 18, 2026
43 checks passed
@MikeMcQuaid MikeMcQuaid deleted the missing-head-on-install branch June 18, 2026 13:59
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.

3 participants