Skip to content

Create internal rust_binary rule instead of using transitions #1187

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

Merged
merged 5 commits into from
Mar 12, 2022

Conversation

dfreese
Copy link
Collaborator

@dfreese dfreese commented Mar 12, 2022

This seems like a cleaner way to break the dependency that rust_binary
has on the process wrapper. The common args are consumed and modified
to set the process wrapper to None, which is detected and performs the
appropriate switch. The transitions and settings can then be removed.

This seems like a cleaner way to break the dependency that rust_binary
has on the process wrapper.  The common args are consumed and modified
to set the process wrapper to None, which is detected and performs the
appropriate switch.  The transitions and settings can then be removed.
@ddeville
Copy link
Contributor

I can confirm that this fixes some cycles I was seeing when querying the bazel dependency graph after #1159.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

One question but otherwise looks good!

@dfreese
Copy link
Collaborator Author

dfreese commented Mar 12, 2022

All of those changes look fine to me. I'll be able to get back to this later Sunday night, so feel free to update and merge. I think you should have permission.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

@UebelAndre UebelAndre merged commit de726a1 into bazelbuild:main Mar 12, 2022
@hlopko
Copy link
Member

hlopko commented Mar 14, 2022

Thanks! Indeed much simpler!

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.

4 participants