Skip to content

wip(fix): import mapping resource overlap - #557

Closed
vados-cosmonic wants to merge 3 commits into
bytecodealliance:mainfrom
vados-cosmonic:fix=import-map-resource-overlap
Closed

wip(fix): import mapping resource overlap#557
vados-cosmonic wants to merge 3 commits into
bytecodealliance:mainfrom
vados-cosmonic:fix=import-map-resource-overlap

Conversation

@vados-cosmonic

@vados-cosmonic vados-cosmonic commented Jan 29, 2025

Copy link
Copy Markdown
Collaborator

This commit attempts to fix the case of an import map that perfectly overlaps an existing component resource.

When a resource that exists in a component is overriden with an import map, the interface can change arbitrarily.


This handle failure seems to actually be much deeper... Where I've landed is that resources are broken when import mapped over.

I've been able to figure out that the import map'd resource that overlaps with the local one is being interpreted as a host & imported & owned resource.

They basically are being detected as owned when they should not be -- while it is possible to have a ResourceData::Host that is imported and owned, in this case the resource is actually coming from the import map (it's not the same resource as the component created/is in the WIT).

I can't tell if the right move here is to:

  • Disable host-mapping over resources that are defined by the component in question
  • Modify bindgen to figure out that import-mapped things (which are missing/don't construct properly) and use them differently somehow
  • make changes to componentize.js to maybe avoid producing lowering code (???) that may be causing the failures

I'm thinking it might be a componentize problem because the function in the WASM has no idea about the import mapping above, and is probably trying to lower using the definitions in the component (that don't match the import map).

The first issue was attempting to remove the resource that wasn't actually successfully created which I found a way to work around (that maybe makes sense), but I'm finding that while you can create the resource, you can't consume it (i.e. from consumeBar) inside the component.

@guybedford

Copy link
Copy Markdown
Collaborator

A local resource should never be an import - and imports always need to provide external resources that correspond to the correct imported resource type.

So this should be less about import mapping and more about whether the resource is imported or not.

That is, the approach in this PR doesn't sound correct to me - it would be good to dig in further to find the root cause fix here.

@guybedford

Copy link
Copy Markdown
Collaborator

Do we have a test case that replicates the issue?

@vados-cosmonic

Copy link
Copy Markdown
Collaborator Author

This failure actually is a part of the test cases that was commented out (I'm assuming because something was wrong):

jco/test/cli.js

Line 564 in 46e729c

// strictEqual(m.consumeBar(m.createBar()), 'bar1');

Once I tried to uncomment that bit of the test, I ran into this issue and trying to trace it down, going through the generated code. What I ran into was that I described here. It was a bit ago but I tried to write enough to describe what I thought was happening.

Definitely open to ideas on what the problem could actually be here!

@guybedford

Copy link
Copy Markdown
Collaborator

A good starting point would be to uncomment that test in this PR and see if the tests are passing.

@vados-cosmonic

vados-cosmonic commented Feb 22, 2025

Copy link
Copy Markdown
Collaborator Author

Sure, that's reasonable! just pushed, will rebase to main as well (core.rs has changed)

[EDIT] BTW, this is the error I was seeing (I don't think it's changed):

     RuntimeError: unreachable
      at  (wasm://wasm/0574e2aa:wasm-function[292]:0x2a60e)
      at  (wasm://wasm/0574e2aa:wasm-function[293]:0x2a61a)
      at  (wasm://wasm/0574e2aa:wasm-function[8312]:0x47aa1f)
      at consume-bar (wasm://wasm/0574e2aa:wasm-function[24267]:0xb7db0f)
      at Module.consumeBar (file:///tmp/qodNnt/out-component-dir/componentize.js:97:38)
      at Context.<anonymous> (file:///path/to/jco/test/cli.js:564:21)

This commit attempts to fix the case of an import map that perfectly
overlaps an existing component resource.

When a resource that exists in a component is overriden with an import
map, the interface can change arbitrarily.

Signed-off-by: Victor Adossi <vadossi@cosmonic.com>
Signed-off-by: Victor Adossi <vadossi@cosmonic.com>
Signed-off-by: Victor Adossi <vadossi@cosmonic.com>
@vados-cosmonic
vados-cosmonic force-pushed the fix=import-map-resource-overlap branch from 19cf57a to 5721251 Compare February 22, 2025 02:22
@vados-cosmonic

Copy link
Copy Markdown
Collaborator Author

Created backing issue, closing to avoid distracting from other in-flight PRs

@vados-cosmonic
vados-cosmonic deleted the fix=import-map-resource-overlap branch November 4, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants