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

Stop overriding CMake's release compile flags. #11467

Merged
merged 1 commit into from Jun 1, 2021

Conversation

chandlerc
Copy link
Contributor

@chandlerc chandlerc commented May 31, 2021

These flags include basic compiler optimization flags without which software can get built completely unoptimized.

While the shims appear to add their own optimization flag on macOS (at least, based on my ARM testing), they do not in the default Linux build which results in CMake packages like LLVM being built without optimizations. This both results in a very large bottle and exceedingly bad performance.

This seems like the best fix to me, as the shims already seem to have the necessary logic to re-map optimization flag levels to what Homebrew prefers for platforms like macOS, and so leaving CMake alone to just do its thing seems like the cleanest solution. But I'm open to other suggested fixes if needed.

I will note that without this, LLVM and any other CMake-built software on Linux is ... mostly unusable for me.

Fixes #11455


  • 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?

These flags include basic compiler optimization flags without which software can get built completely unoptimized.

While the shims appear to add their own optimization flag on macOS (at least, based on my ARM testing), they do not in the default Linux build which results in CMake packages like LLVM being built without optimizations. This both results in a very large bottle and exceedingly bad performance.

This seems like the best fix to me, as the shims already seem to have the necessary logic to *re-map* optimization flag levels to what Homebrew prefers for platforms like macOS, and so leaving CMake alone to just do its thing seems like the cleanest solution. But I'm open to other suggested fixes if needed.

I will note that without this, LLVM and any other CMake-built software on Linux is ... mostly unusable for me.
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

I think this is a good change to make, but I'll leave this open a bit longer in case someone else wants to chime in with an opinion.

@chandlerc
Copy link
Contributor Author

I think this is a good change to make, but I'll leave this open a bit longer in case someone else wants to chime in with an opinion.

Thanks for approving.

I'm (perhaps unsurprisingly) very eager to land, but of course merge whenever you all are comfortable. Let me know if I can do anything else.

@carlocab
Copy link
Member

carlocab commented Jun 1, 2021

Let's give this a go. Thanks, @chandlerc!

@carlocab carlocab merged commit ed922bf into Homebrew:master Jun 1, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 2, 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.

Linuxbrew doesn't build an optimized LLVM (or potentially other CMake packages)
2 participants