fix(bootstrap) handle when the runfiles env vars are not correct by adhoc-bobcat · Pull Request #3644 · bazel-contrib/rules_python

@adhoc-bobcat

There was still an issue where the runfiles environment variables could
be set for a parent Python binary, and if that spawned another Python
binary (directly or indirectly), the child would try to use the parents
runfiles.

This was confirmable by adding a py_library dependency to the existing
bin_calls_bin test, and having inner.py try to import it.

A workaround sugggested in the bug for code outside of rules_python was
to remove RUNFILES_DIR from the environment inherited by the child when
launching the child process, though in some cases you might need to also
remove RUNFILES_MANIFEST_FILE, but working around the issue is not very
satisfactory.

Diving into the bootstrapping process, it seemed like the proper fix was
to conditionally unset them, if they were not correct. I found
equivalent places in both bootstrap template files, and if the test to
confirm "runfile_dir" was correct fails, the bootstrap code itself now
removes those invalid values from the environment.

Later bootstrap code assumes they are set correctly, if set. When they
are not set, it goes through some fallback logic to locate them.

By conditionally unsetting them, the fallback logic is not invoked when
it isn't necessary, shortening the bootstrap process.

With that change, the modified tests now pass.

Fixes bazel-contrib#3518