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: proof of concept for license splicing #685

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

Conversation

aiuto
Copy link
Contributor

@aiuto aiuto commented Apr 25, 2022

@danielmachlab This is proof of concept of the license lookup indirection.

I am not happy that we can't get a fallback repository. But, it could be done in setup,
with the requirement that you call setup.

@aiuto aiuto added the wip label Apr 25, 2022
@aiuto aiuto marked this pull request as draft April 25, 2022 21:20
@aiuto
Copy link
Contributor Author

aiuto commented Apr 26, 2022

This is quite annoying.

  • The primary setup method rules_jvm_external_deps() calls maven_install to create the repo rules_jvm_external_deps
  • so maven_install can't load the the classifier repository unless that was created
  • but the place you want to create it is in rules_jvm_external_deps()
  • then the setup method depends on the repo rules_jvm_external_deps

Sigh...

@danielmachlab
Copy link
Contributor

@aiuto this workflow looks good, especially considering the restrictions we have. It all works locally for me. What should the next steps be? It might be useful to get a third opinion on the direction we are heading in while finishing up the prototype lookup_license function. I can make a PR tomorrow or Friday into this PR to fill in the lookup_license function.

@aiuto
Copy link
Contributor Author

aiuto commented Apr 28, 2022

I don't know. I don't really like that you need a third setup function for this repository. I want to get the @license_checker repository injected by one of the first rules, so that all the examples don't have to change. My thought is that I could point to rules_jvm_external for the checker as a default, and add an optional rule that rewrites the dependency reader to point to the other repository.

But... you could certainly use what I have here to test the license injection at your scale.

@jin
Copy link
Collaborator

jin commented Apr 28, 2022

I'm quite confused about what's happening here. Is there an issue/context for this FR?

@danielmachlab
Copy link
Contributor

Hi @jin we are trying to find a good way to allow users of rules_jvm_external to apply rules_license licenses to the artifacts that they import with maven_install. This is coming out of the new rules_license SIG (see monthly meeting notes here). Here is a document which describes our problem and our original approach for this change. The initial approach didn't work out since we can't pass a macro to a repository rule.

@danielmachlab
Copy link
Contributor

Hi @aiuto, circling back to this. In this approach, why couldn't we make it so it is backward compatible where users not using a license classifier can avoid adding use_default_license_classifier() to their WORKSPACE?

I'm thinking that the optimal case would be for a user to:

  1. create a file defining lookup_license
  2. load set_license_classifier from rje, which accepts the path of the lookup_license file.

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.

3 participants