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

keg_relocate: use correct number of arguments in call to change_install_name #3219

Merged
merged 1 commit into from Sep 27, 2017

Conversation

Projects
None yet
2 participants
@scpeters
Contributor

scpeters commented Sep 27, 2017

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

During the refactoring of macho file relocation in #3101, #3138, and #3139,
the number of arguments to the mach::change_install_name function changed from 3 to 2, but there was still an instance of the function being called with the wrong number of arguments.

I noticed this while building a high_sierra bottle for osrf/simulation/simbody and received a Error: Failed to fix install linkage in the jenkins console output and saw the following when building with --debug:

==> wrong number of arguments (given 3, expected 2)
/usr/local/Homebrew/Library/Homebrew/os/mac/mach.rb:78:in `change_install_name'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:19:in `block (3 levels) in fix_dynamic_linkage'

more context:

...
Fixing /usr/local/Cellar/simbody/3.5.4_2/lib/simbody/examples/UserGuide permissions from 755 to 555
Fixing /usr/local/Cellar/simbody/3.5.4_2/lib/simbody/examples/UserGuideLimits permissions from 755 to 555
Changing dylib ID of /usr/local/Cellar/simbody/3.5.4_2/lib/libSimTKcommon.3.5.dylib
  from libSimTKcommon.3.5.dylib
    to /usr/local/opt/simbody/lib/libSimTKcommon.3.5.dylib
Changing dylib ID of /usr/local/Cellar/simbody/3.5.4_2/lib/libSimTKmath.3.5.dylib
  from libSimTKmath.3.5.dylib
    to /usr/local/opt/simbody/lib/libSimTKmath.3.5.dylib
Error: Failed to fix install linkage
The formula built, but you may encounter issues using it or linking other
formula against it.
==> wrong number of arguments (given 3, expected 2)
/usr/local/Homebrew/Library/Homebrew/os/mac/mach.rb:78:in `change_install_name'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:19:in `block (3 levels) in fix_dynamic_linkage'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:91:in `each'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:91:in `each_install_name_for'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:10:in `block (2 levels) in fix_dynamic_linkage'
/usr/local/Homebrew/Library/Homebrew/extend/pathname.rb:381:in `ensure_writable'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:4:in `block in fix_dynamic_linkage'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:3:in `each'
/usr/local/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:3:in `fix_dynamic_linkage'
/usr/local/Homebrew/Library/Homebrew/formula_installer.rb:811:in `fix_dynamic_linkage'
/usr/local/Homebrew/Library/Homebrew/formula_installer.rb:589:in `finish'
/usr/local/Homebrew/Library/Homebrew/cmd/install.rb:337:in `install_formula'
/usr/local/Homebrew/Library/Homebrew/cmd/install.rb:226:in `block in install'
/usr/local/Homebrew/Library/Homebrew/cmd/install.rb:224:in `each'
/usr/local/Homebrew/Library/Homebrew/cmd/install.rb:224:in `install'
/usr/local/Homebrew/Library/Homebrew/brew.rb:95:in `<main>'
/usr/local/Homebrew/Library/Homebrew/postinstall.rb (Formulary::FromPathLoader): loading /usr/local/Homebrew/Library/Taps/osrf/homebrew-simulation/simbody.rb

Just removing the file argument allows my formula to build successfully. I think this isn't a controversial change.

keg_relocate: fix call to change_install_name
During the changes to macho file relocation refactoring
in #3101, #3138, and #3139,
the number of arguments to the mach::change_install_name
function changed from 3 to 2, but
there was still an instance of the function being called
with the wrong number of arguments.
@woodruffw

This comment has been minimized.

Show comment
Hide comment
@woodruffw

woodruffw Sep 27, 2017

Member

Yikes. I'm very sorry about that. I'll merge this ASAP.

@ilovezfs, you were right once again.

Member

woodruffw commented Sep 27, 2017

Yikes. I'm very sorry about that. I'll merge this ASAP.

@ilovezfs, you were right once again.

@woodruffw woodruffw merged commit 3a5de99 into Homebrew:master Sep 27, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@scpeters

This comment has been minimized.

Show comment
Hide comment
@scpeters

scpeters Sep 27, 2017

Contributor

Thanks for the quick response!

Contributor

scpeters commented Sep 27, 2017

Thanks for the quick response!

@scpeters scpeters deleted the scpeters:change_install_name_takes_2_args branch Sep 27, 2017

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.