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

Fix LD_LIBRARY_PATH update #8471

Merged
merged 1 commit into from
Jan 8, 2024
Merged

Conversation

nvdias0
Copy link
Contributor

@nvdias0 nvdias0 commented Jan 2, 2024

After [065d3f5], LD_LIBRARY_PATH is not updated, even when there are *.so inside $addon/lib dirs.

Changing LD_LIBRARY_PATH in a set of code between parenthesis will keep the change only inside that scope.
I propose replacing the parenthesis as follows.

After  [065d3f5], LD_LIBRARY_PATH is not updated, even when there are *.so inside $addon/lib dirs.

Looking into the code: changing LD_LIBRARY_PATH in a set of code between parenthesis will keep the change only inside that scope.
I propose replacing the parenthesis.
@vpeter4
Copy link
Contributor

vpeter4 commented Jan 2, 2024

I would go with this (no need to save all the files in variable because they don't matter).
Also maxdepth parameter is global and must be used early.

[ -d "${addon}" ] && file="$(find ${addon} -maxdepth 1 ! -type d -name '*.so*' -print -quit)" && [ -n "$file" ] && LD_LIBRARY_PATH="$LD_LIBRARY_PATH:${addon}"
or
[ -d "${addon}" ] && file="$(find -L ${addon} -maxdepth 1 -type f -name '*.so*' -print -quit)" && [ -n "$file" ] && LD_LIBRARY_PATH="$LD_LIBRARY_PATH:${addon}"

@nvdias0
Copy link
Contributor Author

nvdias0 commented Jan 4, 2024

I would go with this (no need to save all the files in variable because they don't matter). Also maxdepth parameter is global and must be used early.

[ -d "${addon}" ] && file="$(find ${addon} -maxdepth 1 ! -type d -name '*.so*' -print -quit)" && [ -n "$file" ] && LD_LIBRARY_PATH="$LD_LIBRARY_PATH:${addon}" or [ -d "${addon}" ] && file="$(find -L ${addon} -maxdepth 1 -type f -name '*.so*' -print -quit)" && [ -n "$file" ] && LD_LIBRARY_PATH="$LD_LIBRARY_PATH:${addon}"

I've proposed this PR to solve the EXPORT not being properly applied, making a simple syntax adjustment by exchanging the "(...)" for a "&&".
But the changes you proposed are fine with me.

Copy link
Contributor

@heitbaum heitbaum left a comment

Choose a reason for hiding this comment

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

I have tested and it is now adding the “lib” directories with .so* files.

@heitbaum
Copy link
Contributor

heitbaum commented Jan 5, 2024

@nvdias0 please revert the commit you just pushed. No need to sync this PR up to master.

@nvdias0 nvdias0 closed this Jan 5, 2024
@nvdias0 nvdias0 deleted the LD_LIBRARY_PATH_FIX branch January 5, 2024 17:07
@nvdias0 nvdias0 restored the LD_LIBRARY_PATH_FIX branch January 5, 2024 17:08
@nvdias0
Copy link
Contributor Author

nvdias0 commented Jan 5, 2024

@nvdias0 please revert the commit you just pushed. No need to sync this PR up to master.

I'm sorry, but I don't understand. And I believe I've just deleted the wrong branch.

@nvdias0 nvdias0 reopened this Jan 5, 2024
@heitbaum
Copy link
Contributor

heitbaum commented Jan 5, 2024

@nvdias0 please revert the commit you just pushed. No need to sync this PR up to master.

I'm sorry, but I don't understand. And I believe I've just deleted the wrong branch.

There are 2 commits in this branch, the second on e “Merge …” occurred when a sync to master was done. This is the commit that you will need to drop.
IMG_0219

@timhae
Copy link

timhae commented Jan 6, 2024

@nvdias0 please revert the commit you just pushed. No need to sync this PR up to master.

I'm sorry, but I don't understand. And I believe I've just deleted the wrong branch.

checkout your branch, git reset --hard HEAD~1 and git push -f

@nvdias0
Copy link
Contributor Author

nvdias0 commented Jan 8, 2024

@nvdias0 please revert the commit you just pushed. No need to sync this PR up to master.
checkout your branch, git reset --hard HEAD~1 and git push -f

Done.
Thank you.

@heitbaum heitbaum merged commit f9912ab into LibreELEC:master Jan 8, 2024
@nvdias0
Copy link
Contributor Author

nvdias0 commented Jan 8, 2024

Sorry for the "noobish" question - is my 1st PR into LibreELEC.

I understand I can press the delete button. Right ?
image

And even delete my fork ?

thanks

@heitbaum
Copy link
Contributor

heitbaum commented Jan 8, 2024

Sorry for the "noobish" question - is my 1st PR into LibreELEC.

I understand I can press the delete button. Right ? image

And even delete my fork ?

thanks

Correct on both.

@nvdias0 nvdias0 deleted the LD_LIBRARY_PATH_FIX branch January 8, 2024 10:27
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.

6 participants