[wasm/wasi] Consolidate build targets - #95775

Merged
lewing merged 9 commits into
dotnet:mainfrom
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged

[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom
radical:wasm-build-consolidate-targets

Conversation

@radical

@radicalradical commented Dec 8, 2023•
edited
Loading

Copy link
Copy Markdown
Member

[wasm/wasi] Consolidate build targets

Current state of build files:

 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.

This commit consolidates these into:

 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}

This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.

  • The top level target remains the same - WasmBuildApp.

  • There are a few "public" targets that can be hooked into:

    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet

  • all these public targets have corresponding *DependsOn properties
    which can be used for extending the build

note: this commit does not add a public target for AOT, but it might be
added in future.

WasmApp.{props,targets}

This is for browser-wasm projects. The file might be renamed in
future.

WasiApp.{props,targets}

This is for wasi-wasm projects. ILStrip becomes usable as a feature
for wasi-wasm because of this consolidation.

@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical added the arch-wasm WebAssembly architecture label Dec 8, 2023
@ghostghost assigned radicalDec 8, 2023
@ghostghost added the area-Build-mono label Dec 8, 2023
@ghost

ghost commented Dec 8, 2023

Copy link
Copy Markdown

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

Issue Details

[wasm/wasi] Consolidate build targets

Current state of build files:

 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical

radical commented Dec 8, 2023

Copy link
Copy Markdown
MemberAuthor

dotnet/emsdk#621 is needed for this.

@radicalradical mentioned this pull request Dec 8, 2023
Merged
@radical

radical commented Dec 8, 2023

Copy link
Copy Markdown
MemberAuthor

This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.

@radical

radical commented Dec 8, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

TODO before merge:

TODO for follow up PRs:

  • add wasi-sdk version check
  • runtime tests - move to using LocalBuild instead of InTree targets

radical
radical commented Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical

radical commented Dec 8, 2023

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Dec 8, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@radical
radical marked this pull request as ready for review December 8, 2023 03:47
This was referenced Dec 8, 2023
Open
Closed
pavelsavara
pavelsavara reviewed Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
<BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

<BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
<!-- FIXME: what would be the correct place to do this? -->

@pavelsavarapavelsavaraDec 8, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?

@radicalradicalDec 8, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.

@build-analysisbuild-analysisBot mentioned this pull request Dec 8, 2023
Closed
@radical

radical commented Dec 8, 2023

Copy link
Copy Markdown
MemberAuthor

For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.
For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).

@pavelsavarapavelsavara mentioned this pull request Dec 10, 2023
Merged
@pavelsavarapavelsavara mentioned this pull request Dec 11, 2023
Merged
ilonatommy
ilonatommy approved these changes Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets Outdated
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz

vargaz commented Dec 11, 2023

Copy link
Copy Markdown
Contributor

Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834

fanyang-mono
fanyang-mono approved these changes Dec 11, 2023
View reviewed changes

@fanyang-monofanyang-mono left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ILStrip test looks good to me.

Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs Outdated
lewing
lewing approved these changes Dec 11, 2023
View reviewed changes

@lewinglewing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds

@lewing
lewing merged commit a128c15 into dotnet:mainDec 11, 2023
@radical
radical deleted the wasm-build-consolidate-targets branch December 11, 2023 20:46
@radicalradical mentioned this pull request Dec 11, 2023
Closed
radical added a commit to radical/runtime that referenced this pull request Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit that referenced this pull request Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
[wasm/wasi] Consolidate build targets - #95775
Merged
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Dec 11, 2023
Merged
[wasm/wasi] Consolidate build targets#95775
lewing merged 9 commits into
dotnet:mainfrom 
radical:wasm-build-consolidate-targets
Conversation
@radical
@radicalradical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

Member
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets

The wasm, and wasi build have lot of shared code but that is not

representative in the actual files, since the wasi targets started life

as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
 common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets

WasmApp.Common.{props,targets}
This has all the common parts of the build for browser-wasm, and wasi,

and includes bits from WasmApp.{props,targets}, and
WasmApp.Native.{props,targets}.
  • The top level target remains the same - WasmBuildApp.
  • There are a few "public" targets that can be hooked into:
    • PrepareInputsForWasmBuild
    • WasmGenerateAppBundle
    • PrepareForWasmBuildNative
    • WasmLinkDotNet
  • all these public targets have corresponding *DependsOn properties
    
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be

added in future.
WasmApp.{props,targets}
This is for browser-wasm projects. The file might be renamed in

future.
WasiApp.{props,targets}
This is for wasi-wasm projects. ILStrip becomes usable as a feature

for wasi-wasm because of this consolidation.
@radical
[wasm/wasi] Consolidate build targets
be9a2d7
Current state of build files:
```
wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:
```
common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets
```
## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
@radical
[wasi] WBT: Use TestOutputWrapper
74ee7cd
@radical
[wasm] WBT: Track target name changes
0f72775
@radical
[wasi] WBT: Add new tests
c6408c3
@radical
cleanup
25ab062
@radicalradical
added
the 
arch-wasm
WebAssembly architecture
label
Dec 8, 2023
@ghostghost
assigned radicalDec 8, 2023
@ghostghost
added
the 
area-Build-mono

label
Dec 8, 2023
@ghost
ghost
commented
Dec 8, 2023

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
[wasm/wasi] Consolidate build targets
Current state of build files:
 wasm: WasmApp.props, WasmApp.targets, WasmApp.Native.targets
wasi: WasiApp.props, WasiApp.targets, WasiApp.Native.targets
``
The wasm, and wasi build have lot of shared code but that is not
representative in the actual files, since the wasi targets started life
as a quick-copy-comment-out-bits of the wasm targets.
This commit consolidates these into:

common: WasmApp.Common.props, WasmApp.Common.targets
wasm : WasmApp.props, WasmApp.targets
wasi : WasiApp.props, WasiApp.targets


## `WasmApp.Common.{props,targets}`
This has all the common parts of the build for browser-wasm, and wasi,
and includes bits from `WasmApp.{props,targets}`, and
`WasmApp.Native.{props,targets}`.
- The top level target remains the same - `WasmBuildApp`.
- There are a few "public" targets that can be hooked into:
- `PrepareInputsForWasmBuild`
- `WasmGenerateAppBundle`
- `PrepareForWasmBuildNative`
- `WasmLinkDotNet`
- all these public targets have corresponding `*DependsOn` properties
which can be used for extending the build
note: this commit does not add a public target for AOT, but it might be
added in future.
## WasmApp.{props,targets}
This is for `browser-wasm` projects. The file might be renamed in
future.
## WasiApp.{props,targets}
This is for `wasi-wasm` projects. `ILStrip` becomes usable as a feature
for `wasi-wasm` because of this consolidation.
<table>
<tr>
<th align="left">Author:</th>
<td>radical</td>
</tr>
<tr>
<th align="left">Assignees:</th>
<td>-</td>
</tr>
<tr>
<th align="left">Labels:</th>
<td>
`arch-wasm`
</td>
</tr>
<tr>
<th align="left">Milestone:</th>
<td>-</td>
</tr>
</table>
</details>

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
dotnet/emsdk#621 is needed for this.
@radicalradical
mentioned this pull request
Dec 8, 2023
 Merged

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
This is admittedly kinda hard to review, so I'm depending on the tests quite a bit. This does not change core specifics of the build, but moves around things, different public targets, and dependencies.
@radical
radical
commented
Dec 8, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
TODO before merge:
TODO for follow up PRs:
  •  add wasi-sdk version check
  •  runtime tests - move to using LocalBuild instead of InTree targets
radical
radical
commented
Dec 8, 2023
View reviewed changes
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Dec 8, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@radical
radical
marked this pull request as ready for review
December 8, 2023 03:47

This was referenced Dec 8, 2023

Open


Closed

pavelsavara
pavelsavara
reviewed
Dec 8, 2023
View reviewed changes
Comment threadeng/testing/tests.wasm.targets
 <BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" />

 <BundleFiles Include="$(WasmSharedPath)data\aot-tests\*" TargetDir="publish" />
 <!-- FIXME: what would be the correct place to do this? -->
@pavelsavarapavelsavaraDec 8, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we rename wasm -> browser (OS) and use wasm for the (architecture) ?
@radicalradicalDec 8, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Yeah, I avoided those renames in this PR. We are following the pattern you suggested in most places now, like tests.{wasm,browser,wasi}.targets, and recently sendtohelix-{wasm,browser,wasi}.proj.
@build-analysisbuild-analysisBot
mentioned this pull request
Dec 8, 2023

Closed

@radical
radical
commented
Dec 8, 2023

Copy link


Copy Markdown

MemberAuthor
For review, for the wasm case I think we have pretty good test coverage to be reasonably confident. There will be likely be some cases missed, or small regressions, which we can be fixed in follow up PRs.

For wasi, we have limited build support right now anyway, and some of that is covered by tests, and this PR makes it possible to have the same kinda features as the wasm build - for example, ilstrip, singlefilebundle (tbd).
@pavelsavarapavelsavara
mentioned this pull request
Dec 10, 2023
 Merged

@pavelsavarapavelsavara
mentioned this pull request
Dec 11, 2023
 Merged

ilonatommy
ilonatommy
approved these changes
Dec 11, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
Outdated

Comment threadsrc/mono/wasm/build/WasmApp.Common.targets
@vargaz
vargaz
commented
Dec 11, 2023

Copy link


Copy Markdown

Contributor
Would be nice to review/merge this quickly so it doesn't conflict with other PRs, i.e.
#95834
fanyang-mono
fanyang-mono
approved these changes
Dec 11, 2023
View reviewed changes
@fanyang-monofanyang-mono
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

ILStrip test looks good to me.
Comment threadsrc/mono/wasi/Wasi.Build.Tests/ILStripTests.cs
Outdated

lewing
lewing
approved these changes
Dec 11, 2023
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Looks ready to merge then we will need to keep an eye on the builds
@lewing
lewing
merged commit a128c15
into
dotnet:mainDec 11, 2023
@radical
radical
deleted the

wasm-build-consolidate-targets

branch
December 11, 2023 20:46
@radicalradical
mentioned this pull request
Dec 11, 2023

Closed

radical added a commit
to radical/runtime
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression
b72c3ae
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (dotnet#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (dotnet#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
radical added a commit
that referenced
this pull request
Dec 14, 2023
@radical
[wasm/wasi] Fix size regression (#95992)
f59ef7f
* [wasm/wasi] Fix size regression
Blazor size regression was fixed by:
```
commit ec31705
Author: Ankit Jain <radical@gmail.com>
Date: Wed Dec 6 05:36:59 2023 -0500
[wasm] Fix regressed file sizes for blazor (#92664)
```
.. but a subsequent PR created close to that undid some of the changes:
```
commit a128c15
Author: Ankit Jain <radical@gmail.com>
Date: Mon Dec 11 15:45:58 2023 -0500
[wasm/wasi] Consolidate build targets (#95775)
```
Essentially, `-g` was being passed to the link, and compile-bc steps.
Found in dotnet/perf-autofiling-issues#25891 .
* [wasm] Add flag missed in the consolidate PR
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jan 11, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@fanyang-monofanyang-monofanyang-mono approved these changes
@lewinglewinglewing approved these changes
@ilonatommyilonatommyilonatommy approved these changes
@marafmarafAwaiting requested review from marafmaraf is a code owner
@kgkgAwaiting requested review from kg
@marek-safarmarek-safarAwaiting requested review from marek-safar

Assignees

@radicalradical

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@radical@vargaz@lewing@pavelsavara@ilonatommy@fanyang-mono