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] Add support for native relinking after Build, and AOT after publish #57556

Closed
wants to merge 24 commits into from

Conversation

radical
Copy link
Member

@radical radical commented Aug 17, 2021

Wasm app build can run in two scenarios:

  1. For build, eg. when running the app in VS, or dotnet build foo.csproj
  2. For Publish, eg, when publishing the app to a folder, or Azure

A dotnet wasm app has some native wasm files (dotnet.wasm, and dotnet.js). How these files are obtained, or generated:

  • Build

    • with no native libraries referenced (AOT setting is ignored here)
      • files from the runtime pack are used as-is
    • with native libraries referenced
      • dotnet.wasm is relinked with the native libraries
  • Publish

    • dotnet.wasm is relinked with the native libraries, and updated pinvoke/icalls from the trimmed assemblies
    • if RunAOTCompilation=true, then the relinking includes AOT'ed assemblies

Build

Implementation:

  • Target WasmBuildApp

  • runs after Build by default

    • which can be disabled by $(DisableAutoWasmBuildApp)
    • or the run-after target can be set via $(WasmBuildAppAfterThisTarget)
  • To run a custom target

    • before any of the wasm build targets, use $(WasmBuildAppDependsOn), and prepend your target name to that
    • after any of the wasm build targets, use AfterTargets="WasmBuildApp" on that target
  • Avoid depending on this target, because it is available only when the workload is installed. Use $(WasmNativeWorkload) to check if it is installed.

Publish

Implementation:

  • This part runs as a nested build using a MSBuild task, which means that the project gets reevaluated. So, if there were any changes made to items/properties in targets before this, then they won't be visible in the nested build.

  • By default WasmTriggerPublishApp runs after the Publish target, and that triggers the nested build

    • The nested build runs WasmNestedPublishApp, which causes Build, and Publish targets to be run
    • Because this causes Build to be run again, if you have any targets that get triggered by that, then they will be running twice.
      • But the original build run, and this publish run can be differentiated using $(WasmBuildingForPublish)
  • WasmTriggerPublishApp essentially just invokes the nested publish

    • This runs after Publish

      • which can be disabled by $(DisableAutoWasmPublishApp)
      • or the run-after target can be set via $(WasmTriggerPublishAppAfterThisTarget)
    • To influence the wasm build for publish, use WasmNestedPublishApp

      • To run a custom target before it, use $(WasmNestedPublishAppDependsOn)
      • to run a custom target after it, use AfterTargets="WasmNestedPublishApp"
    • If you want to dependsOn on this, then use DependsOnTargets="WasmTriggerPublishApp"

Fixes #56783, and #53612

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@radical radical added the arch-wasm WebAssembly architecture label Aug 17, 2021
@ghost
Copy link

ghost commented Aug 17, 2021

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

Issue Details

!!WIP!!

Based on the incremental build PR

Author: radical
Assignees: -
Labels:

arch-wasm

Milestone: -

@radical
Copy link
Member Author

radical commented Aug 19, 2021

/azp run runtime,runtime-staging

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ghost
Copy link

ghost commented Aug 23, 2021

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

!!WIP!!

Based on the incremental build PR

Author: radical
Assignees: -
Labels:

arch-wasm, area-Infrastructure-mono

Milestone: -

@radical radical marked this pull request as ready for review September 8, 2021 00:12
radical added a commit to radical/sdk that referenced this pull request Sep 8, 2021
[This PR](dotnet/runtime#57556) adds support for
relinking the native wasm files during the build (instead of only during
publish).

This required changes to the targets available, and how they can be
used, and thus require changes in the blazor targets to work with that.

The new setup is explained in
https://github.com/radical/runtime/blob/wasm-build/src/mono/wasm/build/README.md
.
@radical
Copy link
Member Author

radical commented Sep 8, 2021

1 failing test on wasm/windows is #58812

Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@radical
Copy link
Member Author

radical commented Sep 9, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

@radical backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: rebased
Applying: disable non-wasm builds
Applying: Fix helix work item name
Applying: fix test
Applying: get all the assemblies from the publishdir
Applying: AOTCompiler: Expand the paths used with MONO_PATH
Applying: Implement support for skipping assemblies for AOT
Applying: Update blazorwasm targets file with a locally patched copy
Applying: Cleanup
Applying: fix path for copying blazorwasm targets file
Applying: Fix tests on emsdk
Applying: Don't fixup runtime pack paths when not using workloads
Applying: update platform-matrix
error: sha1 information is lacking or useless (eng/pipelines/common/platform-matrix.yml).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0013 update platform-matrix
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

radical added a commit to radical/sdk that referenced this pull request Sep 10, 2021
[This PR](dotnet/runtime#57556) adds support for
relinking the native wasm files during the build (instead of only during
publish).

This required changes to the targets available, and how they can be
used, and thus require changes in the blazor targets to work with that.

The new setup is explained in
https://github.com/radical/runtime/blob/wasm-build/src/mono/wasm/build/README.md .
radical added a commit to radical/sdk that referenced this pull request Sep 10, 2021
[This PR](dotnet/runtime#57556) adds support for
relinking the native wasm files during the build (instead of only during
publish).

This required changes to the targets available, and how they can be
used, and thus require changes in the blazor targets to work with that.

The new setup is explained in
https://github.com/radical/runtime/blob/wasm-build/src/mono/wasm/build/README.md .
@radical
Copy link
Member Author

radical commented Sep 10, 2021

Marking this as dont-merge for now, in favor of #58913 which combines this, and #58306 .

@radical radical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 10, 2021
@radical
Copy link
Member Author

radical commented Sep 16, 2021

Merged in #58913 .

@radical radical closed this Sep 16, 2021
@radical radical deleted the wasm-build branch September 16, 2021 14:58
radical added a commit to dotnet/sdk that referenced this pull request Sep 23, 2021
[This PR](dotnet/runtime#57556) adds support for
relinking the native wasm files during the build (instead of only during
publish).

This required changes to the targets available, and how they can be
used, and thus require changes in the blazor targets to work with that.

The new setup is explained in
https://github.com/radical/runtime/blob/wasm-build/src/mono/wasm/build/README.md .

Forward ports the changes from release/6.0.1xx
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[net6.0][BlazorWasmSdk] build does not link native files
5 participants