Suggestion: Share AppSource and ResolvedAppSource in some form

[Mossaka]

As a maintainer of the spin shim, I found these two structs particularly useful for running spin application either from the local filesystem or from oci, but we can't directly take a dependency on the spin-cli crate due to build.rs. In order to share more logic between the up command and the spin Engine implementation in the containerd-shim, I'd suggest to move these two structs to spin_app crate that we depend on. Thoughts?

See: deislabs/containerd-wasm-shims#164

[itowlson]

I wouldn't want to see these in spin-app, which relates specifically to the runtime environment and is bound to spin-core and wasmtime, but agree they may be reusable (cloud-plugin has very similar logic and has the same goal of loading interchangeably from spin.toml or OCI reference). They are bound up with the load process and particularly with how the Spin UI resolves references (whether paths or OCI references), so maybe in loader, or in their own crate so as to be specific and limited about dependencies...?

[Mossaka]

Yeah that makes sense. I am fine with either solutions as long as they are shareable in some form because we can't depend on spin-cli.

Maybe we can create a new crate like spin-cli-lib and move as much logic from spin cli to it as possible. WDYT?

[itowlson]

My feelings about that are very mixed.

On the one hand, yes, it could be a powerful enabler of consistent behaviour and experience across different Spin runtime hosts (for hosts that want to opt into Spin CLI-like experiences, of course).

On the other hand, it likely ends up linking to everything in the world, meaning that innocent helper tools that just want to resolve a --from flag end up dragging in the whole of the runtime (as cloud-plugin did until recently!). This can technically be avoided with features, but features are unreliable at avoiding unintended dependencies. But perhaps breaking out lighter-weight subsets could be deferred - the cli-lib crate could always re-export broken-out types just as spin-app now does.

Let's have a look and see if any existing crates feel like a good home for these types. Otherwise, I'd suggest creating a tiny new crate for them, with cli-lib as a longer term idea. Thoughts?

[Mossaka]

For the time being, I think it would be great to add them to the loader crate. I can do some work here.

[lann]

There isn't very much code in (Resolved)AppSource; could you clarify which part(s) of it would be useful to share?

[Mossaka]

I was hoping to refactor this section of the code out: https://github.com/deislabs/containerd-wasm-shims/blob/main/containerd-shim-spin/src/engine.rs#L266-L294

[lann]

ResolvedAppSource exists for spin up to be able to "partially load" an app just to make trigger-specific --help work properly. Since the shim (presumably) doesn't need to do that I would suggest skipping that step and going straight to the LockedApp. Then you only need the OciRegistry arm of that function: https://github.com/deislabs/containerd-wasm-shims/blob/81d24438921494684ea9aaf0c3573dde01bf20df/containerd-shim-spin/src/engine.rs#L283-L292

[Mossaka]

Close as this is not needed anymore.