Repository: bazelbuild/bazel
Issue: Neither python_top nor python toolchain works with starlark actions on windows
Labels: type: bug
P1
team-Rules-Python

### Description of the problem / feature request:

Trying to use a `py_binary()` target as executable with `actions.run()` fails on windows if `py_runtime()` is used. Error Message:
```
LAUNCHER ERROR: Cannot launch process: "python.exe" ${PATH}\my_py_binary.zip ${ARGS}
Reason: (error: 2): The system cannot find the file specified.
```

This issue happens with `python_top` and the new python toolchain feature. It works fine on Linux though.

### Feature requests: what underlying problem are you trying to solve with this feature?

Executing python actions during the build with a hermetic python environment. 

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

see [here](https://github.com/pziggo/bazel_py_runtime_win_example)

### What operating system are you running Bazel on?

Windows 10

### What's the output of `bazel info release`?

`development version`

### If `bazel info release` returns "development version" or "(@non-git)", tell us how you built Bazel.

`bazel build //src:bazel_jdk_minimal` on Windows 10

### What's the output of `git remote get-url origin ; git rev-parse master ; git rev-parse HEAD` ?
https://github.com/bazelbuild/bazel.git
78d68315cb6dc2d7e747b444fa493a78cb2a6f09
78d68315cb6dc2d7e747b444fa493a78cb2a6f09

###  Have you found anything relevant by searching the web?

Discussion has been started on Slack: https://bazelbuild.slack.com/archives/CA306CEV6/p1554299740008400

### Any other information, logs, or outputs that you want to share?

From the resulting `__main__.py`
```
PYTHON_BINARY = 'python3_windows/python.exe'
if IsWindows() and not HasWindowsExecutableExtension(PYTHON_BINARY):
 PYTHON_BINARY = PYTHON_BINARY + '.exe'
```

from the rule definition:
```
"_generator": attr.label(
    cfg = "host",
    executable = True,
    allow_files = False,
    default = Label("//bazel/tools/generator:generator"),
),
```

and from the rule implementation
```
ctx.actions.run(
    inputs = [],
    outputs = [out_file],
    executable = ctx.executable._generator,
    arguments = [args],
)
```


Issue comments:
cc @brandjon 
Ok, I played around with our windows sandbox vm and think I understand. The most likely situation is that your `py_runtime`'s `interpreter_path` attribute doesn't correctly identify a `Python.exe` binary. Please double check the path, and if you don't think that's the problem, please post the `py_runtime` definition here.

From my experimentation, it doesn't look like the `PATH` environment variable affects whether or not the absolute path is able to resolve successfully at execution time (i.e. we're not sandboxing anything in Windows in a way that breaks this).

The error message is a little misleading: Our Python launcher [will first check](https://github.com/bazelbuild/bazel/blob/1f684e1b87cd8881a0a4b33e86ba66743e32d674/src/tools/launcher/python_launcher.cc#L30-L35) whether the given Python interpreter file exists, and if not, fall back on "python.exe" on its own. This means that the error message you see will always say it can't find "python.exe" even if you specified `interpreter_path = r"C:\my_custom_interpreter.exe"` in your `py_runtime`.

(Note the "r" raw string literal prefix in that last sentence. Depending on your path to the Python interpreter, it's possible you're using a single backslash before something that's interpreted as a valid escape sequence, producing a different string than you think. When in doubt, it's valid to replace backslashes with slashes in windows paths.)

Please let me know if this helps. If nothing else, we'll keep this issue open to track the bad error message in the Python launcher (or the removal of that fallback entirely).
Actually we are using an in-build runtime from an external dependency and finally I managed to setup a simple example here [pziggo/bazel_py_runtime_win_example](https://github.com/pziggo/bazel_py_runtime_win_example).

I also debugged the issue a bit more and I think I'm getting closer to understand the root cause. Thanks for the link which was a good starting point.

The [python launcher](https://github.com/bazelbuild/bazel/blob/master/src/tools/launcher/python_launcher.cc#L34) checks if the given python interpreter exists by converting the `PYTHON_BIN_PATH` into an [absolute path on Windows](https://github.com/bazelbuild/bazel/blob/master/src/tools/launcher/util/launcher_util.cc#L99). In fact by using the in-build runtime from an external dependency, the constructed path really doesn't exists:
```
LAUNCHER ERROR: Pyhon binary: python3_windows/python.exe
LAUNCHER ERROR: Absolute python binary path: \\?\c:\users\me\_bazel_me\rsotqivv\execroot\bazel_py_runtime_win_example\python3_windows\python.exe
```
while the following path exists (note the "external" in the path):
```
C:\Users\me\_bazel_me\rsotqivv\execroot\bazel_py_runtime_win_example\external\python3_windows
```

So, before handing over the `python_bin_path` to the python launcher [here](https://github.com/bazelbuild/bazel/blob/558b717e906156477b1c6bd29d049a0fb8e18b27/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java#L218), the path will be [modified](https://github.com/bazelbuild/bazel/blob/558b717e906156477b1c6bd29d049a0fb8e18b27/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java#L362) and the `external` part stripped for the [runfiles tree](https://github.com/bazelbuild/bazel/blob/5458e5bb9130a537d2d6c40f3d9e42c502e4276a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java#L685). But in fact, on Windows the runfiles tree lives inside the resulting zip file and thus is not a valid path for running the packaged zip if I'm not mistaken.
That would mean, for `createWindowsExeLauncher()` the value of `pythonBinary` must be different then for the functions that build the content of the zip file / using the runfiles.
Thank you for the investigation and repro! I haven't run your repro yet, but if I understand correctly it sounds like the scope of this problem is anytime the `py_runtime` references an in-build interpreter from an external repo on Windows.
Yes, sounds about right.
But I wonder what would be a good change to solve the problem. Fixing the path could be relatively easy, but that would not work on remote execution if I’m not mistaken.
So, the interpreter should become a direct runtime dependency and available in the runfiles tree. But I think having the interpreter twice (in the runfiles and inside the zip) is not optimal either for the overall performance.
+@laszlocsomor FYI.

So to summarize: When an in-build interpreter is used on Windows, the Python launcher will take its workspace-relative path and use that in combination with its own working directory to construct the full (non-runfile) path to the interpreter. Due to a bug, this fails when the interpreter is defined in an external repo.

I'm not sure what the plan is for remote execution and windows, but there are likely already other blockers within the Python rules. For instance, BazelPythonSemantics [checks](https://github.com/bazelbuild/bazel/blob/4c6fb96cebefa5fc2b0cb31d1fcd2292fb30e652/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java#L145) `OS.getCurrent` to decide whether to use a launcher, but that's not needed if the target platform isn't windows.

Aside from that, this behavior is another difference between unix and Windows. On unix, the shebang of the stub script is `#!/usr/bin/env python` regardless of what interpreter will later be used to execute the payload. But on Windows, this launcher behavior means that the stub script (in the zip file's `__main__.py`) will use whatever the payload's interpreter is, instead of always using a standard system interpreter to bootstrap. I don't see an obvious flaw in doing it that way, but it is a conceptual difference.

You can see an analogous version of this bug in google/subpar#98, where in-build interpreters caused the shebang to get a relative path.
Regarding priority: This issue prevents using many kinds of in-workspace Python runtimes on Windows, including interpreters provided by external repos (e.g. `@bazel_tools`) and interpreters that are generated artifacts as opposed to checked-in binaries. In particular, it blocks the addition of the autodetecting toolchain on Windows (#7844). However, it does not block the roll out of Python toolchains (#7899), because the default toolchain on Windows is a hack that preserves legacy behavior.

Therefore, a fix for this won't be in 0.27, but it'd be great to have it for 0.28.

---

Now on to the actual design issue. As I alluded to above, on unix we use a two-phase approach to launching Python programs.

1. A system interpreter is used to run the stub script or zip file, which initializes env vars, locates/extracts runfiles, etc. This interpreter is discovered by the shebang `#!/usr/bin/env python`.

2. A separate interpreter is called by the stub logic to run the payload user code. This may be either a system interpreter or an in-workspace (i.e. runfiles) interpreter.

This separation into two separate phases is what enables us to use an in-workspace interpreter packaged within a zip file. The zip depends on a system interpreter to bootstrap and extract itself.

On Windows, this separation does not currently exist: Regardless of whether you're using a zip file or not, the Python launcher will try to run the stub script with the same interpreter that your toolchain requests to be used for payload user code. *This can be an interpreter in runfiles, which haven't been extracted yet.*

The way we currently get around this is by giving the launcher a path to the interpreter binary on disk, i.e. the actual checked-in source file or the built binary file in the output tree. This is pretty brittle. First, there's the breakage described above, when the path construction doesn't quite work out. But even if we fix that by passing an absolute path to the on-disk file, we'd end up in a situation where the resulting `py_binary` (launcher) executable breaks if you move your source files around or run `bazel clean`, even if you've already copied the exe and zip files elsewhere on your system.

So let's revisit our strategy: There's no reason we need to use the same interpreter for the zip file / stub script that we do for the payload code. We can assume the presence of a system Python interpreter for the purposes of bootstrapping. The launcher can hardcode that assumption, which could take different forms, for example:

* `python.exe` is on PATH
* `py.exe` is on PATH
* I can locate a Python interpreter by doing some registry stuff

It's even possible to let the Python toolchain customize how the bootstrapping interpreter is found, just like you could let it parameterize the shebang string. But I don't see much value in this at the moment.

So right now, I think the goal should be to add `python.exe` / `py.exe` detection to the launcher, and no longer pass an interpreter to the launcher. Note that we write the stub script to target a lowest common denominator of reasonable Python interpreters, so it shouldn't matter what version interpreter the launcher comes up with.
Thanks for the summary!

>  on unix we use a two-phase approach to launching Python programs.

IIUC we do so because the stub script itself is in Python, so the binary requires a system interpreter even in presence of a bundled interpreter. But isn't part of a bundled interpreter's goal to enable running without a system interpreter?

The analogous problem is running Bazel itself. The embedded JVM lets you run Bazel without a system JVM. Its launcher (the bazel client) is a native binary.

> On Windows, this separation does not currently exist: Regardless of whether you're using a zip file or not, the Python launcher will try to run the stub script with the same interpreter that your toolchain requests to be used for payload user code. *This can be an interpreter in runfiles, which haven't been extracted yet.*

Sure, the launcher should begin with extracting the payload. We can fix that.

> We can assume the presence of a system Python interpreter for the purposes of bootstrapping.

Doesn't that partially beat the purpose of a bundled interpreter? If the stub weren't a python script but were part of the .exe launcher, we wouldn't need this assumption.

My questions:
- what do you all think about my assessment that the bundled interpreter should enable the binary to run without a system interpreter?
- what does the stub script do? could the native launcher do that instead?
- the native launcher's `python_binary` field could either contain a runfile path (e.g. `runfile:python3_windows/python.exe`, meaning either a runfile path to be retrieved with rlocation or a path in the zip), or a basename (e.g. `python.exe`, meaning the current interpreter on the PATH, whatever it may be) -- WDYT?
> the native launcher's python_binary field could either contain a runfile path (e.g. runfile:python3_windows/python.exe, meaning either a runfile path to be retrieved with rlocation or a path in the zip)

This is true when using python toolchain or `--python_top`. That's why #8440 can fix this problem.
With #8440, I can run all commands in https://github.com/pziggo/bazel_py_runtime_win_example, but got error like this
```
pcloudy@pcloudy0-w MSYS ~/workspace/bazel_py_runtime_win_example
$ bazel build --python_top=//tools/python:py3_windows_runtime //:test
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Reading 'startup' options from c:\users\pcloudy\.bazelrc: --output_user_root=C:/src/tmp
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=320
INFO: Options provided by the client:
  'build' options: --python_path=C:/Python36/python.exe
INFO: Reading rc options for 'build' from c:\users\pcloudy\.bazelrc:
  'build' options: --curses=yes --color=yes --verbose_failures --announce_rc
INFO: Analyzed target //:test (14 packages loaded, 113 targets configured).
INFO: Found 1 target...
INFO: Deleting stale sandbox base C:/src/tmp/ewf7xl5y/sandbox
ERROR: C:/tools/msys64/home/pcloudy/workspace/bazel_py_runtime_win_example/BUILD:3:1: Generating file failed (Exit 1): generator.exe failed: error executing command
  cd C:/src/tmp/ewf7xl5y/execroot/bazel_py_runtime_win_example
bazel-out/host/bin/tools/generator/generator.exe bazel-out/x64_windows-fastbuild/bin/file
Execution platform: @bazel_tools//platforms:host_platform
Traceback (most recent call last):
  File "D:\obj\Windows-Release\37amd64_Release\msi_python\zip_amd64\runpy.py", line 193, in _run_module_as_main
  File "D:\obj\Windows-Release\37amd64_Release\msi_python\zip_amd64\runpy.py", line 85, in _run_code
  File "C:\src\tmp\ewf7xl5y\execroot\bazel_py_runtime_win_example\bazel-out\host\bin\tools\generator\generator.zip\__main__.py", line 303, in <module>
  File "C:\src\tmp\ewf7xl5y\execroot\bazel_py_runtime_win_example\bazel-out\host\bin\tools\generator\generator.zip\__main__.py", line 282, in Main
NameError: name 'exit' is not defined
C:\Users\pcloudy\AppData\Local\Temp\Bazel.runfiles_s6rwiedn\runfiles\python3_windows\python.exe
Target //:test failed to build
INFO: Elapsed time: 5.256s, Critical Path: 0.47s
INFO: 0 processes.
FAILED: Build did NOT complete successfully
```
But it's caused by the python binary we are using, not a Bazel bug.
> But isn't part of a bundled interpreter's goal to enable running without a system interpreter?

I don't think this is something we need to prioritize, but it'd be nice to have in the future. With a bundled interpreter that requires the system interpreter for bootstrapping, you still get hermeticity once you're past the stub script phase.

> The analogous problem is running Bazel itself. The embedded JVM lets you run Bazel without a system JVM. Its launcher (the bazel client) is a native binary.

Then by that logic we could embed a native Python interpreter (perhaps miniaturized) into each py_binary artifact. But at that point perhaps it'd be worth rewriting the stub script in C++.

An alternative to embedding an extra interpreter into the artifact would be to have a way of extracting the payload user code's interpreter from runfiles without using Python... and again the way to do that would be essentially rewriting the stub in C++.

> Sure, the launcher should begin with extracting the payload. We can fix that.

Sounds like we're arriving at similar conclusions.

> what does the stub script do? could the native launcher do that instead?

The [stub script](https://github.com/bazelbuild/bazel/blob/6f38f345a1bd278a71170c5d80aba3928afdc6ec/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt#L221):

1. locates the runfiles directory, or locates and extracts the zip to create the runfiles directory
2. initializes environment variables, including for the runfiles library, and PYTHONPATH
3. locates the main file (entry point of payload user code) and second-stage Python interpreter
4. invokes the interpreter on the main file and cleans up the extracted zip contents

> the native launcher's python_binary field could either contain a runfile path (e.g. runfile:python3_windows/python.exe, meaning either a runfile path to be retrieved with rlocation or a path in the zip), or a basename (e.g. python.exe, meaning the current interpreter on the PATH, whatever it may be) -- WDYT?

In principle the stub script should be runnable under any reasonable Python interpreter (with some hand wavyness around "reasonable"). So there shouldn't be a need to customize which system interpreter is used, whether it's `python2`, `python3`, `python.exe`, `py.exe` (the Python wrapper on Windows), etc. I'd like to avoid adding the ability to customize that until it proves to be appropriate and useful. Same goes for the unix shebang, which is currently hardcoded as `#!/usr/bin/env python`.

However, if we're eliminating the dependency on a system interpreter and just using whatever interpreter is specified for the payload user code (whether system or runfiles based), then we'd just pass that along to the launcher, as we do today.
> But it's caused by the python binary we are using, not a Bazel bug.

[Apparently](https://stackoverflow.com/a/6501134) the `exit()` function doesn't exist if you invoke Python with `-S`. We should change our calls to `exit()` in the stub script to instead use `sys.exit()`. I'll make a quick fix for that.
> > But isn't part of a bundled interpreter's goal to enable running without a system interpreter?
> 
> I don't think this is something we need to prioritize, but it'd be nice to have in the future. With a bundled interpreter that requires the system interpreter for bootstrapping, you still get hermeticity once you're past the stub script phase.

Thanks for confirming. I just wanted to note that requiring an ambient Python interpreter might cause frustration. People were unhappy about Bazel requiring MSYS2 too.

> > The analogous problem is running Bazel itself. The embedded JVM lets you run Bazel without a system JVM. Its launcher (the bazel client) is a native binary.
> 
> Then by that logic we could embed a native Python interpreter (perhaps miniaturized) into each py_binary artifact. But at that point perhaps it'd be worth rewriting the stub script in C++.

Sounds reasonable.

> An alternative to embedding an extra interpreter into the artifact would be to have a way of extracting the payload user code's interpreter from runfiles without using Python... and again the way to do that would be essentially rewriting the stub in C++.

Agreed. The Bazel client does the same.

> > Sure, the launcher should begin with extracting the payload. We can fix that.
> 
> Sounds like we're arriving at similar conclusions.

Indeed. :)

> > what does the stub script do? could the native launcher do that instead?
> 
> The [stub script](https://github.com/bazelbuild/bazel/blob/6f38f345a1bd278a71170c5d80aba3928afdc6ec/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt#L221):
> 
> 1. locates the runfiles directory, or locates and extracts the zip to create the runfiles directory
> 2. initializes environment variables, including for the runfiles library, and PYTHONPATH
> 3. locates the main file (entry point of payload user code) and second-stage Python interpreter
> 4. invokes the interpreter on the main file and cleans up the extracted zip contents

Thanks. None of that sounds daunting. I implemented the [test wrapper in C++](https://github.com/bazelbuild/bazel/blob/6f38f345a1bd278a71170c5d80aba3928afdc6ec/tools/test/windows/tw.cc). It solves similar problems, we could reuse it. (It's Windows-focused though.)
That all sounds good to me. I forked off #8446.

In the meantime, we should make the launcher better at detecting a system interpreter. We can track that in this bug.
Let's discuss the merits of the approach of #8440 here. That PR makes it so if a runfiles path is passed to the launcher, it uses rlocation to resolve that to its absolute path. Since when a zip file is used the runfiles are manifest-based rather than directory-based, this doesn't suffer the chicken-and-egg problem of accessing an interpreter in a zip.

It does suffer the problem where if the build is cleaned, the .exe artifact is broken, even if it is copied out of the output tree. Currently I believe the .exe only depends on having the .zip in the same directory. I'm ok with settling for this limitation for now as a fix of existing behavior. It'll be moot once the launcher extracts the zip.
Although, one consequence of this design is that if you have no system Python interpreter, but your build has a checked-in interpreter, it'll work on your development machine since the actual not-in-runfiles interpreter file is available, and then fail on a user's machine. But we don't have a better option before #8446, unless we want to fail fast on the development machine.
Indeed #8440 is just for making existing behavior that works on Linux also works on Windows. 

Beyond this, it's not a Windows only problem. Ideally we should move all logic in `python_stub_template.txt` to the python native launcher. I considered to do so when implementing the native launcher, but at that time we were still making many changes to `python_stub_template.txt` and native launcher is only for Windows (also true today).
@brandjon , @meteorcloudy : does any of you plan to work on this?
@brandjon has a change to enable more python toolchain integration tests on Windows, I will submit my fix for this issue afterwards.

Yet I have no plan to migrate the `python_stub_template.txt` to C++.
Thanks. I consider migrating it.
Can anyone help me prioritize it?
My pending fix blocked on a failing test over the long weekend, looking at it again today.
My fix finally went through, merging the fix to this issue now.

Pull request review comments:
Can the input `bash_binary` ever be just `bash.exe` for example, which would mean "whatever is on the PATH"?
I think there are two ways to tell which shell binary to use, `BAZEL_SH` or `--shell_executable`. Both way requires user to set an absolute path. The only case `bash_binary` is a relative path (runfile path) is that when using shell toolchain (similar to python toolchain), but I'm not sure it's implemented.
But here, `python_binary` will just be `python` if nothing is specified.. I need to fix this.
Thanks, let me know when the PR is ready for another review.
Thanks for catching the problem! Please take a look again. ;)
Just checking: `bash_binary` can either be an absolute Bash path (from `BAZEL_SH` or `--shell_executable`), or (theoretically perhaps) a bash toolchain's runfiles-relative path. Correct?

And it always ends with "bash" or "bash.exe" (so just "bash" without the extension), right?

Any other option I'm missing?
What other values could `python_binary` have, and in what situations?
Everything you said I believe is true, I don't think there's any other option.
Similar to shell, we have

1. `--python_path`, gives an absolute path
2. `--python_top`, gives an runfile path
3. python toolchain (`py_runtime`), gives a runfile path
4. Default if nothing specified: `python`

@brandjon Can confirm this.
Thanks for confirming that!
Could you please add a comment about this?
Thanks! Waiting for @brandjon to confirm before LGTM. Please add a comment in the code summarizing Jon's answer.
Will do that!
Oh, just realized I had a typo. It should be `GetBinaryPathWithoutExtension(bash_binary) != L"bash")`, `GetBinaryPathWithoutExtension` just strips out the ending `.exe` if there is any. I wanted to check if bash_binary is already `bash` or `bash.exe`. Similar for the python case.
Will add comment for this!
Both `--python_top` and Python toolchains select a `py_runtime` to use. `py_runtime` provides a runfiles path if its `interpreter` attribute is set, and an absolute path if its `interpreter_path` attribute is set (these attributes are exclusive).

`--python_path` is validated to be an absolute path. If nothing is specified, the `python` default is passed along the same code path that `--python_path` uses but skips this validation check as a special case. It is intended that both of these approaches are deprecated and replaced by toolchains, with the `python` PATH lookup behavior provided by an autodetecting toolchain.
Thanks for the confirmation, I'll add those to comment
Rewrite comment as follows:

> There are three kinds of values for `python_binary`:
> 1. An absolute path to a system interpreter. This is the case if `--python_path` is set by the user, or if a `py_runtime` is used that has `interpreter_path` set.
> 2. A runfile path to an in-workspace interpreter. This is the case if a `py_runtime` is used that has `interpreter` set.
> 3. The special constant, "python". This is the default case if neither of the above apply.
> Rlocation resolves runfiles paths to absolute paths, and if given an absolute path it leaves it alone, so it's suitable for cases 1 and 2.

There's a slight detail that the default toolchain's py_runtime on windows contains a sentinel value hack that tells bazel to use legacy --python_path, but I don't think we need to mention that in this comment.
Thanks for help with the comment!

Patch that resolved the issue:
diff --git a/src/tools/launcher/bash_launcher.cc b/src/tools/launcher/bash_launcher.cc
index 0020d5f92d55d3..67f0a5d53a2b51 100644
--- a/src/tools/launcher/bash_launcher.cc
+++ b/src/tools/launcher/bash_launcher.cc
@@ -30,6 +30,14 @@ static constexpr const char* BASH_BIN_PATH = "bash_bin_path";
 
 ExitCode BashBinaryLauncher::Launch() {
   wstring bash_binary = this->GetLaunchInfoByKey(BASH_BIN_PATH);
+
+  // If bash_binary is already "bash" or "bash.exe", that means we want to
+  // rely on the shell binary in PATH, no need to do Rlocation.
+  if (GetBinaryPathWithoutExtension(bash_binary) != L"bash") {
+    // Rlocation returns the original path if bash_binary is an absolute path.
+    bash_binary = this->Rlocation(bash_binary, true);
+  }
+
   if (DoesFilePathExist(bash_binary.c_str())) {
     wstring bash_bin_dir = GetParentDirFromPath(bash_binary);
     wstring path_env;
diff --git a/src/tools/launcher/python_launcher.cc b/src/tools/launcher/python_launcher.cc
index c4d2d5c04670fd..be655cf1c4eeb2 100644
--- a/src/tools/launcher/python_launcher.cc
+++ b/src/tools/launcher/python_launcher.cc
@@ -30,6 +30,20 @@ static constexpr const char* WINDOWS_STYLE_ESCAPE_JVM_FLAGS = "escape_args";
 
 ExitCode PythonBinaryLauncher::Launch() {
   wstring python_binary = this->GetLaunchInfoByKey(PYTHON_BIN_PATH);
+
+  // There are three kinds of values for `python_binary`:
+  // 1. An absolute path to a system interpreter. This is the case if `--python_path` is set by the
+  //    user, or if a `py_runtime` is used that has `interpreter_path` set.
+  // 2. A runfile path to an in-workspace interpreter. This is the case if a `py_runtime` is used
+  //    that has `interpreter` set.
+  // 3. The special constant, "python". This is the default case if neither of the above apply.
+  // Rlocation resolves runfiles paths to absolute paths, and if given an absolute path it leaves
+  // it alone, so it's suitable for cases 1 and 2.
+  if (GetBinaryPathWithoutExtension(python_binary) != L"python") {
+    // Rlocation returns the original path if python_binary is an absolute path.
+    python_binary = this->Rlocation(python_binary, true);
+  }
+
   // If specified python binary path doesn't exist, then fall back to
   // python.exe and hope it's in PATH.
   if (!DoesFilePathExist(python_binary.c_str())) {


Test patch: