fix(python): Set envvar for runfiles manifest, not runfiles dir, when using a manifest

[fmeum]

Fixes #17675

[fmeum]

@rickeylev Could you review?

[fmeum]

@bazel-io flag

[rickeylev]

Can we add a test for this? In src/test/shell/integration/python_stub_test.sh, we can easily plug in a no-op interpreter that just prints env, then grep for the RUNFILES_MANIFEST_FILE. I think we just need to set --build_runfile_manifests when doing the build and invoke it using bazel-bin to make the right code paths get followed.

[fmeum]

@rickeylev Started working on the test, but I can't figure out how to build an input but not an output manifest. --build_runfile_manifests does seem to control both. I could of course delete one from the output base manually, but that feels hacky.

[rickeylev]

Hmm....yeah I don't see a way to generate only one and not the other. Maybe it's specific to certain platforms? Or maybe if...ah, what if it's running from a zip file? CreateModuleSpace() will unzip to a temp directory. The zip will contain MANIFEST, but the temp directory won't have .runfiles_manifest

In the original issue description, they manually copy the runfiles tree elsewhere and run, which is approximately the same as unzipping elsewhere.

I also think it'd be fine to just delete the .runfiles_manifest manually. It's effectively the same as the other two approaches, just with less copy+pasting of things around.

Let's just put a comment in the test about the case we're trying to reproduce; it'll look pretty funny deleting files from the output tree like that.

[ShreeM01]

@bazel-io fork 6.2.0

[fmeum]

@rickeylev I tried to get this to work but noticed that I can't come up with a scenario in which the change becomes relevant: In ZIP-based builds the incorrect branch isn't reached since IsRunningFromZip() is checked earlier. With --nobuild_runfile_links and a non-ZIP-based build, Python can't run as it needs the runfiles directory structure laid out. I would need a scenario in which the runfiles directory can only be found using the manifest file, which in turn is found via argv[0] - but is there a natural case in which this is the case?

[rickeylev]

Ah darn. Ok, well, lets just go back to the other ideas:

• Manually deleting the {name}.runfiles_manifest file
• The steps described in the issue: cp -r bazel-bin/foo bazel-bin/foo.runfiles /tmp/work && /tmp/work/foo

If none of that works, well, lets just omit the tests, because we can't figure out a way to test it, which is fine. That line of code is obviously wrong.

[keertk]

Hi @fmeum @rickeylev we're planning to start creating RCs for 6.2.0 on 4/24. Checking if we still want to include this?

[fmeum]

@rickeylev I couldn't figure out how to reproduce this situation in tests, even with these ideas. I added the returns comment and your suggestions.

[rickeylev]

re: LocalDiffAwarenessIntegrationTest failure -- no idea what that is or why it would fail. Looking at the test, I don't see any mention of Python in it. I'm guessing its flaky mac test. I pressed retry.

In any case, I'd say this is fine to begin importing. Our internal test infrastructure will handle flaky tests better.