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

Add formula DSL support for omitting install name rewriting for @rpath/* install names #15354

Open
1 task done
carlocab opened this issue May 3, 2023 · 6 comments
Open
1 task done
Labels
features New features help wanted We want help addressing this

Comments

@carlocab
Copy link
Member

carlocab commented May 3, 2023

Verification

Provide a detailed description of the proposed feature

Currently, we rewrite all library install names to something like opt_prefix/library_install_name.basename.

if file.dylib?
id = relocated_name_for(file.dylib_id, relocation)
change_dylib_id(id, file)
end

Typically, these will be because the install name refers to a Cellar path, e.g., prefix/library_install_name.basename. The install name rewriting allows us to avoid unnecessary rebuilds of dependents with version/revision bumps.

This install name rewriting isn't needed when the install name starts with @rpath, because these will typically not hardcode a Cellar path reference that will need to be rebuilt on version/revision bumps.

Formulae should support some sort of DSL that makes the code in keg_relocate.rb skip rewriting an install name if it starts with @rpath.

What is the motivation for the feature?

Our rewriting of @rpath-prefixed install names breaks macdeployqt. See Homebrew/discussions#2823.

This is arguably an upstream bug, but convincing upstream to fix it is unlikely to succeed without a reproduction of the issue outside of Homebrew, and I don't think anyone has the time or energy to do this. Even if one had such a reproducer, it would likely still be rather difficult to convince upstream to fix this short of providing a patch.

Such a DSL could also replace a very old workaround in Rust:

  def post_install
    Dir["#{lib}/rustlib/**/*.dylib"].each do |dylib|
      chmod 0664, dylib
      MachO::Tools.change_dylib_id(dylib, "@rpath/#{File.basename(dylib)}")
      MachO.codesign!(dylib) if Hardware::CPU.arm?
      chmod 0444, dylib
    end
  end

How will the feature be relevant to at least 90% of Homebrew users?

It probably won't be, but it would likely be relevant to our users who use Qt, which form a non-negligible fraction of our user base.

What alternatives to the feature have been considered?

See alternatives described in this comment.

@carlocab carlocab added help wanted We want help addressing this features New features labels May 3, 2023
@carlocab
Copy link
Member Author

carlocab commented May 3, 2023

Also, to explain why we want an additional DSL rather than just skipping the rewriting of @rpath/* install names entirely:

Typically, these will make linking with libraries harder. For example, if libfoo.dylib has an absolute path for its install name, linking with libfoo only requires

-L$HOMEBREW_PREFIX/lib -lfoo

However, if libfoo has @rpath/libfoo.dylib for its install name, one would need some variation of

-L$HOMEBREW_PREFIX/lib -lfoo -Wl,-rpath,$HOMEBREW_PREFIX/lib

which is likely to trip up many users and result in may issues/discussion posts created.

If we had such a DSL, the above would not be a consideration for Rust (cargo handles linker invocation), and isn't likely to be a significant problem for Qt (since upstream ships libraries/frameworks with install names that start with @rpath in their pre-built binaries).

@MikeMcQuaid
Copy link
Member

Thanks @carlocab. Agree with everything you've said here and the desire for this for both Qt and Rust.

leonlynch added a commit to openemv/dukpt that referenced this issue Dec 30, 2023
* Update MacOS test builds to test both Qt5 and Qt6. However, due to
  Homebrew/brew#15354, macdeployqt is still
  broken for Qt6 on Homebrew, so MacOS release builds will continue to
  use Qt5 instead.
* Update Windows test builds to test Qt5 for i686 and Qt6 for x86_64
* Update Windows release build to use MinGW. This more closely matches
  the Qt builds and partially reverts 7cab5e2.
* Update Windows release build to use Qt6
* Update README to mention CMAKE_PREFIX_PATH because this appears to be
  necessary for Windows release builds that use a standalone Qt
  installation
@ubruhin
Copy link

ubruhin commented Mar 8, 2024

@carlocab May I ask for the progress of this PR? If this fixes the "Cannot resolve rpath" errors described here, it would help us a lot to get LibrePCB migrated to Qt6. Currently this task is blocked by the broken deployment with macdeployqt.

@pawsaw
Copy link

pawsaw commented Aug 1, 2024

What's the status of this?

@MikeMcQuaid
Copy link
Member

@pawsaw We're waiting for someone to implement it.

@Bo98
Copy link
Member

Bo98 commented Oct 25, 2024

Here's a case that doesn't involve @rpath: Swift ships dylibs that point to /usr/lib/swift/libswift* rather than the installed file. This is intentional due to the unique way that Swift ships with the OS but new features can still be back deployed to older OS.

My initial thought was to skip anything that didn't start with HOMEBREW_PREFIX or the build directory. @rpath however is hard and do acknowledge the DSL argument here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
features New features help wanted We want help addressing this
Projects
None yet
Development

No branches or pull requests

6 participants
@MikeMcQuaid @Bo98 @pawsaw @ubruhin @carlocab and others