From b491dbde5e1832f6d2a1ab0c7331b8f4fe67df14 Mon Sep 17 00:00:00 2001 From: Chris Novakovic Date: Tue, 21 Oct 2025 16:47:39 +0100 Subject: [PATCH] Make `InterpreterOptions` configuration option a list There are problems associated with `InterpreterOptions` being a string, chiefly that it isn't safe to assume where individual Python interpreter command line arguments begin and end simply by splitting the string on space characters. This isn't so much of a concern while the value of `InterpreterOptions` is just appended to the main .pex shebang in please_pex, but it'll become more of a concern when we instead rely on a native-code .pex preamble to invoke the interpreter because we'll be passing arguments variadically to `execvp(3)`. Make `InterpreterOptions` a repeatable configuration option and pass each value to please_pex as a separate `--interpreter_options` command line option (which itself also becomes repeatable). This isn't a breaking change because the values passed via `--interpreter_options` are for now ultimately just being stringified via a space-delimited join and appended to the .pex shebang, but the distinction will become meaningful when we implement a native-code .pex preamble. --- .plzconfig | 5 +++-- README.md | 3 ++- build_defs/python.build_defs | 12 +++++++++--- tools/please_pex/pex/pex.go | 6 +++--- tools/please_pex/pex_main.go | 2 +- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/.plzconfig b/.plzconfig index 5f116107..99608b77 100644 --- a/.plzconfig +++ b/.plzconfig @@ -42,8 +42,9 @@ Help = A path or build label for the pex tool, which is used to create .pex file [PluginConfig "interpreter_options"] ConfigKey = InterpreterOptions -DefaultValue = "" -Help = Any additional flags to pass to the python interpreter +Optional = true +Repeatable = true +Help = Additional command line arguments to pass to the Python interpreter. [PluginConfig "test_runner"] ConfigKey = TestRunner diff --git a/README.md b/README.md index 45239c48..9357f433 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,8 @@ The available configuration options are: ```ini [Plugin "python"] -InterpreterOptions = -b -s +InterpreterOptions = -b +InterpreterOptions = -s DefaultInterpreter = python3 PexTool = //tools/please_pex TestRunner = unittest diff --git a/build_defs/python.build_defs b/build_defs/python.build_defs index 2d0ab7df..e41da18e 100644 --- a/build_defs/python.build_defs +++ b/build_defs/python.build_defs @@ -147,8 +147,9 @@ def python_binary(name:str, main:str, srcs:list=[], resources:list=[], out:str=N shebang = shebang or _interpreter_cmd(interpreter) zipsafe_flag = '' if zip_safe is False else '--zip_safe' - cmd = '$TOOLS_PEX -s "%s" -m "%s" %s --interpreter_options="%s"' % ( - shebang, module_dir, zipsafe_flag, CONFIG.PYTHON.INTERPRETER_OPTIONS) + interpreter_opts = _interpreter_opts(CONFIG.PYTHON.INTERPRETER_OPTIONS) + cmd = '$TOOLS_PEX -s "%s" -m "%s" %s %s' % ( + shebang, module_dir, zipsafe_flag, interpreter_opts) # If content hashing feature flag is enabled, we use the hash of the built # dependencies instead of the RULE_HASH as the base of the pex extraction @@ -270,7 +271,8 @@ def python_test(name:str, srcs:list, data:list|dict=[], resources:list=[], deps: """ test_runner = test_runner or CONFIG.PYTHON.TEST_RUNNER shebang = shebang or _interpreter_cmd(interpreter) - cmd = f'$TOOLS_PEX -t -s "{shebang}" -m "{module_dir}" -r "{test_runner}" --zip_safe --add_test_runner_deps --interpreter_options="{CONFIG.PYTHON.INTERPRETER_OPTIONS}" --stamp="$RULE_HASH"' + interpreter_opts = _interpreter_opts(CONFIG.PYTHON.INTERPRETER_OPTIONS) + cmd = f'$TOOLS_PEX -t -s "{shebang}" -m "{module_dir}" -r "{test_runner}" --zip_safe --add_test_runner_deps {interpreter_opts} --stamp="$RULE_HASH"' if site: cmd += ' -S' @@ -746,6 +748,10 @@ def _interpreter_cmd(interpreter:str): return f'$(out {interpreter})' if looks_like_build_label(interpreter) else interpreter +def _interpreter_opts(opts:list): + return " ".join([f'--interpreter_options="{o}"' for o in opts]) + + def _patch_cmd(patch): """Returns a command for the given patch or patches, with OS-specific tweaks where needed.""" patches = [patch] if isinstance(patch, str) else patch diff --git a/tools/please_pex/pex/pex.go b/tools/please_pex/pex/pex.go index 8c65918c..1fb6822a 100644 --- a/tools/please_pex/pex/pex.go +++ b/tools/please_pex/pex/pex.go @@ -47,7 +47,7 @@ type Writer struct { } // NewWriter constructs a new Writer. -func NewWriter(entryPoint, interpreter, options, stamp string, zipSafe, noSite bool) *Writer { +func NewWriter(entryPoint, interpreter string, options []string, stamp string, zipSafe, noSite bool) *Writer { pw := &Writer{ zipSafe: zipSafe, noSite: noSite, @@ -59,8 +59,8 @@ func NewWriter(entryPoint, interpreter, options, stamp string, zipSafe, noSite b } // SetShebang sets the leading shebang that will be written to the file. -func (pw *Writer) SetShebang(shebang string, options string) { - shebang = strings.TrimSpace(fmt.Sprintf("%s %s", shebang, options)) +func (pw *Writer) SetShebang(shebang string, options []string) { + shebang = strings.TrimSpace(fmt.Sprintf("%s %s", shebang, strings.Join(options, " "))) if !path.IsAbs(shebang) { shebang = "/usr/bin/env " + shebang } diff --git a/tools/please_pex/pex_main.go b/tools/please_pex/pex_main.go index 4fca6f69..f5f86594 100644 --- a/tools/please_pex/pex_main.go +++ b/tools/please_pex/pex_main.go @@ -22,7 +22,7 @@ var opts = struct { TestRunner string `short:"r" long:"test_runner" default:"unittest" description:"Test runner to use"` Shebang string `short:"s" long:"shebang" description:"Explicitly set shebang to this"` Stamp string `long:"stamp" description:"Unique value used to derive cache directory for pex"` - InterpreterOptions string `long:"interpreter_options" description:"Options-string to pass to the python interpreter"` + InterpreterOptions []string `long:"interpreter_options" description:"Additional command line arguments to pass to the Python interpreter"` Test bool `short:"t" long:"test" description:"True if we're to build a test"` Debug pex.Debugger `short:"d" long:"debug" optional:"true" optional-value:"pdb" choice:"pdb" choice:"debugpy" description:"Debugger to generate a debugging pex"` Site bool `short:"S" long:"site" description:"Allow the pex to import site at startup"`