Skip to content
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

Add aliases support to build_script.template #475

Closed
wants to merge 2 commits into from
Closed

Add aliases support to build_script.template #475

wants to merge 2 commits into from

Conversation

scasagrande
Copy link

This PR adds support for generated BUILD files containing cargo_build_script to make use of the aliases keyword.

The need for this was discovered when using cargo raze on a project that pulled in value-bag as a sub-dependency. This project requires an alias of version-check as rustc in order to run build.rs.

Copy-pasting the code block from common-attrs where this has already been solved for rust_library adds the same solution to cargo_build_script.

I am a Rust noob, so if this isn't the best-practices solution to this problem, feel free to close.

@google-cla
Copy link

google-cla bot commented Mar 21, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

Copy link
Collaborator

@dfreese dfreese left a comment

Choose a reason for hiding this comment

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

looks good to me. Can you merge in the latest main to pick up some clippy fixes? If it still passes CI after that, I'm good with this.

@scasagrande
Copy link
Author

Thanks @dfreese ! I have updated my branch, and it looks like we just need CI to be approved to run again

@dfreese
Copy link
Collaborator

dfreese commented Mar 30, 2022

Still have this on my plate. #477 made me want to double check first that this wouldn't cause breakages by unintentionally adding aliases on build dependencies. I don't know off hand if they should be shared between build and normal deps.

@scasagrande
Copy link
Author

No problem @dfreese . I'm jumping between many different problems in my bazel adventure, so as I learn more I might be able to cycle back and more rigorously implement this change

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.

2 participants