Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14494 )
Change subject: IMPALA-3926: part 2: avoid setting LD_LIBRARY_PATH ...................................................................... Patch Set 16: (4 comments) This will cause a bit of annoyance when running binaries directly, but it is a step in the right direction. http://gerrit.cloudera.org:8080/#/c/14494/16//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14494/16//COMMIT_MSG@27 PS16, Line 27: An alternative solution would be to add rpaths to : all binaries we build. I decided to avoid this : because we don't necessarily want random rpaths : from our dev environment to be present in output : artifacts. If there was a way to get the rpath stuff to work for binaries that don't escape the developer environment (gtests, benchmarks), that would be nice. I definitely agree that we don't want to do the rpath fixup for impalad or any binary we output. http://gerrit.cloudera.org:8080/#/c/14494/16//COMMIT_MSG@52 PS16, Line 52: * Manually tested that my dev env with : LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu continued : to work (for now). All ubuntu 16.04 and 18.04 dev : envs that were set up with bootstrap_development.sh : will be in this state. With this change, can developers remove this setting? i.e. this code: https://github.com/apache/impala/blob/master/bin/bootstrap_system.sh#L332-L338 Should we remove it in this change or a separate change? The context is that when we build binaries with gcc7 on ubuntu16, they are using a std lib that is newer than the system std lib. We won't want to have the system std lib on the LD_LIBRARY_PATH. http://gerrit.cloudera.org:8080/#/c/14494/16/bin/gen-backend-test-script.py File bin/gen-backend-test-script.py: http://gerrit.cloudera.org:8080/#/c/14494/16/bin/gen-backend-test-script.py@38 PS16, Line 38: ${IMPALA_HOME}/bin/run-binary.sh \ The backend tests often require the JVM and classpath, so I think we could use the JVM version here. I like to run these directly rather than through ctest. http://gerrit.cloudera.org:8080/#/c/14494/16/bin/impala-python-common.sh File bin/impala-python-common.sh: http://gerrit.cloudera.org:8080/#/c/14494/16/bin/impala-python-common.sh@27 PS16, Line 27: export LD_LIBRARY_PATH="$(python "$IMPALA_HOME/infra/python/bootstrap_virtualenv.py" \ Do we care about respecting a user's existing LD_LIBRARY_PATH here? -- To view, visit http://gerrit.cloudera.org:8080/14494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I61c83e6cca6debb87a12135e58ee501244bc9603 Gerrit-Change-Number: 14494 Gerrit-PatchSet: 16 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 27 Apr 2020 21:15:54 +0000 Gerrit-HasComments: Yes
