-
Notifications
You must be signed in to change notification settings - Fork 431
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 distcheck #15
fix distcheck #15
Conversation
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
@@ -74,8 +74,7 @@ list: gtest | |||
|
|||
test_valgrind: uct gtest | |||
. /usr/share/Modules/init/sh && \ | |||
module load dev/mofed_valgrind && \ | |||
valgrind $(VALGRIND_ARGS) $(abs_builddir)/gtest --gtest_filter=$(GTEST_FILTER) | |||
env LD_LIBRARY_PATH=/usr/lib64/mlnx_ofed/valgrind:${LD_LIBRARY_PATH} valgrind $(VALGRIND_ARGS) $(abs_builddir)/gtest --gtest_filter=$(GTEST_FILTER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded path to mofed … no good. It has to be one of configure options. --with-valgrind. The m4 script may check by default few locations, including /bin, /usr/bin and mofed if it was earlier discovered by verbs script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove it: env LD_LIBRARY_PATH=/usr/lib64/mlnx_ofed/valgrind:${LD_LIBRARY_PATH}
mofed now shipped with valgrind compiled libibverbs and friends which can be selected at runtime.
ucx already has "--with-valgrind"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And it is good. My point that it has to be set from valgrind configure and not hardcoded in make. If MOFED is detected, we may automatically update/set valgrind location. should be easy to do.
Merged build started. |
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
@yosefe - could you please fix coverity errors: |
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
add jenkins script which can be executed as ./contrib/test_jenkins.sh detect/add mlnx valgrind libs
940fd41
to
ef63824
Compare
Merged build triggered. |
1 similar comment
Merged build triggered. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
@miked-mellanox I was under immersion that it was already merged. |
refactor mem address domain detection
UCP/TAG: Release RNDV request in case of error
AZP: trigger for push and pr
No description provided.