Skip to content

Conversation

@cho-m
Copy link
Member

@cho-m cho-m commented Nov 21, 2025

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

system_library_paths is used by refurbished_args to detect duplicate/unneeded -L<path> flags; however, the shim does comparisons to canonical_path(<path>). This means an SDK path like /Library/Developer/CommandLineTools/SDKs/MacOSX26.sdk won't work as it is a symlink to MacOSX26.0.sdk

The old behavior can cause issues if build is run with -L to SDK path as it can find wrong libraries to be linked. The usage of SDK path is often an issue elsewhere (e.g. seen in PHP formula) but this PR allows the shim to handle particular scenario better.


The alternative is workarounds like https://github.com/Homebrew/homebrew-core/blob/a41bd1a9053482dcdce96ab83b7f3e81f03d6ec3/Formula/p/php%408.4.rb#L133-L135

      # PHP build system incorrectly links system libraries: https://github.com/php/php-src/issues/10680
      # Homebrew's superenv can only discard these if using realpath of SDK
      ENV["HOMEBREW_SDKROOT"] = sdk_path.realpath

system_library_paths is used by refurbished_args to detect
duplicate/unneeded `-L<path>` flags; however, the shim does
comparisons to `canonical_path(<path>)`. This means an SDK path like
`/Library/Developer/CommandLineTools/SDKs/MacOSX26.sdk` won't work as
it is a symlink to MacOSX26.0.sdk

This then can cause issues if build is run with `-L` to SDK path as
it can cause the wrong libraries to be linked. This is often an issue
elsewhere (e.g. seen in PHP) but this PR allows the shim to handle this
case better.
Copilot AI review requested due to automatic review settings November 21, 2025 20:36
Copy link
Contributor

Copilot AI left a comment

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 an issue where SDK symlinks (e.g., MacOSX26.sdkMacOSX26.0.sdk) prevent the superenv shim from properly detecting and removing duplicate -L<path> flags during argument refurbishment. The fix applies canonical_path to resolve symlinks in the sysroot library path, ensuring consistent path comparisons.

  • Applies canonical_path() to "#{sysroot}/usr/lib" in system_library_paths method
  • Ensures SDK library paths are canonicalized for proper deduplication in refurbished_args
  • Eliminates the need for workarounds in formula files that manually set HOMEBREW_SDKROOT to the realpath

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

@botantony botantony added this pull request to the merge queue Nov 22, 2025
Merged via the queue into main with commit 922fd1d Nov 22, 2025
41 checks passed
@botantony botantony deleted the sysroot-canonical-path branch November 22, 2025 23:55
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

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.

5 participants