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

Remove old cctools references now that only ruby-macho is used. #1086

Merged
merged 1 commit into from
Sep 23, 2016

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Sep 22, 2016

  • 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 tests with your changes locally?

Continuation of #1051. The big changes here are that Keg#require_install_name_tool? is now Keg#require_relocation? and OS::Mac.{install_name_tool,otool} are removed.

Other notes:

We still check for otool and install_name_tool behavior in extend/os/mac/diagnostic.rb, even though both are currently eliminated entirely from the codebase.

Is it worth keeping checks like check_for_bad_install_name_tool? A cursory search of the core shows that at least 11 formulae invoke it directly, rather than using our ruby implementation.

cc @UniqMartin, @MikeMcQuaid

@MikeMcQuaid
Copy link
Member

Is it worth keeping checks like check_for_bad_install_name_tool

In that case it's worth renaming but adding to our compat/ layer.

@MikeMcQuaid MikeMcQuaid merged commit 9001855 into Homebrew:master Sep 23, 2016
@MikeMcQuaid
Copy link
Member

Thanks again @woodruffw!

@woodruffw
Copy link
Member Author

In that case it's worth renaming but adding to our compat/ layer.

Got it! I'll also look into changing those formula to use ruby-macho instead.

@woodruffw woodruffw deleted the macho-cosmetics branch September 23, 2016 14:59
@UniqMartin
Copy link
Contributor

Nice work here and in #1051! If you feel like leveraging more of ruby-macho's capabilities in Homebrew, you could look into relocating and cleaning up bogus RPATHs. This could also help with making a few more bottles relocatable (cf. #371), though admittedly RPATHs are still not that prevalent among Homebrew formulae, so the impact won't be massive.

And then there was also the idea of adding some form of validation of the install name in Homebrew/homebrew-core#4655 (comment) which also falls into the realm of Mach-O related tweaks in Homebrew, thus something you might want to look into.

@woodruffw
Copy link
Member Author

Thanks!

I'll read those issues and see what I can do about them. Expect PRs in a few days 😄

@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants