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

tests/run_tests.sh: fix Meson library path being dropped #138

Closed
wants to merge 1 commit into from

Conversation

blmaier
Copy link
Contributor

@blmaier blmaier commented Jun 24, 2024

Meson automatically passes in LD_LIBRARY_PATH pointing at the correct build directory for libfdt.so. So preserve LD_LIBRARY_PATH if it's already set.

Meson automatically passes in LD_LIBRARY_PATH pointing at the correct
build directory for libfdt.so. So preserve LD_LIBRARY_PATH if it's
already set.

Signed-off-by: Brandon Maier <[email protected]>
@@ -55,7 +55,11 @@ fi
if [ -z "$TEST_LIBDIR" ]; then
TEST_LIBDIR=../libfdt
fi
export LD_LIBRARY_PATH="$TEST_LIBDIR"
if [ -n "$LD_LIBRARY_PATH" ]; then
export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$TEST_LIBDIR"
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't seem quite right to me. This code right here is attempting to find the library and point LD_LIBRARY_PATH at it. If Meson has already done that, we don't need to do anything: we're kind of doubling up here taking both the Meson modified path and then adding a (wrong) guess at where the library is as well.

I think we'd be better off assuming that if LD_LIBRARY_PATH is set, then the caller knows what they're doing with it and leave it alone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem quite right to me. This code right here is attempting to find the library and point LD_LIBRARY_PATH at it. If Meson has already done that, we don't need to do anything: we're kind of doubling up here taking both the Meson modified path and then adding a (wrong) guess at where the library is as well.

I'm not a fan of this code as it is guessing at where the files are, but it's still needed for the Makefile system.

I think we'd be better off assuming that if LD_LIBRARY_PATH is set, then the caller knows what they're doing with it and leave it alone.

The issue is LD_LIBRARY_PATH could already be defined by the system or user for other libraries. So doing that would cause run_tests to fail to find the dtc library.

Copy link
Owner

Choose a reason for hiding this comment

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

Hrm, I see your point. It's pretty nasty, but I can't quickly see a better way to do it.

@dgibson
Copy link
Owner

dgibson commented Jun 26, 2024

Merged, thanks.

@dgibson dgibson closed this Jun 26, 2024
@blmaier blmaier deleted the run-tests-ld-libary-path branch October 1, 2024 19:46
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.

2 participants