Wasm runs failing with empty WasmAssembliesToBundle about performance HOT 10 CLOSED

@caaavik-msft and I found that path used by the PrepareForWasmBuild task and the path the files are being put do not match, /AppBundle/publish/ and /AppBundlepublish/ respectively. This seems to stem from the update here: dotnet/BenchmarkDotNet@e6fdc6b#diff-77e0d627a9d7c20f6d24179d3ca8dbe29aba70b95b5f464786ee93386e63412aR262-R268 missing trailing slashes, at least for OutDir: https://learn.microsoft.com/en-us/cpp/build/reference/common-macros-for-build-commands-and-properties?view=msvc-170

from performance.

After some testing with the trailing slash added for the different properties, the build-no-restore step was able to find some of the WasmAssembliesToBundle. However, it is now hitting an error due to not being able to find the test-main.js file for testing. With the trailing slashes, this file is being put in one folder too deep, specifically into ./AppBundle/AppBundle/test-main.js instead of ./AppBundle/test-main.js, so there is a duplication to track down somewhere.

from performance.

cc @radical @timcassell

from performance.

missing trailing slashes, at least for OutDir: https://learn.microsoft.com/en-us/cpp/build/reference/common-macros-for-build-commands-and-properties?view=msvc-170

Interesting, when I implemented that change, I followed https://learn.microsoft.com/en-us/visualstudio/msbuild/common-msbuild-project-properties?view=vs-2022 which doesn't mention trailing slash (only for IntermediateOutputPath). It doesn't appear to be necessary for CoreCLR or NativeAOT builds. Do we need to conditionally add it depending on the runtime, or it would be fine to include it always? [Edit] It seems that it's fine to always have it.

from performance.

cc @lewing @akoeplinger

from performance.

With the trailing slashes, this file is being put in one folder too deep, specifically into ./AppBundle/AppBundle/test-main.js instead of ./AppBundle/test-main.js, so there is a duplication to track down somewhere.

Does the wasm build auto append AppBundle to the output path? If so, that would explain it, as previously BDN was only reading the path to get the executable, and now it's passing it to the cli which it wasn't doing before. If that's how it works, we may need to pass a different path to the cli with AppBundle removed.

from performance.

Does the wasm build auto append AppBundle to the output path? If so, that would explain it, as previously BDN was only reading the path to get the executable, and now it's passing it to the cli which it wasn't doing before. If that's how it works, we may need to pass a different path to the cli with AppBundle removed.

I don't know for sure, but that is definitely a possibility and would make sense as to why the additional AppBundle is occuring.

from performance.

Does the wasm build auto append AppBundle to the output path? If so, that would explain it, as previously BDN was only reading the path to get the executable, and now it's passing it to the cli which it wasn't doing before. If that's how it works, we may need to pass a different path to the cli with AppBundle removed.

I don't know for sure, but that is definitely a possibility and would make sense as to why the additional AppBundle is occuring.

Likely found it. In the _InitializeCommonProperties from _WasmBuildAppCore, the WasmAppDir which is used later for where to copy test-main.js to is being set with: <WasmAppDir Condition="'$(WasmAppDir)' == ''">$([MSBuild]::NormalizeDirectory($(OutputPath), 'AppBundle'))</WasmAppDir>.

from performance.

Likely found it. In the _InitializeCommonProperties from _WasmBuildAppCore, the WasmAppDir which is used later for where to copy test-main.js to is being set with: <WasmAppDir Condition="'$(WasmAppDir)' == ''">$([MSBuild]::NormalizeDirectory($(OutputPath), 'AppBundle'))</WasmAppDir>.

So what's the best fix? Do we pass output paths without AppBundle? Do we pass WasmAppDir? Something else?

from performance.

This has been fixed by dotnet/BenchmarkDotNet#2531.

from performance.