[wasm] Add support for using emscripten from packages by radical · Pull Request #50077 · dotnet/runtime

radical · 2021-03-23T04:39:39Z

When using a workload that has the emscripten packages, certain
properties get set. If that isn't set then we fallback to using
$(EMSDK_PATH).

This should allow being able to use emscripten from workloads, but for
other cases where we depend on an installed emscripten, that should work
too.

When using a workload that has the emscripten packages, certain properties get set. If that isn't set then we fallback to using `$(EMSDK_PATH)`. This should allow being able to use emscripten from workloads, but for other cases where we depend on an installed emscripten, that should work too.

ghost · 2021-03-23T04:39:43Z

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

Issue Details

When using a workload that has the emscripten packages, certain
properties get set. If that isn't set then we fallback to using
$(EMSDK_PATH).

This should allow being able to use emscripten from workloads, but for
other cases where we depend on an installed emscripten, that should work
too.

Author:	radical
Assignees:	-
Labels:	`arch-wasm`, `area-Build-mono`
Milestone:	-

lewing

I think this looks reasonable. I wish we had windows AOT tests running

- If a workload pack is being used, then ignore `EMSDK_PATH` - And error in that order too - Emit better error messages Example messages: - Missing python dir when relinking: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/e10djxv0.wdr/python/3.7.4-2_64bit/ . Emscripten SDK is required for building native files.` - Missing node directory, or the wrong version dir: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk is missing some paths: $(EmscriptenNodeToolsPath)=/Users/radical/dev/r3/src/mono/wasm/emsdk/node/12.18.1_64bit/ . Emscripten SDK is required for building native files.` - Empty `EMSDK_PATH` `Error : Could not find Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk/. Emscripten SDK is required for AOT'ing assemblies.` - Bad `EMSDK_PATH` `error : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/l2mcsipf.isn/python/3.7.4-2_64bit/ . Emscripten SDK is required for AOT'ing assemblies.`

@lewing

.. and listen to @lewing's suggestions!

…t in emscripten sdk

ghost · 2021-03-24T19:53:44Z

Hello @radical!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

@lewing

* [wasm] Add support for using emscripten from packages When using a workload that has the emscripten packages, certain properties get set. If that isn't set then we fallback to using `$(EMSDK_PATH)`. This should allow being able to use emscripten from workloads, but for other cases where we depend on an installed emscripten, that should work too. * Fix setting PATH on windows * Using msbuild NormalizeDirectory to construct dir path * fix setting PATH on windows, when paths have trailing slash * [wasm] Improve checking emscripten from workload, and EMSDK_PATH - If a workload pack is being used, then ignore `EMSDK_PATH` - And error in that order too - Emit better error messages Example messages: - Missing python dir when relinking: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/e10djxv0.wdr/python/3.7.4-2_64bit/ . Emscripten SDK is required for building native files.` - Missing node directory, or the wrong version dir: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk is missing some paths: $(EmscriptenNodeToolsPath)=/Users/radical/dev/r3/src/mono/wasm/emsdk/node/12.18.1_64bit/ . Emscripten SDK is required for building native files.` - Empty `EMSDK_PATH` `Error : Could not find Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk/. Emscripten SDK is required for AOT'ing assemblies.` - Bad `EMSDK_PATH` `error : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/l2mcsipf.isn/python/3.7.4-2_64bit/ . Emscripten SDK is required for AOT'ing assemblies.` * [wasm] Don't hardcode versions for python/node bundled in emsdk .. and listen to @lewing's suggestions! * [wasm] GetDirectories doesn't like non-existant paths * Fix the earlier fix * improve errors when some paths are missing * cleanup * debugging on ci * [wasm] emscripten uses system python on linux, so don't try to find it in emscripten sdk

lewing · 2021-03-25T04:58:14Z

/backport to release/6.0-preview3

@lewing

* [wasm] Add support for using emscripten from packages When using a workload that has the emscripten packages, certain properties get set. If that isn't set then we fallback to using `$(EMSDK_PATH)`. This should allow being able to use emscripten from workloads, but for other cases where we depend on an installed emscripten, that should work too. * Fix setting PATH on windows * Using msbuild NormalizeDirectory to construct dir path * fix setting PATH on windows, when paths have trailing slash * [wasm] Improve checking emscripten from workload, and EMSDK_PATH - If a workload pack is being used, then ignore `EMSDK_PATH` - And error in that order too - Emit better error messages Example messages: - Missing python dir when relinking: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/e10djxv0.wdr/python/3.7.4-2_64bit/ . Emscripten SDK is required for building native files.` - Missing node directory, or the wrong version dir: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk is missing some paths: $(EmscriptenNodeToolsPath)=/Users/radical/dev/r3/src/mono/wasm/emsdk/node/12.18.1_64bit/ . Emscripten SDK is required for building native files.` - Empty `EMSDK_PATH` `Error : Could not find Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk/. Emscripten SDK is required for AOT'ing assemblies.` - Bad `EMSDK_PATH` `error : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/l2mcsipf.isn/python/3.7.4-2_64bit/ . Emscripten SDK is required for AOT'ing assemblies.` * [wasm] Don't hardcode versions for python/node bundled in emsdk .. and listen to @lewing's suggestions! * [wasm] GetDirectories doesn't like non-existant paths * Fix the earlier fix * improve errors when some paths are missing * cleanup * debugging on ci * [wasm] emscripten uses system python on linux, so don't try to find it in emscripten sdk

@lewing

* [wasm] Add support for using emscripten from packages When using a workload that has the emscripten packages, certain properties get set. If that isn't set then we fallback to using `$(EMSDK_PATH)`. This should allow being able to use emscripten from workloads, but for other cases where we depend on an installed emscripten, that should work too. * Fix setting PATH on windows * Using msbuild NormalizeDirectory to construct dir path * fix setting PATH on windows, when paths have trailing slash * [wasm] Improve checking emscripten from workload, and EMSDK_PATH - If a workload pack is being used, then ignore `EMSDK_PATH` - And error in that order too - Emit better error messages Example messages: - Missing python dir when relinking: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/e10djxv0.wdr/python/3.7.4-2_64bit/ . Emscripten SDK is required for building native files.` - Missing node directory, or the wrong version dir: `warning : Specified Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk is missing some paths: $(EmscriptenNodeToolsPath)=/Users/radical/dev/r3/src/mono/wasm/emsdk/node/12.18.1_64bit/ . Emscripten SDK is required for building native files.` - Empty `EMSDK_PATH` `Error : Could not find Emscripten sdk at $(EMSDK_PATH)=/Users/radical/dev/r3/src/mono/wasm/emsdk/. Emscripten SDK is required for AOT'ing assemblies.` - Bad `EMSDK_PATH` `error : Specified Emscripten sdk at $(EMSDK_PATH)=. is missing some paths: $(EmscriptenPythonToolsPath)=/Users/radical/dev/r3/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/l2mcsipf.isn/python/3.7.4-2_64bit/ . Emscripten SDK is required for AOT'ing assemblies.` * [wasm] Don't hardcode versions for python/node bundled in emsdk .. and listen to @lewing's suggestions! * [wasm] GetDirectories doesn't like non-existant paths * Fix the earlier fix * improve errors when some paths are missing * cleanup * debugging on ci * [wasm] emscripten uses system python on linux, so don't try to find it in emscripten sdk Co-authored-by: Ankit Jain <radical@gmail.com>

radical added arch-wasm WebAssembly architecture area-Build-mono labels Mar 23, 2021

Fix setting PATH on windows

3a9e053

radical marked this pull request as ready for review March 23, 2021 17:22

radical requested a review from marek-safar as a code owner March 23, 2021 17:22

radical requested review from akoeplinger, lewing and steveisok March 23, 2021 17:22

lewing requested a review from radekdoulik March 23, 2021 18:58

Using msbuild NormalizeDirectory to construct dir path

c07ea5a

lewing approved these changes Mar 23, 2021

View reviewed changes

Comment thread src/mono/wasm/build/WasmApp.targets Outdated

steveisok approved these changes Mar 23, 2021

View reviewed changes

radical added 2 commits March 23, 2021 17:42

fix setting PATH on windows, when paths have trailing slash

6d53c49

radical requested review from lewing and steveisok March 24, 2021 01:08

radical added 7 commits March 23, 2021 21:46

[wasm] Don't hardcode versions for python/node bundled in emsdk

8d787b1

.. and listen to @lewing's suggestions!

[wasm] GetDirectories doesn't like non-existant paths

66551b8

Fix the earlier fix

bd69028

improve errors when some paths are missing

efd31e5

cleanup

b6fd9be

debugging on ci

ce0cc39

[wasm] emscripten uses system python on linux, so don't try to find i…

584fe6b

…t in emscripten sdk

radekdoulik approved these changes Mar 24, 2021

View reviewed changes

radical added the auto-merge label Mar 24, 2021

radical added auto-merge and removed auto-merge labels Mar 24, 2021

ghost merged commit c547790 into dotnet:main Mar 24, 2021

radical deleted the emscripten-from-pkg branch March 24, 2021 21:16

lewing mentioned this pull request Mar 25, 2021

[release/6.0-preview3] Backport of Add support for using emscripten from packages #50077 #50227

Merged

ghost locked as resolved and limited conversation to collaborators Apr 24, 2021

karelz added this to the 6.0.0 milestone May 20, 2021

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[wasm] Add support for using emscripten from packages#50077

[wasm] Add support for using emscripten from packages#50077
12 commits merged into
dotnet:mainfrom
radical:emscripten-from-pkg

radical commented Mar 23, 2021

Uh oh!

ghost commented Mar 23, 2021

Uh oh!

lewing left a comment •

edited

Loading

Uh oh!

Uh oh!

ghost commented Mar 24, 2021

Uh oh!

lewing commented Mar 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

radical commented Mar 23, 2021

Uh oh!

ghost commented Mar 23, 2021

Uh oh!

lewing left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost commented Mar 24, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

lewing commented Mar 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lewing left a comment •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.