aspect-build / rules_py

More compatible Bazel rules for running Python tools and building Python projects
Apache License 2.0
86 stars 29 forks source link

[FR]: Add ability to pass additional user defined flags to interpreter #436

Open muravev-vasilii opened 2 weeks ago

muravev-vasilii commented 2 weeks ago

What is the current behavior?

Currently bootstrap script doesn't allow to pass custom interpreter flags to Python interpreter: https://github.com/aspect-build/rules_py/blob/main/py/private/run.tmpl.sh#L61

Two flags are added by rules_py implementation (https://github.com/aspect-build/rules_py/blob/main/py/private/py_semantics.bzl#L6-L14), but I didn't find an option to add more flags defined by user to interpreter

args argument for py_binary target allows to pass custom arguments to invoked script, but not to interpreter itself.

Describe the feature

I would like to have ability to pass custom user flags to Python interpreter used by rules_py.

Main motivation for it to allow to run py_binary targets under debugpy debugger via bazel. Instead of having following line in bootstrap file

exec "{{EXEC_PYTHON_BIN}}" {{INTERPRETER_FLAGS}} "$(rlocation {{ENTRYPOINT}})" "$@"

I want to have something like +exec "{{EXEC_PYTHON_BIN}}" {{INTERPRETER_FLAGS}} $RULES_PY_USER_INTERPRETER_FLAGS "$(rlocation {{ENTRYPOINT}})" "$@"

Example usage will be: RULES_PY_USER_INTERPRETER_FLAGS="-m debugpy --listen 12345 --wait-for-client" bazel run //path/to/rules_py/py_binary/target

I've seen and tried proposed way to debug via creating venv from https://github.com/bazelbuild/rules_python/issues/1401#issuecomment-2389768034

This way doesn't work for us in our monorepo setting for couple of reasons:

Similar feature request from bazel slack: https://bazelbuild.slack.com/archives/CA31HN1T3/p1730573900089359

Another alternative I was thinking is to use run_under flag (https://bazel.build/reference/command-line-reference#flag--run_under), but it seems to be not feasible for debugpy use case, as bootstrap script is shell and not Python.

If there is a way to specify custom toolchain for this use case I am happy to learn how to do it as well!

Thanks!

muravev-vasilii commented 1 week ago

Created example PR with proposed change and described how I tested it: https://github.com/aspect-build/rules_py/pull/442

mattem commented 1 week ago

I think we should explore allowing the interpreter flags to be set on the toolchain and target (like node_interpreter_args from rules_js). The DX of just the env var isn't great. We also need to validate the flags passed perhaps, as there are interpreter flags that rules_py relies on, and we don't want users to override theses.

For the debug case, I'd propose this case, rather than modifying the template too much.

py_binary(
    name = "foo",
    srcs = [...],
    deps = [...],
    env = select({
         "//:is_dbg": {"PYDEVD_RESOLVE_SYMLINKS": "1" },
         "//conditions:default": {},
    }),
    interpreter_args = select({
         "//:is_dbg": ["-m", "debugpy", "--listen", "12345", "--wait-for-client"],
         "//conditions:default": [],
    }),
)

Then the bazel invocation is bazel run -c dbg //:foo for debugging (in the debug compilation mode). This allows the flags to be defined in the BUILD file, users don't have to remember them, or the magic env var to set.

muravev-vasilii commented 1 week ago

Yes, that would be great for us if we could just add flags and env setting as another py_binary arguments. Running with -c dbg is also preferable, as we planned to use it anyway to add debugpy requirement as an optional dependency.

muravev-vasilii commented 3 days ago

@mattem I tried out your idea, it works. Updated PR https://github.com/aspect-build/rules_py/pull/442 with new proposed change. Not sure about validating flags part. If you could share some specifics here, I would try to add this as well.

alexeagle commented 3 days ago

@mattem do you want me to take this one over, or can you still continue as design reviewer?