Skip to content

fix: plugin_output in py_proto_library rule#1280

Merged
chrislovecnm merged 2 commits intobazel-contrib:mainfrom
comius:fix-plugin_output
Jun 20, 2023
Merged

fix: plugin_output in py_proto_library rule#1280
chrislovecnm merged 2 commits intobazel-contrib:mainfrom
comius:fix-plugin_output

Conversation

@comius
Copy link
Copy Markdown
Contributor

@comius comius commented Jun 20, 2023

plugin_output was wrong in case multiple repositories are involved and/or _virtual_imports.

The code is taken from cc_proto_library and has been verified in practice.

@comius comius requested a review from rickeylev as a code owner June 20, 2023 16:43
Copy link
Copy Markdown
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

Is there an issue somewhere describing the bug in more detail? Or did you just come across this accidentally?

Otherwise lgtm

@comius
Copy link
Copy Markdown
Contributor Author

comius commented Jun 20, 2023

Is there an issue somewhere describing the bug in more detail? Or did you just come across this accidentally?

Otherwise lgtm

Accidentally. We had the same problem internally, reported on b/277335175.

And I spent a month fixing _virtual_imports and otherwise generally cleaning up/optimising proto_library implementation.

@chrislovecnm chrislovecnm added this pull request to the merge queue Jun 20, 2023
Merged via the queue into bazel-contrib:main with commit 1a333ce Jun 20, 2023
eed3si9n pushed a commit to eed3si9n/rules_python that referenced this pull request Aug 24, 2023
plugin_output was wrong in case multiple repositories are involved
and/or _virtual_imports.

The code is taken from `cc_proto_library` and has been verified in
practice.
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.

3 participants