Skip to content

Fix _get_important_outputs#53

Open
hartikainen wants to merge 2 commits intogoogle-deepmind:mainfrom
hartikainen:github/46
Open

Fix _get_important_outputs#53
hartikainen wants to merge 2 commits intogoogle-deepmind:mainfrom
hartikainen:github/46

Conversation

@hartikainen
Copy link
Copy Markdown

Fixes #46.

visited.add(current)
ns = named_sets.get(current)
if ns:
outputs.extend(ns.files)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I noticed that ns.files resulting from py_binary targets may contain more than one output: one for the binary and more than zero for the source code. This messes up the downstream usages which check len(paths) == 1. See e.g. here:

assert len(paths) == 1

I think we only care for the binary and should probably filter out the source files.

Copy link
Copy Markdown
Author

@hartikainen hartikainen Jul 7, 2025

Choose a reason for hiding this comment

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

See 19c62cd. My bazel-foo is a little weak and I don't know what the right way to handle this is. But I've verified not that it works for raw py_binary targets and also oci_load container image targets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_get_important_outputs fails (perhaps due to newer Bazel/Bzlmod)

1 participant