Skip to content

Commit 7827e0b

Browse files
fix: handle unsubstituted template placeholders for external native py_binary (#3495)
### Problem In #3242, it looks like `rules_python` introduced new template placeholders in the bootstrap scripts: `%stage2_bootstrap%` and `%interpreter_args%`. And from what I can tell, also made their successful substitution a requirement. This works totally fine when the caller is calling `rules_python` directly. However, when external repositories (like gRPC's cython) define py_binary using the native rule, these placeholders don't seem to be substituted? The result is that the literal placeholder text ends up in the generated bootstrap scripts, causing `SyntaxError`s or file-not-found errors at runtime ### Fix Detect if the `%stage2_bootstrap%` variable isn't expanded and fallback to `%main%` which IS substituted even for native `py_binary`. For `%interpreter_args%`, wrap it in triple-quotes so it's hopefully always valid Python syntax, then detect the sentinel and default to an empty list. This is a bit hacky, but is fairly non-invasive. --------- Co-authored-by: Richard Levasseur <rlevasseur@google.com>
1 parent b4ec825 commit 7827e0b

2 files changed

Lines changed: 30 additions & 8 deletions

File tree

python/private/py_executable.bzl

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -728,10 +728,7 @@ def _create_stage1_bootstrap(
728728
resolve_python_binary_at_runtime = "1"
729729

730730
subs = {
731-
"%interpreter_args%": "\n".join([
732-
'"{}"'.format(v)
733-
for v in ctx.attr.interpreter_args
734-
]),
731+
"%interpreter_args%": "\n".join(ctx.attr.interpreter_args),
735732
"%is_zipfile%": "1" if is_for_zip else "0",
736733
"%python_binary%": python_binary_path,
737734
"%python_binary_actual%": python_binary_actual,

python/private/python_bootstrap_template.txt

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,31 @@ import sys
1010
import os
1111
import subprocess
1212
import uuid
13-
1413
# runfiles-relative path
14+
# NOTE: The sentinel strings are split (e.g., "%stage2" + "_bootstrap%") so that
15+
# the substitution logic won't replace them. This allows runtime detection of
16+
# unsubstituted placeholders, which occurs when native py_binary is used in
17+
# external repositories. In that case, we fall back to %main% which Bazel's
18+
# native rule does substitute.
19+
_STAGE2_BOOTSTRAP_SENTINEL = "%stage2" + "_bootstrap%"
1520
STAGE2_BOOTSTRAP="%stage2_bootstrap%"
1621

22+
# NOTE: The fallback logic from stage2_bootstrap to main is only present
23+
# as a courtesy for an older, unsupported, configuration. It can be removed
24+
# when that case is unlikely to be a concern anymore.
25+
# See https://github.com/bazel-contrib/rules_python/pull/3495
26+
if STAGE2_BOOTSTRAP == _STAGE2_BOOTSTRAP_SENTINEL:
27+
_MAIN_SENTINEL = "%main" + "%"
28+
_main = "%main%"
29+
if _main != _MAIN_SENTINEL and _main:
30+
STAGE2_BOOTSTRAP = _main
31+
else:
32+
STAGE2_BOOTSTRAP = ""
33+
34+
if not STAGE2_BOOTSTRAP:
35+
print("ERROR: %stage2_bootstrap% (or %main%) was not substituted.", file=sys.stderr)
36+
sys.exit(1)
37+
1738
# runfiles-relative path to venv's python interpreter
1839
# Empty string if a venv is not setup.
1940
PYTHON_BINARY = '%python_binary%'
@@ -35,9 +56,13 @@ RECREATE_VENV_AT_RUNTIME="%recreate_venv_at_runtime%"
3556
WORKSPACE_NAME = "%workspace_name%"
3657

3758
# Target-specific interpreter args.
38-
INTERPRETER_ARGS = [
39-
%interpreter_args%
40-
]
59+
# Sentinel split to detect unsubstituted placeholder (see STAGE2_BOOTSTRAP above).
60+
_INTERPRETER_ARGS_SENTINEL = "%interpreter" + "_args%"
61+
_INTERPRETER_ARGS_RAW = "%interpreter_args%"
62+
if _INTERPRETER_ARGS_RAW == _INTERPRETER_ARGS_SENTINEL:
63+
INTERPRETER_ARGS = []
64+
else:
65+
INTERPRETER_ARGS = [arg for arg in _INTERPRETER_ARGS_RAW.split("\n") if arg]
4166

4267
ADDITIONAL_INTERPRETER_ARGS = os.environ.get("RULES_PYTHON_ADDITIONAL_INTERPRETER_ARGS", "")
4368

0 commit comments

Comments
 (0)