Skip to content
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

[wasm] Changing DllImport does not result in a new wasm module #95192

Closed
silesmo opened this issue Nov 24, 2023 · 2 comments · Fixed by #95307
Closed

[wasm] Changing DllImport does not result in a new wasm module #95192

silesmo opened this issue Nov 24, 2023 · 2 comments · Fixed by #95307
Assignees
Labels
arch-wasm WebAssembly architecture area-Interop-mono os-wasi Related to WASI variant of arch-wasm
Milestone

Comments

@silesmo
Copy link

silesmo commented Nov 24, 2023

Description

Rebuilding a project targeting the wasi-experimental workload when changing the arguments in the DllImport attribute doesn't result in a new wasm module to be built.

I have built this tool that you can use to see all the imports and exports easily directly in the browser, that can hopefully assist when checking the produced wasm: https://wa2.dev/

@lewing

Reproduction Steps

Build module using nightly targeting wasi-experimental workload

Example function:

    internal static class Float32ParamInterop
    {
        [WasmImportLinkage]
        [DllImport("foo", EntryPoint = "float32-param")]
        internal static extern void wasmImportFloat32Param(float p0);
    }

Then change the EntryPoint to "float32-param-test":

    internal static class Float32ParamInterop
    {
        [WasmImportLinkage]
        [DllImport("foo", EntryPoint = "float32-param-test")]
        internal static extern void wasmImportFloat32Param(float p0);
    }

Rebuild using dotnet build -c Release
Observe that the import name hasn't changed.

If I delete the bin folder and run dotnet build -c Release then all the files except the wasm module is created in bin/Release/net9.o/wasi-wasm/AppBundle
image

It's not until I delete the obj folder that it actually builds and produces the new wasm module as expected.

Expected behavior

That the module is rebuilt and produces a new wasm module with the correct import module and function name.

Actual behavior

Doesn't produce a new wasm module without deleting the obj folder.

Regression?

Related to: #94615

Known Workarounds

Delete obj folder and rebuild.

Configuration

No response

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 24, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 24, 2023
@jkotas jkotas added the arch-wasm WebAssembly architecture label Nov 24, 2023
@ghost
Copy link

ghost commented Nov 24, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Rebuilding a project targeting the wasi-experimental workload when changing the arguments in the DllImport attribute doesn't result in a new wasm module to be built.

I have built this tool that you can use to see all the imports and exports easily directly in the browser, that can hopefully assist when checking the produced wasm: https://wa2.dev/

@lewing

Reproduction Steps

Build module using nightly targeting wasi-experimental workload

Example function:

    internal static class Float32ParamInterop
    {
        [WasmImportLinkage]
        [DllImport("foo", EntryPoint = "float32-param")]
        internal static extern void wasmImportFloat32Param(float p0);
    }

Then change the EntryPoint to "float32-param-test":

    internal static class Float32ParamInterop
    {
        [WasmImportLinkage]
        [DllImport("foo", EntryPoint = "float32-param-test")]
        internal static extern void wasmImportFloat32Param(float p0);
    }

Rebuild using dotnet build -c Release
Observe that the import name hasn't changed.

If I delete the bin folder and run dotnet build -c Release then all the files except the wasm module is created in bin/Release/net9.o/wasi-wasm/AppBundle
image

It's not until I delete the obj folder that it actually builds and produces the new wasm module as expected.

Expected behavior

That the module is rebuilt and produces a new wasm module with the correct import module and function name.

Actual behavior

Doesn't produce a new wasm module without deleting the obj folder.

Regression?

Related to: #94615

Known Workarounds

Delete obj folder and rebuild.

Configuration

No response

Other information

No response

Author: silesmo
Assignees: -
Labels:

arch-wasm, untriaged, needs-area-label

Milestone: -

@jkotas jkotas added os-wasi Related to WASI variant of arch-wasm area-Interop-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 24, 2023
@lewing
Copy link
Member

lewing commented Nov 25, 2023

Sounds like something isn't doing incremental builds correctly for the table generator correctly right now in the wasi targets, @radical can you take a look. You if you haven't already, please try adding <WasmBuildNative>true</WasmBuildNative>, it's possible that will resolve the issue but I haven't tested it.

@silesmo thanks for the report, we ceased wasi work somewhat abruptly last year and the targets still have some very rough edges that we've mostly forgotten about since. Apologies for the mess and thank you for helping us find the problems. Do you have a sample csproj? I can probably offer some suggestions (dotnet publish and WasmSingleFileBundle=true) to make things smoother while we clean things up.

@lewing lewing added this to the 9.0.0 milestone Nov 25, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 25, 2023
radical added a commit to radical/runtime that referenced this issue Nov 28, 2023
This follows the convention from `WasmApp.*targets` of writing to `.rsp`
files, and then adding them to the dependencies to trigger native
relinking.

Fixes dotnet#95192 .
Fixes dotnet#95273 .
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 28, 2023
lewing pushed a commit that referenced this issue Nov 28, 2023
This follows the convention from `WasmApp.*targets` of writing to `.rsp`
files, and then adding them to the dependencies to trigger native
relinking.

Fixes #95192 .
Fixes #95273 .
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 28, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Interop-mono os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants