[Graph] error encoding a component: "import name conflicts with previous name"

[rylev]

Imagine two components targeting this world:

world {
  import wasi:http/[email protected];
  export wasi:http/[email protected];
}

The following code composes these components such that the export from component 2 is supplied as the import to component 1 and then the export from component 1 is ultimately exported from the composition:

    let component1 = wac_graph::types::Package::from_bytes("component1", None, Arc::new(component.into())).unwrap();
    let component2 = wac_graph::types::Package::from_bytes("component2", None, Arc::new(component.into())).unwrap();

    let mut graph = wac_graph::CompositionGraph::new();
    let component1 = graph.register_package(component1).unwrap();
    let component2 = graph.register_package(component2).unwrap();

    let component1 = graph.instantiate(component1).unwrap();
    let component2 = graph.instantiate(component2).unwrap();

    let alias = graph
        .alias_instance_export(component2, "wasi:http/[email protected]")
        .unwrap();

    graph
        .connect_argument(alias, component1, "wasi:http/[email protected]")
        .unwrap();
    let export = graph
        .alias_instance_export(component1, "wasi:http/[email protected]")
        .unwrap();

    graph
        .export(export, "wasi:http/[email protected]")
        .unwrap();

    graph.encode().unwrap();

This errors with the following message:

import name `wasi:http/[email protected]` conflicts with previous name `wasi:http/[email protected]

Looking into where implicit imports are encoded in CompositionGraphEncoder::populate_implicit_args, it seems that the type aggregator is not dedupping interfaces as I would expect. After aggregation the aggregator's Types collection has a duplicate copy of the wasi:http/[email protected] interface.

I believe the issue seems to be in remap_interface that the interface id associated with any used type is not in the same arena as the types in aggregated collection. The indices seem to be the same, but when doing look ups against the aggregated collection, None is returned.

This means that we treat the used type's interface as a completely unseen interface, and we'll remap and then add it to the interfaces collection instead of seeing that it's already been aggregated. Perhaps this is ultimately correct as we can't be sure that this interface is actually equivalent to the interfaces we've seen before.

However, instead of remapping the interface and adding it to the collection of interfaces, we need to try to merge the interface with any previously seen interface with the same name. This would indicate to me that instead of doing remapping when iterating through all the used types, we need to merge the used type's interface into an existing interface with the same name.

I believe this is what the aggregate method essentially does, so perhaps the right answer is to call aggregate instead of remap_interface. This would require threading the cache down into remap_interface and making remap_interface fallible as the merging could potentially fail. This would also require making a version of aggregate that does not consume self.

Am I thinking about this correctly or am I totally off base?

[peterhuene]

I think your assessment is probably correct, something is going wrong in the aggregation such that the transitively "used" interface of wasi:http/types is not being properly deduped and merged.

[peterhuene]

Which reminds me, I need to address #55 in the graph as well: encode on the graph should error if you attempt to implicitly import an argument that is used from another argument which has been supplied (i.e. incoming-handler is supplied, but types is not).

[peterhuene]

Also, I think you're right about recursing with aggregate, as merging of usings is really a transitive import so we need to keep track of whether or not we've seen that "name" before.

I think we could probably replace the cache argument to aggregate with the checker and continue to just pass that down.

[peterhuene]

Although, aggregate intentionally consumes self; let me see what a proper fix would look like.