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
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion tests/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.

else
export LD_LIBRARY_PATH="$TEST_LIBDIR"
fi

export QUIET_TEST=1
STOP_ON_FAIL=0
Expand Down
Loading