-
-
Notifications
You must be signed in to change notification settings - Fork 44
Added module extension for fetching CPAN dependencies #86
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
Conversation
293c30e to
a12c410
Compare
|
@lalten I would love it if you could take a look at the changes here since you have some experience in this domain with rules_cpan! |
|
For developing this I had to use a bootstrap script to originally generate the lock file. I'll post it here for future use in case the compiler tool breaks and lockfiles can no longer be generated. |
|
Could you summarize how the implementation is different from rules_cpan? I think it would be preferable to have just one way that works rather than two different implementations. I agree with #83 (comment) that we should move the repos closer together to increase discoverability and improve maintainability |
The main difference is that users do not need to separately go run There's currently a shared limitation with both implementations in that the dependencies need to be installable on the host which requires some host tools but given the interface in this PR I think there's a path forward where the tool just hits the CPAN API and only operates on metadata. Additionally, the repository rules in this PR generate a DAG of dependencies vs a flat target which can be useful when trying to debug issues in external libraries. Down the road this could also be useful for allowing mods/annotations to the packages to inject user defined alterations into the generated module (similar to @rules_rust//crate_universe:defs.bzl%crate.annotation) |
|
@lalten would you still be willing to do a full review if you think the direction is good? |
lalten
left a comment
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.
Great work!
Co-authored-by: Laurenz <lalten@users.noreply.github.com>
Co-authored-by: Laurenz <lalten@users.noreply.github.com>
|
@lalten back to you! |
|
lgtm! I think this is better than what's currently at rules_cpan so once this lands it would be cool if you could migrate the current BCR users (which is only Lcov I believe)? Then I'd archive rules_cpan and point users at this implementation. |
|
@skeletonkey are you also able to take a look? |
This change introduces the
perl_cpan_compilerrule andperl_cpanextension module.The
perl_cpan_compilerrule is backed by Carton and generates a lock file from a givencpanfilethat can then be passed toperl_cpanto generate dependencies.Known limitations:
This change does not impact any existing rules but does add a new entry to
MODULE.bazelso should be considered a minor change.closes #83