Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[wasm] Don't use _WasmDevel=true when publishing #87544

Merged
merged 7 commits into from
Jun 15, 2023
Merged

[wasm] Don't use _WasmDevel=true when publishing #87544

merged 7 commits into from
Jun 15, 2023
+34 −22

Conversation

radical
Copy link
Member

@radical radical commented Jun 14, 2023 •
edited
Loading

_WasmDevel=true causes the optimization flags to be -O0.

  • This was automatically set whenever Configuration=Debug, and
    WasmBuildNative=true, which is useful when relinking in inner loop.
  • But when publishing, and relinking/AOT, -O0 results in larger output.
  • Instead, this is changed to never use _WasmDevel when publishing.

[wasm] host: Add --no-silent command line parameter

  • this is the equivalent to --verbose
  • The default is "silent" behavior

Cannot use --verbose because that conflicts with dotnet run.

Also:

  • CI: Trigger only wasm jobs on workload-testing.targets changes, since only wasm is using this currently
  • Add missing Wasm.Build.Tests.Blazor.AppsettingsTests to list of tests to run

Fixes #87414 - side effect of the _WasmDevel change.

radical added 5 commits June 14, 2023 05:59
@radical
[wasm] Add missing Wasm.Build.Tests.Blazor.AppsettingsTests to list o…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
e08bb64 
…f tests to run
@radical
[wasm] CI: Trigger only wasm jobs on workload-testing.targets changes…
ba5fd17 
…, since only wasm is using this currently
@radical
[wasm] host: Add --no-silent command line parameter
c61deb0 
- this is the equivalent to `--verbose`
- The default is "silent" behavior

Cannot use `--verbose` because that conflicts with `dotnet run`.
@radical
[wasm] WBT: Update template tests to use --no-silent with dotnet run
76c2062 
.. which would be useful in debugging tests.
@radical
[wasm] Don't use _WasmDevel=true whenever publishing
d440063 
`_WasmDevel=true` causes the optimization flags to be `-O0`.

- This *was* automatically set whenever `Configuration=Debug`, and
`WasmBuildNative=true`, which is useful when relinking in inner loop.
- But when publishing, and relinking/AOT, `-O0` results in larger output.
@radical radical requested review from vargaz, maraf and ilonatommy June 14, 2023 06:13
@ghost ghost assigned radical Jun 14, 2023
@radical radical added the arch-wasm WebAssembly architecture label Jun 14, 2023
@radical radical added this to the 8.0.0 milestone Jun 14, 2023
@ghost
Copy link

ghost commented Jun 14, 2023

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

Issue Details

_WasmDevel=true causes the optimization flags to be -O0.

  • This was automatically set whenever Configuration=Debug, and
    WasmBuildNative=true, which is useful when relinking in inner loop.
  • But when publishing, and relinking/AOT, -O0 results in larger output.

[wasm] host: Add --no-silent command line parameter

  • this is the equivalent to --verbose
  • The default is "silent" behavior

Cannot use --verbose because that conflicts with dotnet run.

Also:

  • CI: Trigger only wasm jobs on workload-testing.targets changes, since only wasm is using this currently
  • Add missing Wasm.Build.Tests.Blazor.AppsettingsTests to list of tests to run
Author: radical
Assignees: radical
Labels:

arch-wasm, area-Build-mono
Milestone: -

@radical
Copy link
Member Author

radical commented Jun 14, 2023

The _WasmDevel change also fixes #87414 .

@radical radical requested a review from steveisok June 14, 2023 06:15
@radical
Copy link
Member Author

radical commented Jun 14, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member

perhaps --verbose-publish or --verbose-build or --verbose-aot or --verbose-compilation

ilonatommy
ilonatommy approved these changes Jun 14, 2023
View reviewed changes
@radical
Copy link
Member Author

radical commented Jun 14, 2023

perhaps --verbose-publish or --verbose-build or --verbose-aot or --verbose-compilation

This is for dotnet run, so none of these would be relevant. I don't particularly like --no-silent though, but don't have a good alternative right now. OTOH, --no-* does have precedence like --no-build, --no-restore etc for dotnet run.

@pavelsavara
Copy link
Member

This is for dotnet run, so none of these would be relevant.

Could you please explain what are we making verbose ?

@radical
Copy link
Member Author

radical commented Jun 14, 2023

This is for dotnet run, so none of these would be relevant.

Could you please explain what are we making verbose ?

Not much right now. The js engine exit code for example, will be shown in verbose, and the js engine invocation command line.

@radical
Copy link
Member Author

radical commented Jun 15, 2023

@pavelsavara I'm merging this to get CI green. If we want to change the parameter name, then I can handle that in a follow up PR.

@radical radical merged commit 226e3ec into dotnet:main Jun 15, 2023
@radical radical deleted the wbt-wasm-devel branch June 15, 2023 01:24
@ghost ghost locked as resolved and limited conversation to collaborators Jul 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ilonatommy ilonatommy

@vargaz vargaz

@maraf maraf

@lewing lewing

@pavelsavara pavelsavara

@kg kg

@steveisok steveisok

Assignees

@radical radical

Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

WasmTemplateTests.ConsolePublishAndRun fails with exit code 137
3 participants
@radical @pavelsavara @ilonatommy