fix(py_image_layer): support multiple binaries#1349
Conversation
|
|
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
The multi-binary API needs a correctness guard and real execution coverage before this replaces the local patch. Different lock universes/interpreters can currently pass the plan comparison and then run a secondary launcher against the primary runtime; the public signature also breaks positional callers.
|
|
||
| return _source_map | ||
|
|
||
| def _dependency_plan_signature(info): |
There was a problem hiding this comment.
This signature compares normalized package labels/layer groups and the interpreter group, not the actual wheel-install repository/version or interpreter identity. Two binaries from divergent lock universes can therefore compare equal, after which the secondary discards its own deps/interpreter and runs against the primary's runtime. Please compare the concrete runtime identities (or reject distinct install repos/interpreters) and add an e2e that executes both launchers plus a divergent-plan negative.
| @@ -998,6 +1120,8 @@ _py_image_layer = rule( | |||
| def py_image_layer( | |||
There was a problem hiding this comment.
additional_binaries and launcher_dir are inserted before the existing groups argument, so existing positional py_image_layer(...) calls silently rebind. Please append the new optional parameters after the established public arguments (or make the interface explicitly keyword-only without changing existing calls).
| mandatory = True, | ||
| aspects = [_layer_aspect, _merge_aspect], | ||
| ), | ||
| "additional_binaries": attr.label_list( |
There was a problem hiding this comment.
Why not a single binaries attribute and the macro can deal with binary|binaries|additional_binaries terminology while the real rule just gets the single list?
|
Reimplemented the multi-binary path independently in #1353. It unions concrete wheel, interpreter, and first-party artifacts across binaries, preserves the existing positional API, uses one inner binaries attribute, and executes two launchers across separate lock universes/Python versions. This addresses the three open design/correctness threads here. |
|
Done in #1353 |
4 participants
Summary
Validation