-
-
Notifications
You must be signed in to change notification settings - Fork 692
feat: expose interpreter files-to-run on PyRuntimeInfo #3795
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,25 +39,44 @@ def _py_runtime_impl(ctx): | |
| runfiles = ctx.runfiles() | ||
|
|
||
| hermetic = bool(interpreter) | ||
| interpreter_files_to_run = None | ||
| if not hermetic: | ||
| if runtime_files: | ||
| fail("if 'interpreter_path' is given then 'files' must be empty") | ||
| if not paths.is_absolute(interpreter_path): | ||
| fail("interpreter_path must be an absolute path") | ||
| else: | ||
| interpreter_di = interpreter[DefaultInfo] | ||
|
|
||
| if interpreter_di.files_to_run and interpreter_di.files_to_run.executable: | ||
| interpreter_file = None | ||
|
|
||
| # Direct file targets also expose files_to_run.executable. They should | ||
| # keep py_runtime's file-only behavior and not populate | ||
| # interpreter_files_to_run. Rule targets have OutputGroupInfo; direct | ||
| # file targets do not. | ||
| is_file_target = OutputGroupInfo not in interpreter | ||
| if _is_singleton_depset(interpreter_di.files): | ||
| interpreter_file = interpreter_di.files.to_list()[0] | ||
|
|
||
| if is_file_target and interpreter_file: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth noting somewhere that source-file interpreter now takes this path instead of the executable branch? eg. py_runtime(interpreter = ...) pointing at a downloaded/extracted file @python_x86_64//:bin/python3 rather than a *_binary This used to hit the executable branch and pick up the runfiles.merge/runtime_files expansion, and now skip it. |
||
| # Direct file label: use the file as the interpreter, but do not | ||
| # treat it as an executable target with runfiles metadata. | ||
| interpreter = interpreter_file | ||
| elif interpreter_di.files_to_run and interpreter_di.files_to_run.executable: | ||
| # Executable rule target: use the executable and preserve the full | ||
| # FilesToRunProvider so action consumers can stage its runfiles. | ||
| interpreter = interpreter_di.files_to_run.executable | ||
| interpreter_files_to_run = interpreter_di.files_to_run | ||
| runfiles = runfiles.merge(interpreter_di.default_runfiles) | ||
|
|
||
| runtime_files = depset(transitive = [ | ||
| interpreter_di.files, | ||
| interpreter_di.default_runfiles.files, | ||
| runtime_files, | ||
| ]) | ||
| elif _is_singleton_depset(interpreter_di.files): | ||
| interpreter = interpreter_di.files.to_list()[0] | ||
| elif interpreter_file: | ||
| # Non-executable rule with exactly one output: preserve the | ||
| # historical file-only interpreter behavior. | ||
| interpreter = interpreter_file | ||
| else: | ||
| fail("interpreter must be an executable target or must produce exactly one file.") | ||
|
|
||
|
|
@@ -111,6 +130,7 @@ def _py_runtime_impl(ctx): | |
| py_runtime_info_kwargs = dict( | ||
| interpreter_path = interpreter_path or None, | ||
| interpreter = interpreter, | ||
| interpreter_files_to_run = interpreter_files_to_run, | ||
| files = runtime_files if hermetic else None, | ||
| coverage_tool = coverage_tool, | ||
| coverage_files = coverage_files, | ||
|
|
@@ -119,6 +139,7 @@ def _py_runtime_impl(ctx): | |
| bootstrap_template = ctx.file.bootstrap_template, | ||
| ) | ||
| builtin_py_runtime_info_kwargs = dict(py_runtime_info_kwargs) | ||
| builtin_py_runtime_info_kwargs.pop("interpreter_files_to_run", None) | ||
|
|
||
| # There are all args that BuiltinPyRuntimeInfo doesn't support | ||
| py_runtime_info_kwargs.update(dict( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this key off
interpreter_file.is_sourceinstead of theOutputGroupInfocheck? The case you're guarding against is really a source file — that's the only non-executable input with afiles_to_run.executable— andis_sourcedoesnt rely on which providers Bazel attaches to rule vsfile targets.