Dependencies by component ID reference

[itowlson]

Draft for discussion of the approach.

The idea here is to allow dependencies of the form:

[component.the-main-course.dependencies]
"useless:[email protected]" = { component_id = "yet-another-example-dep" }

where yet-another-example-dep is another component in the manifest (typically, one not attached to a trigger).

I originally took a sane and reasonable approach, but had to abandon it because I had problems passing enough component info through to the composer for it to resolve the ID. The current approach uses or misuses the normalisation step instead, to turn the ID reference into a "proper" dependency source (e.g. local file, package reference, etc.) based on the component.source of the dependency.

This works (by which I mean it has worked for me at least once). However:

• It makes normalisation fallible, which is a new thing and not one I love.
• It puts composition logic into normalisation (knowing which fields are allowed in dependencies).
• It requires ComponentDependency to have a variant that's used in deserialisation but which must never be seen by the composer (because the composer can't process it, it has to be normalised away first).
  • This could in principle be addressed by decoupling the composer from the serialisation objects - I am not yet sure how much of a faff that is.

So I'm putting this up for people to point out the OBVIOUSLY SUPERIOR IN EVERY RESPECT way of doing this that I've completely overlooked.

(or of course to tell me it's a terrible idea and we shouldn't do it anyway)

Thanks!

cc @fibonacci1729 @tschneidereit

[tschneidereit]

This is great, thank you so much!

I can't really comment on the implementation all that much, so instead I'll comment on the color of this bikeshed's cladding and suggest changing component_id to just component. That matches the use in trigger definitions, besides in my eyes being a bit tidier.

Okay, I have a slightly more substantial question, though it still pertains to the paint more than the construction, and is only kinda-sorta related to this particular change:

Would it at all be possible to support the following syntax, by any chance?

[component.the-main-course.dependencies."useless:feature@0.0.1"]
component_id = "yet-another-example-dep"

The reason I ask is that I would quite like to be able to add more "stuff" to the individual dependencies' tables, which is impossible for all reasonable intents and purposes if all I can work with are inline tables.

[lann]

Would it at all be possible to support the following syntax, by any chance?

Not just possible; those are equivalent in TOML.

[lann]

Suggested change

	anyhow::bail!("Dependencies may not have their own resources or permissions. Component {depended_on_id} cannot be used as a dependency of {depender_id} because it specifies: {}", surprises.join(", "));
	anyhow::bail!("🎉 Dependencies may not have their own resources or permissions. Component {depended_on_id} cannot be used as a dependency of {depender_id} because it specifies: {} 🥳", surprises.join(", "));

[lann]

Just confirming: is the goal here just to enable nicer spin build for dependencies?

[lann]

This kind of validation always makes me a little nervous where a future field addition could easily slip through the net here.

Two alternative suggestions:

• Destructure all the fields of component and let the compiler complain if you miss one:

  let Component {
      ai_models,
      // ...
      variables,
  } = component;

• (Ab)use toml::Value; something vaguely like:

  #[derive(Deserialize)]
  struct DependencyComponent {
      source: serde::de::IgnoredAny,
      #[serde(flatten)]
      surprises: Map<String, serde::de::IgnoredAny>,
  }
  let value = toml::Value::try_from(component).unwrap();
  let dep_component: DependencyComponent = value.try_into()?;
  let surprises = dep_component.surprises.keys();

[itowlson]

Good call - I did the first and it showed me I'd forgotten allowed_http_hosts!

[tschneidereit]

Just confirming: is the goal here just to enable nicer spin build for dependencies?

Kinda, in this version?

Where this gets more interesting is once you combine it with profiles. I would expect (and, @itowlson, we should probably make sure that whichever of these PRs lands first makes it so) a dependency defined in this way to automatically use the right profile for the dependency, so you don't have to list the entire [...dependencies] table again for each profile.

[tschneidereit]

t just possible; those are equivalent in TOML.

I could swear I had this failing for me the other day, but I must've done something wrong.

(For posterity: I was, indeed, wrong, and the error I encountered was unrelated.)

[lann]

Another option, with its own warts:

[component.the-main-course.dependencies."useless:feature@0.0.1"]
path = "dep/target/wasm32-wasip2/release/dep.wasm"

[component.the-main-course.dependencies."useless:feature@0.0.1".build]
command = "cargo build"
workdir = "dep"

[tschneidereit]

Another option, with its own warts

So effectively inline definition of dependencies? I don't think I'm necessarily opposed to this as an option, but it's certainly not a replacement for anything: we'll always want to have the ability to use a component as a dependency for multiple other components.

[itowlson]

Just confirming: is the goal here just to enable nicer spin build for dependencies?

As well as Till's note, for me it is intention-revealing. That is, although we can achieve the same thing by duplicating a file path, that creates a submarine dependency - the reader has to spot "oh that file is the same as this file" - whereas naming the depended-on component makes it obvious "ah that is part of this application and I can go here to look at it."