[5.3.1] Bazel 5.3 fails to run external tests

[bazel-io]

Forked from #16003

[fmeum]

At Bazel HEAD, #16008 (comment) could be fixed by adding yet another rpath entry using #14600 which is going to be merged soon.

For 5.3.1, one option would be to cherry-pick #14600 and that additional rpath entry.

Alternatively, we could revert both e745468 and 0080572 in Bazel 5.3.1, but that could regress users that started to rely on tests being executed from the runfiles root.

@oquenchil What do you think is the best path forward here? Would having up to three different rpaths on every binary have the potential to cause issues? As far as I understand, the names of dynamically linked binaries are relatively unique within Bazel, but could additional rpath entries that don't apply to the way the binary is actually executed lead to non-hermetic behavior?

[oquenchil]

What do you think is the best path forward here? Would having up to three different rpaths on every binary have the potential to cause issues? As far as I understand, the names of dynamically linked binaries are relatively unique within Bazel, but could additional rpath entries that don't apply to the way the binary is actually executed lead to non-hermetic behavior?

I don't know to be honest. I'd prefer if we do it the cleaner way and revert whatever commits need to be reverted.

[Wyverald]

Unfortunately it's unlikely we can revert 0080572 since it's needed for Roboleaf and has been behavior for over a year now.

According to @oquenchil, @lberki is the best person to talk to about rpaths, but unfortunately he's still on leave for another two weeks.

So, as someone who knows absolutely squat about rpaths (and after consulting @fmeum), I think the best course of action now is merge #14600 into master, cherry-pick it into 5.3.1, cherry-pick another change that adds that extra rpath, and let tests take care of the problem of "do all these combinations work together".

[Wyverald]

Somewhat of an update: #14600 was merged into master and then rolled back since it broke targets with commas in their name.

@pcjanzen How severe is the breakage you noted in #16008 (comment) ? I'm trying to assess whether we need to hold up the 5.3.1 release for it.

[fmeum]

I think that the current issue could be fixed along the lines of #14600 without introducing the breakage on commas. I can have a PR ready by Monday.

[pcjanzen]

How severe is the breakage you noted in #16008 (comment) ?

I don't think the case of running a test from an external repository from the command line is especially severe; however, the problem also affects local execution of cc_binary tools that are either dynamically linked, or that contain a reference to a shared library via cc_import.

See RobotLocomotion/drake#17763 for a real-world example.