Append LD_LIBRARY_PATH for census

[johnkerl]

Sibling of #2216 and #2217

See also #2219 for tracking

[codecov]

Codecov Report

Merging #2218 (8c2224a) into main (29c55e1) will decrease coverage by 6.43%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2218      +/-   ##
==========================================
- Coverage   78.55%   72.13%   -6.43%     
==========================================
  Files         137      103      -34     
  Lines       10689     6875    -3814     
  Branches      215      215              
==========================================
- Hits         8397     4959    -3438     
+ Misses       2194     1818     -376     
  Partials       98       98              

Flag	Coverage Δ
libtiledbsoma	67.72% <ø> (+<0.01%)	⬆️
python	?
r	74.69% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
python_api	∅ <ø> (∅)
libtiledbsoma	48.85% <ø> (+0.07%)	⬆️

[bkmartinjr]

I am under the impression that the linker ignored leading/trailing semi-colons, in which case you can just do:

export LD_LIBRARY_PATH="${tiledb}:${LD_LIBRARY_PATH}"

that said, the extra code should not hurt anything...

[johnkerl]

@bkmartinjr I'd rather not count on that; who knows what system's linker somewhere is non-compliant -- safety is best -- thank you though!

[ryan-williams]

$"LD_LIBRARY_PATH" evaluates to the string literal LD_LIBRARY_PATH; the $ needs to be inside the quotes:

docker run --rm bash -c 'foo=1; echo $"foo" "$foo"'
# foo 1

[ -z $"LD_LIBRARY_PATH" ] is always false, so we've inadvertently been doing what @bkmartinjr suggested (always prepending ${tiledb}:). I think this is also why the set +u seemed necessary; the "append" block was running even with LD_LIBRARY_PATH unset, which set -u was flagging as an error.

A common way of doing these two if blocks is:

export LD_LIBRARY_PATH="$tiledb${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"
export DYLD_LIBRARY_PATH="$tiledb${DYLD_LIBRARY_PATH:+:$DYLD_LIBRARY_PATH}"

cf. SO. This also works in sh:

test() {
  docker run --rm --entrypoint sh ubuntu -c "set -u; $1"
}

test 'echo "prepended_val${foo:+:$foo}"'           # `foo` unset
# prepended_val
test 'foo=; echo "prepended_val${foo:+:$foo}"'     # `foo` empty
# prepended_val
test 'foo=val; echo "prepended_val${foo:+:$foo}"'  # `foo` nonempty
# prepended_val:val
test 'echo "prepended_val${foo:+:}$foo"'           # `foo` unset, and used outside `${foo:+…}` guard ⟹ `set -u` errors (as expected)
# sh: 1: foo: parameter not set

though I'd think this file's shebang line ensures we're always running bash.

#2927 contains a fix, as well as a couple other nits recommended by shellcheck (IntelliJ shellcheck plugin flagged this issue to me, originally).

[johnkerl]

Thank you @ryan-williams -- $" is very clearly wrong and I'm absolutely appalled that I missed a basic shell-syntax error on reviewing this :(

[bkmartinjr]

left a suggested simplification inline.