Hello Dan Burkert, Adar Dembo,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/5100

to review the following change.

Change subject: el6: fix krb5 realm workaround for static builds
......................................................................

el6: fix krb5 realm workaround for static builds

Here's another attempt at enabling the workaround for the numeric realm
problem on el6 (see ba2ae3de4a7c43ff2f5873e822410e066ea99667 for
background).

The previous attempt didn't succeed for static builds because we were
setting LD_PRELOAD to the non-absolute path of the override library.
This worked OK in debug builds because our binary RUNPATHs included the
build/lib/ directory. Static builds didn't have such a RUNPATH and thus
the LD_PRELOAD was unable to locate the specified shared object.

The two options to fix this were (1) determine and specify an absolute
path, or (2) build this workaround into all of our binaries rather than
try to preload it at runtime.

The issue with #1 is that, when we get to Java-based testing, it would
have been inconvenient to determine the absolute path of the override
library. In addition, it would make test binaries non-relocatable, which
is somewhat of a hassle.

So, this takes approach #2. The build incantations are a bit nasty,
since for dynamically linked builds, it's critical that the override
library be listed on the linker invocation prior to the krb5 library
itself. Otherwise, we'll resolve the symbol from libkrb5.so and our
"override" won't work. To solve this, I added the override library just
prior to the 'krb5' library in the list of target dependencies for
'security'. Since this is the only place that krb5 is referenced, CMake
should retain our provided order.

In addition, we need to make sure that the linker doesn't entirely skip
the override binary. I accomplished this using the '--undefined' linker
flag.

I tested this patch using both sasl_rpc-test and
external_mini_cluster-test on an el6 box in both debug (dynamic linked)
and release (static-linked) builds. I also fully built both build types
to ensure there were no linker issues in unrelated tests.

Change-Id: I050d03d58658b650891566b9f955c867b15731e0
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/security/CMakeLists.txt
M src/kudu/security/test/krb5_realm_override.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_main.cc
M src/kudu/util/test_util.cc
7 files changed, 25 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/5100/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I050d03d58658b650891566b9f955c867b15731e0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>

Reply via email to