Add runtimeconfig.json support for WebAssembly #56486

fanyang-mono · 2021-07-28T18:57:08Z

Added runtimeconfig.json support for WebAssembly.
Fixes #38433

Added a test for it.
Fixes #50908

Also added a test to Wasm.Build.Tests, thanks to @radical

ghost · 2021-07-28T18:57:11Z

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

Issue Details

Author:	fanyang-mono
Assignees:	-
Labels:	`arch-wasm`
Milestone:	-

lambdageek

The embedding changes LGTM. There's a couple of unnecessary things copied over from the hot reload functional test that I don't think you need.

Not sure about the Wasm infrastructure changes. @radekdoulik or @radical could oyu review those?

...ctionalTests/WebAssembly/Browser/RuntimeConfig/WebAssembly.Browser.RuntimeConfig.Test.csproj

src/tests/FunctionalTests/WebAssembly/Browser/RuntimeConfig/runtime.js

src/mono/wasm/build/WasmApp.InTree.targets

src/mono/wasm/build/WasmApp.LocalBuild.targets

src/mono/wasm/build/WasmApp.targets

src/tests/FunctionalTests/WebAssembly/Browser/RuntimeConfig/Program.cs

...ctionalTests/WebAssembly/Browser/RuntimeConfig/WebAssembly.Browser.RuntimeConfig.Test.csproj

src/mono/wasm/build/WasmApp.LocalBuild.props

src/mono/wasm/build/WasmApp.targets

src/tests/FunctionalTests/WebAssembly/Browser/RuntimeConfig/Program.cs

...ctionalTests/WebAssembly/Browser/RuntimeConfig/WebAssembly.Browser.RuntimeConfig.Test.csproj

src/mono/wasm/build/WasmApp.targets

radical · 2021-07-29T20:49:05Z

Does this PR mean that setting environment variables like this would work? If so, then that should be included in the Wasm.Build.Tests test being added for runtimeconfig.

src/mono/wasm/build/WasmApp.targets

radical · 2021-07-29T21:46:05Z

Not for this PR, but we maybe need to handle the case when you have

{
   "runtimeOptions": {
      "configProperties": {
         "System.Globalization.Invariant": true
      }
   }
}

.. in your runtimeconfig.template.json, then the wasm targets should treat this as $(InvariantGlobalization)=true. @lewing ?

fanyang-mono · 2021-07-30T15:08:28Z

Not for this PR, but we maybe need to handle the case when you have
{
   "runtimeOptions": {
      "configProperties": {
         "System.Globalization.Invariant": true
      }
   }
}
.. in your runtimeconfig.template.json, then the wasm targets should treat this as $(InvariantGlobalization)=true. @lewing ?

My runtimeconfig.template.json is only valid for the test I added. It needs to be added for the app who needs the configuration of $(InvariantGlobalization)=true. To my understanding, runtimeconfig.template.json needs to be visible during the build/publish process.

lambdageek · 2021-07-30T17:50:16Z

Not for this PR, but we maybe need to handle the case when you have
{
   "runtimeOptions": {
      "configProperties": {
         "System.Globalization.Invariant": true
      }
   }
}
.. in your runtimeconfig.template.json, then the wasm targets should treat this as $(InvariantGlobalization)=true. @lewing ?

runtime.template.json is meant to configure the runtime behavior of the app, not how it is built/packaged. Using it to configure the build doesn't make a lot of sense to me.

radical · 2021-07-30T22:48:38Z

runtime.template.json is meant to configure the runtime behavior of the app, not how it is built/packaged. Using it to configure the build doesn't make a lot of sense to me.

What happens if you only set the "System.Globalization.Invariant": true in your runtimeconfig.template.json, in a non-wasm project? Is that the equivalent to using /p:InvariantGlobalization=true?

If so, then in case of wasm, we need to link out relevant bits, IIUC, and for that wasm targets would need to read the config file.

eng/testing/linker/trimmingTests.targets

eng/testing/tests.mobile.targets

src/mono/wasm/build/WasmApp.targets

lewing

This is a good start but doesn't look like it will work in blazor apps

src/mono/wasm/build/WasmApp.targets

fanyang-mono · 2021-08-03T20:01:07Z

Lane dotnet-linker-tests (Build windows x64 release Runtime_Release) actually passed for rerun. Don't know why it still showed up as failed. https://dev.azure.com/dnceng/public/_build/results?buildId=1272100&view=results

fanyang-mono · 2021-08-03T20:40:28Z

@radical @lewing If this change looks good to you, could you please approve it?

radical · 2021-08-03T21:59:21Z

There are some disabled tests with ActiveIssue on #38433 being fixed here, which should be enabled in this PR.

…ists

src/tests/BuildWasmApps/Wasm.Build.Tests/WasmBuildAppTest.cs

radical

Looks good, just couple of comments

src/mono/wasm/build/WasmApp.targets

This fixes the build for ApplyUpdateReferencedAssembly.csproj, which is a support project for HotReload wasm functional test. ``` D:\workspace\_work\1\s\src\mono\wasm\build\WasmApp.targets(131,5): error : Could not find D:\workspace\_work\1\s\artifacts\bin\ApplyUpdateReferencedAssembly\net6.0-Release\browser-wasm\publish\ApplyUpdateReferencedAssembly.runtimeconfig.json for D:\workspace\_work\1\s\artifacts\bin\ApplyUpdateReferencedAssembly\net6.0-Release\browser-wasm\publish\ApplyUpdateReferencedAssembly.dll. [D:\workspace\_work\1\s\src\tests\FunctionalTests\WebAssembly\Browser\HotReload\ApplyUpdateReferencedAssembly\ApplyUpdateReferencedAssembly.csproj] ``` The wasm targets should be run at all for this project. But they are run because they get imported by tests.wasm.targets, which gets imported because `$(IsTestProject)=true`. The csproj has `$(IsTestProject)=false`, and `$(IsTestSupportProject)=true`, which should mean that the test.props/targets don't get imported. But the import is conditioned on `$(EnableTestSupport)`, which gets set in `src/libraries/Directory.Build.props`. And that means setting `$(IsTestProject)=true` in the csproj is too late. So, instead, set that in a `Directory.Build.props`. And also, ensure that the `Directory.Build.props` for functional tests doesn't override the value!

radical

LGTM, thanks for your patience! 👍

fanyang-mono added the arch-wasm WebAssembly architecture label Jul 28, 2021

dotnet-issue-labeler bot added the area-VM-meta-mono label Jul 28, 2021

fanyang-mono marked this pull request as ready for review July 28, 2021 19:17

fanyang-mono requested a review from marek-safar as a code owner July 28, 2021 19:17

fanyang-mono requested review from radical, lewing and lambdageek July 28, 2021 19:18

radical added this to the 6.0.0 milestone Jul 28, 2021

lambdageek reviewed Jul 28, 2021

View reviewed changes

radical reviewed Jul 29, 2021

View reviewed changes

karelz mentioned this pull request Jul 29, 2021

Connect_DualMode_DnsConnect_RetrievedEndPoints_Success - SocketException: Transport endpoint is not connected #55709

Closed

radical reviewed Jul 29, 2021

View reviewed changes

...ctionalTests/WebAssembly/Browser/RuntimeConfig/WebAssembly.Browser.RuntimeConfig.Test.csproj Outdated Show resolved Hide resolved

radical reviewed Jul 29, 2021

View reviewed changes

src/mono/wasm/build/WasmApp.targets Show resolved Hide resolved

radical reviewed Jul 29, 2021

View reviewed changes

src/mono/wasm/build/WasmApp.targets Outdated Show resolved Hide resolved

radical reviewed Jul 31, 2021

View reviewed changes

eng/testing/linker/trimmingTests.targets Show resolved Hide resolved

radical reviewed Jul 31, 2021

View reviewed changes

eng/testing/tests.mobile.targets Outdated Show resolved Hide resolved

radical reviewed Jul 31, 2021

View reviewed changes

src/mono/wasm/build/WasmApp.targets Outdated Show resolved Hide resolved

lewing reviewed Aug 2, 2021

View reviewed changes

src/mono/wasm/build/WasmApp.targets Show resolved Hide resolved

radical self-requested a review August 3, 2021 21:00

radical mentioned this pull request Aug 3, 2021

Add a test for runtimeconfig.json to Wasm.Build.Tests #56799

Closed

Initial change for adding runtimeconfig to wasm

6bb8c65

fanyang-mono and others added 9 commits August 4, 2021 17:37

Update runtimeconfig file path

e147641

Add RuntimeConfigParserTasksAssemblyPath to linker test project template

9d2f53c

Fix test failures on WASM EAT and AOT lanes

b7bc84c

[wasm] Add a test for reading runtimeconfig.json

e8bfc59

improve error reporting slightly

2cfc3de

Enable tests

034887c

One more thing to enable tests

de4284a

Run target _WasmGenerateRuntimeConfig when runtimeconfig.json file ex…

f0b1fde

…ists

Add a test for configure runtime using msbuild properties

e284581

fanyang-mono force-pushed the wasm_runtimeconfig branch from 16efe2f to e284581 Compare August 4, 2021 21:42

Fix runtime test failures

bef52de

fanyang-mono mentioned this pull request Aug 5, 2021

Adopt runtime settings from runtimeconfig.json in Blazor dotnet/aspnetcore#35010

Closed

radical reviewed Aug 5, 2021

View reviewed changes

src/tests/BuildWasmApps/Wasm.Build.Tests/WasmBuildAppTest.cs Outdated Show resolved Hide resolved

fanyang-mono added 2 commits August 5, 2021 14:27

Update tests base on feedback

9bcb10a

Add missing ")"

ee244ba

radical requested changes Aug 5, 2021

View reviewed changes

src/mono/wasm/build/WasmApp.targets Outdated Show resolved Hide resolved

src/mono/wasm/build/WasmApp.targets Outdated Show resolved Hide resolved

fanyang-mono and others added 4 commits August 5, 2021 20:02

Add one more condition

c909ef2

More format changes

c0a7981

Better error handling

29818f5

radical self-requested a review August 6, 2021 05:01

radical approved these changes Aug 6, 2021

View reviewed changes

fanyang-mono merged commit 13f8065 into dotnet:main Aug 6, 2021

karelz mentioned this pull request Aug 10, 2021

Test failure: System.Net.Quic.Tests.QuicStreamTests_MsQuicProvider.BasicTest #56269

Closed

steveisok mentioned this pull request Aug 10, 2021

Disable System.Drawing.Common tests for mobile workloads and skip a few other tests #57114

Merged

danmoseley mentioned this pull request Aug 12, 2021

PhysicalFileProvider: Avoid NullRef if ResolveLinkTarget returns null #57235

Merged

This was referenced Aug 17, 2021

OSX machines are de-provisioned during CI / PR runs leading to failures #34472

Closed

RunContinueWithStressTestsNoState timing out in CI #2271

Closed

karelz mentioned this pull request Sep 3, 2021

Test failure: ConnectAsync_CancellationRequestedAfterConnect_ThrowsOperationCanceledException #56688

Closed

ghost locked as resolved and limited conversation to collaborators Sep 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add runtimeconfig.json support for WebAssembly #56486

Add runtimeconfig.json support for WebAssembly #56486

fanyang-mono commented Jul 28, 2021 •

edited

Loading

ghost commented Jul 28, 2021

lambdageek left a comment

radical commented Jul 29, 2021

radical commented Jul 29, 2021

fanyang-mono commented Jul 30, 2021

lambdageek commented Jul 30, 2021 •

edited

Loading

radical commented Jul 30, 2021

lewing left a comment

fanyang-mono commented Aug 3, 2021

fanyang-mono commented Aug 3, 2021

radical commented Aug 3, 2021

radical left a comment

radical left a comment

Add runtimeconfig.json support for WebAssembly #56486

Add runtimeconfig.json support for WebAssembly #56486

Conversation

fanyang-mono commented Jul 28, 2021 • edited Loading

ghost commented Jul 28, 2021

lambdageek left a comment

Choose a reason for hiding this comment

radical commented Jul 29, 2021

radical commented Jul 29, 2021

fanyang-mono commented Jul 30, 2021

lambdageek commented Jul 30, 2021 • edited Loading

radical commented Jul 30, 2021

lewing left a comment

Choose a reason for hiding this comment

fanyang-mono commented Aug 3, 2021

fanyang-mono commented Aug 3, 2021

radical commented Aug 3, 2021

radical left a comment

Choose a reason for hiding this comment

radical left a comment

Choose a reason for hiding this comment

fanyang-mono commented Jul 28, 2021 •

edited

Loading

lambdageek commented Jul 30, 2021 •

edited

Loading