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.

Sign up for GitHub

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

Jump to bottom

Don't change LD_LIBRARY_PATH when running as an AppImage #8056

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Don't change LD_LIBRARY_PATH when running as an AppImage #8056

merged 1 commit into from
Jan 9, 2025

Conversation

jandubois
Copy link
Member

No description provided.

@jandubois jandubois requested a review from mook-as January 9, 2025 01:08
@jandubois
Copy link
Member Author

Worked for me on Rocky 9.5 (after deleting libmount.so.1 from the zip file first:

[suse@localhost Downloads]$ ./rancher-desktop-9.main.1736353955.944c6913-Build2894.1.glibc2.34-x86_64.AppImage 
/bin/sh: line 1: p11tool: command not found
[41704:0108/170507.425218:ERROR:connection.cc(711)] Cannot send request of length 18215856
/bin/sh: line 1: p11tool: command not found

I believe all these messages are expected.

@jandubois jandubois marked this pull request as ready for review January 9, 2025 01:10
mook-as
mook-as requested changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

Please add a comment on why we need to skip this for AppImage.

We should also extend LD_LIBRARY_PATH instead of clobbering it, in case the user has other reasons to set it.

pkg/rancher-desktop/backend/lima.ts Outdated Show resolved Hide resolved
@jandubois jandubois force-pushed the ld-library-path branch 2 times, most recently from ea635ec to 0ee17de Compare January 9, 2025 18:13
@jandubois jandubois requested a review from mook-as January 9, 2025 18:38
mook-as
mook-as previously approved these changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

I don't know why you prefer env['X'] over env.X, but whatever.

pkg/rancher-desktop/backend/lima.ts Outdated Show resolved Hide resolved
mook-as
mook-as previously approved these changes Jan 9, 2025
View reviewed changes
@jandubois
Don't change LD_LIBRARY_PATH when running as an AppImage
0c348d0 
Signed-off-by: Jan Dubois <jan.dubois@suse.com>
@jandubois
Copy link
Member Author

I don't know why you prefer env['X'] over env.X, but whatever.

I don't actually, but somehow my brain is not compatible with JavaScript...

@jandubois jandubois merged commit 2503b4b into rancher-sandbox:main Jan 9, 2025
20 checks passed
@jandubois jandubois mentioned this pull request Jan 9, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mook-as mook-as mook-as approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jandubois @mook-as