Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
) # Description The `LibraryMode` test variant with `useWasmSdk: False, isPublish: True` is expected to fail, but the test was asserting success for all variants. Updated test to set `ExpectSuccess: false` when publishing without WASM SDK, verifying the operation exits with code 1 as expected. ```csharp // Publishing without WASM SDK is expected to fail bool expectSuccess = useWasmSdk || !isPublish; if (isPublish) PublishProject(info, config, new PublishOptions(AssertAppBundle: useWasmSdk, ExpectSuccess: expectSuccess)); else BuildProject(info, config, new BuildOptions(AssertAppBundle: useWasmSdk, ExpectSuccess: expectSuccess)); ``` # Customer Impact No customer impact - test-only change. # Regression No. Test was incomplete, not validating the expected failure scenario. # Testing Test compiles successfully. The updated test will properly validate all four variants: - Build without SDK: succeeds - Build with SDK: succeeds - Publish with SDK: succeeds - Publish without SDK: **fails** (now correctly validated) # Risk Minimal - test-only change with no production code impact. <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: maraf <[email protected]>
|
The problem is when a library has |
This fixes an issue where BenchmarkDotNet-generated WASM benchmark projects stopped working after commit 34e2468 removed the _UsingBlazorOrWasmSdk condition from _WasmNativeWorkloadNeeded. The change in that commit was intended to allow library projects with RuntimeIdentifier=browser-wasm to build without requiring the WASM workload (per dotnet/runtime#122607). However, the change was too broad - it affected ALL non-Blazor/WasmSdk projects, including Exe projects that need the workload for runtime pack resolution. This fix adds back the workload requirement specifically for Exe projects that don't use Blazor/WasmSdk, while preserving the library-mode-without-workload behavior. Changes: - WorkloadManifest.targets.in: Added condition to set _WasmNativeWorkloadNeeded for Exe projects without BlazorOrWasmSdk - Added regression tests to verify: - Exe projects trigger workload usage (UsingBrowserRuntimeWorkload=true) - Library projects don't force workload usage - Runtime pack version is available when workload is used - Added test asset WasmConsoleApp for the test scenarios Fixes the 'MicrosoftNetCoreAppRuntimePackDir is empty' error in WASM benchmarks.
This comment has been minimized.
This comment has been minimized.
javiercn
left a comment
There was a problem hiding this comment.
LGTM, but I might be missing some context
| <!-- When trimming non-exe projects, root the whole intermediate assembly. --> | ||
| <Target Name="_RootEntireIntermediateAssembly" AfterTargets="PrepareForILLink" BeforeTargets="_RunILLink" Condition="'$(RuntimeIdentifier)' == 'browser-wasm' And '$(OutputType)' == 'Library'"> | ||
| <ItemGroup> | ||
| <TrimmerRootAssembly Condition="'%(Identity)' == '$(AssemblyName)'"> |
There was a problem hiding this comment.
Opt out per assembly would be good. Maybe existing <IsTrimmable>true</IsTrimmable> ?
There was a problem hiding this comment.
Tell me about the scenario please
There was a problem hiding this comment.
This target simulates the default behavior for OutputType=exe => "application assembly is root assembly for trimmer"
There was a problem hiding this comment.
This is not about components in a nuget ? This is "reactor" runtime+app without Main() ?
Isn't that already covered by [JSExport] being a trimmer root ?
|
How do you handle exports and module imports (WasmImportLinkageAttribute) from managed code without the the workload? |
|
@lewing This is browser, not wasi, and the scenario "app without main method". We can make the workload required for wasi |
|
Those aren't actually wasi specific |
|
It's the same problem we have with UCO or LibraryImport. Until the user explicitly ask for native relink, they won't work. What am I missing? |
Building a
RID=browser-wasmlibrary (OutputType=Library) should not require the wasm workload to be installed, nor should it forceSelfContainedorPublishTrimmed.Changes
_RootEntireIntermediateAssemblyILLink target from workload (BrowserWasmApp.targets) to the Wasm SDK (Microsoft.NET.Sdk.WebAssembly.Browser.targets), so library-mode trimming configuration is available without the workload.SelfContained=truedefault fromWorkloadManifest.targets.inforbrowser-wasm— a library project shouldn't be forced into self-contained mode.PublishTrimmed=truedefault from workload — set by Wasm SDK._UsingBlazorOrWasmSdkgate from_WasmNativeWorkloadNeeded— the workload should not be required solely because the project setRID=browser-wasm.UseMonoRuntimefromLazyLibrary.csprojtest asset (no longer needed).LibraryModetests from 1 case to 4 (useWasmSdk×isPublishmatrix), all taggedno-workload, covering build and publish with and without the Wasm SDK.Fixes #123477