-
Notifications
You must be signed in to change notification settings - Fork 409
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
Generate package license metadata #2476
Generate package license metadata #2476
Conversation
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). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Can you show the rendered output of a BUILD file after this change?
Here is the BUILD file rendered from the rand crate when passing
|
Thanks! Should rules_license be added as a dependency in places? |
If you don't have a |
I have added a dependency on rules_license in both |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the old
# licenses([
# "TODO", # MIT OR Apache-2.0
# ])
snippets from the generated BUILD files. They should be unnecessary in light of rules_license
I've update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far! Question though, the only metadata that's generated is for licensing right? I renamed the PR to Generate package license metadata
, is that a more accurate description or is there more data being generated here? If not I think the config flag should also include the word "license" if possible
It generates two metadata items: Although both are provided by rules_license only the As these two metaitems are the only ones that currently exist, the flag does decide if the BUILD file However if you are concerned that you may include additional default_package_metadata items in the future and don't want them coupled with this config flag perhaps specifying that it is going to generate the metadata providers from rules_license would a good choice? Perhaps: |
I like the latter option if you don’t mind :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you so much!
…bazelbuild#2521) bazelbuild#2476 added rules_license license metadata to crate BUILD files but many crates to do not have a license file specified in their cargo metadata. This PR adds a fallback that attempts to locate a license file in the crate package root directory.
#2475