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

linkers: Fix AppleDynamicLinker not returning any RPATHs to remove #13365

Merged
merged 1 commit into from
Jun 30, 2024

Conversation

pbrzez
Copy link
Contributor

@pbrzez pbrzez commented Jun 28, 2024

Fixes another regression from #13012 - before it, all RPATHs were removed on install, now none of them were :)

That PR relied on rpath_dirs_to_remove being present and correctly filled, which was never the case for the AppleDynamicLinker. The result was that all the build-dir-only RPATHs were being carried over to the installed files. For example, in a local GStreamer build, I was seeing paths like @loader_path/../../../gstreamer/gst present after installation, which were only valid in the builddir and should've been removed.

I implemented returning the list of RPATHs to remove in AppleDynamicLinker, doing pretty much the same thing as what's in the GnuLikeDynamicLinkerMixin. Thanks to that, depfixer now correctly removes build-time Meson-created RPATHs, as it used to before 1.4.1.

I also added a unit test trying to prevent a similar scenario (on macOS only) in the future.

@pbrzez pbrzez requested a review from jpakkane as a code owner June 28, 2024 15:14
@eli-schwartz
Copy link
Member

eli-schwartz commented Jun 28, 2024

Thank you, I'm rather disappointed that that PR apparently never got tested by the person who had macOS hardware and developed the code and was in a position to test it...

@eli-schwartz eli-schwartz added this to the 1.4.2 milestone Jun 28, 2024
@eli-schwartz eli-schwartz force-pushed the darwin-rpath-install-fix branch from 745d428 to 4b0d4e5 Compare June 28, 2024 15:24
@eli-schwartz
Copy link
Member

I reflowed the commit message a bit for better readability in git log:

git range-diff origin/master 745d42825 4b0d4e548

1:  745d42825 ! 1:  4b0d4e548 linkers: Fix AppleDynamicLinker not returning any rpaths to remove
    @@ Metadata
      ## Commit message ##
         linkers: Fix AppleDynamicLinker not returning any rpaths to remove
     
    -    Fixes regression from commit 78e9009.
    +    Fixes regression from commit 78e9009ff9d36925e04f329f9082841002ddd848.
     
    -    The above commit relied on rpath_dirs_to_remove being present and correctly filled, which was never the case for the
    -    AppleDynamicLinker. The result was that all the build-dir-only RPATHs were being carried over to the installed files.
    +    The above commit relied on rpath_dirs_to_remove being present and
    +    correctly filled, which was never the case for the AppleDynamicLinker.
    +    The result was that all the build-dir-only RPATHs were being carried
    +    over to the installed files.
     
    -    This commit implements returning the list of RPATHs to remove in AppleDynamicLinker, doing pretty much the same thing
    -    as what's in the GnuLikeDynamicLinkerMixin. Thanks to that, depfixer now correctly removes build-time Meson-created
    -    RPATHs, as it used to before 1.4.1.
    +    This commit implements returning the list of RPATHs to remove in
    +    AppleDynamicLinker, doing pretty much the same thing as what's in the
    +    GnuLikeDynamicLinkerMixin. Thanks to that, depfixer now correctly
    +    removes build-time Meson-created RPATHs, as it used to before 1.4.1.
     
      ## mesonbuild/linkers/linkers.py ##
     @@ mesonbuild/linkers/linkers.py: class AppleDynamicLinker(PosixDynamicLinkerMixin, DynamicLinker):

@eli-schwartz
Copy link
Member

@grobian

@pbrzez
Copy link
Contributor Author

pbrzez commented Jun 28, 2024

I reflowed the commit message a bit for better readability in git log:

Thanks!

Fixes regression from commit 78e9009.

The above commit relied on rpath_dirs_to_remove being present and
correctly filled, which was never the case for the AppleDynamicLinker.
The result was that all the build-dir-only RPATHs were being carried
over to the installed files.

This commit implements returning the list of RPATHs to remove in
AppleDynamicLinker, doing pretty much the same thing as what's in the
GnuLikeDynamicLinkerMixin. Thanks to that, depfixer now correctly
removes build-time Meson-created RPATHs, as it used to before 1.4.1.
@pbrzez pbrzez force-pushed the darwin-rpath-install-fix branch from 4b0d4e5 to 55b49d0 Compare June 28, 2024 15:34
@pbrzez
Copy link
Contributor Author

pbrzez commented Jun 28, 2024

Sorry about the force-push, had the wrong e-mail set in the commit.

@grobian
Copy link
Contributor

grobian commented Jun 29, 2024

Thank you, I'm rather disappointed that that PR apparently never got tested by the person who had macOS hardware and developed the code and was in a position to test it...

I'm sorry if I caused this. To the best of my knowledge and testing the patch worked.

@eli-schwartz eli-schwartz merged commit dc1b4be into mesonbuild:master Jun 30, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants