Adar Dembo has posted comments on this change. Change subject: Specify full RUNPATH for thirdparty libs for compat with GNU gold ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/2123/1//COMMIT_MSG Commit Message: Line 6: : Specify full RUNPATH for thirdparty libs for compat with GNU gold I find this commit message confusing when compared to the actual code. As Todd discovered, -rpath in LDFLAGS yields different behavior depending on whether ld or gold is used: the former generates DT_RPATH entries in the linked binary while the latter generates DT_RUNPATH. The code change you're making is to -rpath, which means it could yield either new DT_RPATH or DT_RUNPATH entries, depending on which linker is used. So it's weird to see references to "RUNPATH" in the commit description when the tag being added could be either. Further, the commit message doesn't do a great job of fully articulating the problem. See Todd's e-mail. You should also mention the commit hash that changed the behavior (and why it caused the change). And you should explain why your -rpath additions fix the problem: if the linker emitted DT_RUNPATH instead of DT_RPATH, the runtime linker will look for glog's dependencies (namely, gflags) in glog's DT_RUNPATH, and previously, glog didn't have a DT_RUNPATH at all. http://gerrit.cloudera.org:8080/#/c/2123/1/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: Line 158: EXTRA_LDFLAGS="-Wl,-rpath,$PREFIX/lib $EXTRA_LDFLAGS" Please add a comment here explaining why we're setting -rpath. It would be great if you could write it in such a way that it's clear that is also applies to the three other -rpath additions further below. -- To view, visit http://gerrit.cloudera.org:8080/2123 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic50b3667f96cec73497248751e9afc785e3f7979 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
