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

Overriding executable rpath with patchelf #25059

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

mmaslankaprv
Copy link
Member

Overriding Redpanda executable makes it easier to handle shared library loading.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

@mmaslankaprv mmaslankaprv marked this pull request as ready for review February 7, 2025 10:29
@mmaslankaprv mmaslankaprv force-pushed the bazel-patchelf branch 2 times, most recently from b9c472a to 4152c60 Compare February 7, 2025 11:19
Signed-off-by: Michał Maślanka <[email protected]>
By overriding an `executable` rpath we no longer need to care about
setting correct `LB_LIBRARY_PATH`

Signed-off-by: Michał Maślanka <[email protected]>
Copy link
Contributor

@rockwotj rockwotj left a comment

Choose a reason for hiding this comment

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

Amazing thank you 🙏

Just a couple of questions for my understanding

inputs = [original_binary],
outputs = [patched_binary],
executable = ctx.executable._patchelf,
arguments = ["--force-rpath", "--set-rpath", path_override, original_binary.path, "--output", patched_binary.path],
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -17,7 +17,6 @@ go_binary(
visibility = ["//bazel/packaging:__pkg__"],
x_defs = {
"binaryPath": "/opt/redpanda/libexec/redpanda",
"ldLibraryPath": "/opt/redpanda/lib",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need these anymore with this approach? Should we remove the wrappers and just put the real binaries in bin/ instead of libexec/?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants