Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
muravev-vasilii opened this issue Nov 14, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@muravev-vasilii
Copy link

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 bazelbuild/rules_python#1401 (comment)

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

  • We use custom pytest_runner
  • We have our own VSCode extension which uses bazel to run tests and debug in TestExplorer and other things and we would prefer to keep interaction with tests and codebase through bazel, not via python interpreter from rules_py venv
  • It creates one extra step for our user to be able to interact with IDE, comparing with current setup

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
Copy link
Author

Created example PR with proposed change and described how I tested it: #442

@mattem
Copy link
Contributor

mattem commented Nov 20, 2024

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
Copy link
Author

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
Copy link
Author

muravev-vasilii commented Nov 26, 2024

@mattem I tried out your idea, it works. Updated PR #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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants