Skip to content

Conversation

@cburroughs
Copy link
Contributor

The initial implementation merged additional_args from package_pex_binary and the scie args, but inadvertently dropped any additional_args added by create_pex_from_targets itself. Notably create_pex_from_targets does so for local_dists. create_pex_from_targets needs to go "first" do the merge between all three additional_args sources.

LLM Disclosure: A LLM both figured out the bug and wrote the fix+test.

@cburroughs cburroughs self-assigned this Jan 15, 2026
@cburroughs cburroughs added the release-notes:not-required [CI] PR doesn't require mention in release notes label Jan 15, 2026
entry_point="main.py",
dependencies=[":sources", "lib:dist"],
scie="eager", # Going to run it, so might as well
scie_pbs_release="20251031",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this test run, and thus download this PBS each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure how the caching works in RuleRunner/integration-test land. We could keep it lazy and rely on Pex erroring instead of the end-to-end stdout assertion if you are concerned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not concerned yet. Just trying to keep a mental audit of stuff we do that can/will drag test out time - and whether we can cache in CI to help that out.

So, just a bit of an identification process right now - wasn't sure if you'd run this and seen the result yet.

@cburroughs cburroughs marked this pull request as ready for review January 15, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:not-required [CI] PR doesn't require mention in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants