Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not pass -Wl,-headerpad_max_install_names on linux #12408

Merged
merged 1 commit into from Nov 10, 2021

Conversation

evanphx
Copy link

@evanphx evanphx commented Nov 10, 2021

One of the more curious bugs, if you use -Wl,-headerpad_max_install_names on linux, it tries to link a library named eaderpath_max_install_names in, which causes all kinds of weird havoc.

Most notably, gtester inside glib fails to run for bizarre reasons. -Wl,-headerpad_max_install_names is not an option anywhere outside macos anyway, so move it to macos only and avoid the heartache of extremely weird bugs.

  • 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 style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

One of the more curious bugs, if you use
-Wl,-headerpad_max_install_names on linux, it tries to link a library
named "eaderpath_max_install_names" in, which causes all kinds of weird
havoc.

Most notably, gtester inside glib fails to run for bizarre reasons.
-Wl,-headerpad_max_install_names is not an option anywhere outside macos
anyway, so move it to macos only and avoid the heartache of extremely
weild bugs.
@evanphx evanphx changed the title Do not pass -Wl,-headerpad_max_install_names to b-linux-path Do not pass -Wl,-headerpad_max_install_names on linux Nov 10, 2021
Copy link
Member

@Bo98 Bo98 left a comment

Choose a reason for hiding this comment

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

This change makes sense. Though what confuses me is how no one has ever reported the issue you have described. The code has been there for years.

@evanphx
Copy link
Author

evanphx commented Nov 10, 2021

@Bo98 you and me both. My theory is that it's only in std and maybe linux isn't using std by default? I'm using homebrew in a bit more exotic way atm and thusly might have just stumbled into a dusty corner!

@Bo98
Copy link
Member

Bo98 commented Nov 10, 2021

My theory is that it's only in std and maybe linux isn't using std by default?

Yes we only use std by default in formula test blocks (on all platforms - even macOS). Formulae can use env :std to use that environment during the build stage, but that's strongly discouraged and is in fact outright forbidden in homebrew-core. I think this shows how little people use that (though I suppose most taps are macOS only).

I still expected it to have popped up at least once in a test but it seems not.

@carlocab carlocab merged commit ac6196c into Homebrew:master Nov 10, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 11, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants