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

WIP: License lookup - a better version #692

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aiuto
Copy link
Contributor

@aiuto aiuto commented Apr 28, 2022

This is a backwards compatible approach. It is so ugly it is delicious.

We start with a fallback license_lookup, implemented in rules_jvm_external. If you do nothing else you get that, so no license information is added.

If you want to override that with your own method:

  • implement //:license_classifier.bzl%lookup_license in a repository of your choosing. It could be a local one.
  • load that repository before rules_jvm_external.
  • load rules_jvm_external with a patch_cmd that points to your classifier repo.
local_repository(
  name = "my_compliance",
  path = "compliance",
)

http_archive(
    name = "rules_jvm_external",
    urls = ["file:///tmp/rje.tar"],
    patch_cmds = [
      "./use_license_classifier.sh my_compliance",
    ]
)

Note that the patch command is actually implemented in rules_jvm_external, so you don't have to write your own.

@aiuto aiuto added the wip label Apr 28, 2022
@aiuto aiuto marked this pull request as draft April 28, 2022 20:00
@aiuto aiuto removed the request for review from c-parsons April 28, 2022 20:00
@aiuto
Copy link
Contributor Author

aiuto commented Apr 28, 2022

cc: @danielmachlab I like this one better than the other PR.

Copy link
Contributor

@danielmachlab danielmachlab left a comment

Choose a reason for hiding this comment

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

Mmmm, yes, delicious! This definitely works. As you pointed out it is cumbersome, but it is backwards compatible. Sorry for the delayed review, I was OOO most of last week.

For a new user, what they need to do is:

  1. define a new repo with lookup_license
  2. add the patch_cmd to the http_archive or git_repository call

This is not too bad, but could be a little complex to onboard some. Thats okay.

When comparing this with #685, I left a comment on #685

http_archive(
name = "rules_jvm_external",
# path = "..",
urls = ["file:///tmp/rje.tar"],
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you generate this .tar? I ended up just using git_repository() workspace rule pointing at this branch as a work-around

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tar from the command line. This is just to get something to demo, not what we would really do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants