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

Rebuild export for multiple language backends #20487

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

lilatomic
Copy link
Contributor

@lilatomic lilatomic commented Feb 4, 2024

Well the scope is creeping:

  • export goal supports resolves in multiple languages
  • export and generate-lockfiles goals use same machinery for discovering and selecting resolves

Current status:

  • waiting for prework removing GenerateToolLockfileSentinel
  • exporting internal tools is pushed off for a future MR

Copy link
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

Solid approach!

But I wonder if the simpler solution is to set the default value of the resolves config to include all the tools? And we document that when you add your own resolves you append to this set, instead of replacing it? That way these truly are regular resolves, and are exposed as such, instead of special-casing some names.

I'm not 100% sure this is possible - something would have to find all the resolves before non-bootstrap options are parsed. But did you consider this?

@lilatomic
Copy link
Contributor Author

I hadn't considered that approach. I actually started this MR to get export working with TF, and then realised that it would be fairly easy to add a UnionMembership and a simple rule to hook tools into the export machinery. The main downside of this approach is that we're going back to needing to add an ExportSentinel to every exportable tool again. Although, unlike before, (once found) these tool lockfiles go through the same machinery as user lockfiles.

I don't know how options are bootstrapped, so I don't know if that's possible either. I think one complication of doing that would be that not all tools are Python tools (ex some are Java), so we'd need to implement something to know which language ecosystem each tool needs. I think we'd end up with code similar to what's in 007c364 in any case.

@benjyw
Copy link
Contributor

benjyw commented Mar 12, 2024

I dislike all that ExportSentinel boilerplate. I would love to find some way to represent this in a streamlined way as default config. I'll think on this, since I'm deep inside the options system right now.

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

Successfully merging this pull request may close these issues.

2 participants