-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat: support asdf plugins through a proto WASM plugin #540
base: master
Are you sure you want to change the base?
Conversation
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.
@milesj It is an experimental prototype. To track the progress, I have referenced below with my doubts. PTAL
( I have kept them as default values to better know their functionality there)
// Need to ensure file type and unpack accordingly | ||
if input_file.ends_with(".tar.xz") { | ||
//TODO: Implement the untar and unzip moments | ||
// untar(input_file, output_dir)?; |
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.
Is this untar and unzip could be custom implemented or any suggested crates for it?
#[plugin_fn] | ||
pub fn detect_version_files(_: ()) -> FnResult<Json<DetectVersionOutput>> { | ||
let asdf_plugin = AsdfPlugin { | ||
tool: Tool::default(), |
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.
There is no default method for initializing an instance of AsdfPlugin with a default instance of Tool. Can it be implemented in Tool impl?
} | ||
|
||
pub fn pre_install(&self, mut input: InstallHook) { | ||
let config: AsdfConfig = self.tool.config(); |
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.
I had confusion here about referencing config. Any guidance on this?
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.
@@ -0,0 +1,226 @@ | |||
use dirs; | |||
use extism_pdk::*; | |||
use proto_core::Tool; |
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.
Plugins shouldn't use proto_core
at all. Refer to the other plugins as a reference: https://github.com/moonrepo/node-plugin
@varshith257 Plugins don't belong in this repo, so it should probably be its own repo? I'm not sure how bounties work in that case. |
@milesj We can implement it and reference this issue there and if a PR merged there it will automatically considered for claim and another approach is that we can move this issue to the repo |
Moving this work to the new repo as we discussed |
Fixes #539
/claim #539