Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Add placeholder build option for WebAssembly - #4620

Merged
morganbr merged 3 commits into
dotnet:masterfrom
ph1ll:wasm_buildscripts
Sep 30, 2017
Merged

Add placeholder build option for WebAssembly#4620
morganbr merged 3 commits into
dotnet:masterfrom
ph1ll:wasm_buildscripts

Conversation

@ph1ll

@ph1ll ph1ll commented Sep 26, 2017

Copy link
Copy Markdown
Contributor

This commit adds a placeholder build target for WebAssembly to the bash and batch build scripts (#4504).

At present, an error is displayed advising that the wasm build is not yet implemented before exiting.

I was hoping to try and take a look at the cmake scripts for #4505, and this seemed like a sensible first step.

@dnfclas

dnfclas commented Sep 26, 2017

Copy link
Copy Markdown

@ph1ll,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@jkotas

jkotas commented Sep 26, 2017

Copy link
Copy Markdown
Member

You will still want the tool chain to be say Windows x64, but only compile the libraries to WebAssembly ... how are we going to make that happen?

@morganbr

Copy link
Copy Markdown
Contributor

@ph1ll, thanks for getting this started and a special thanks for the first community WebAssembly PR!

@jkotas, that's a very good question. I'd split that question up a little:

  1. Compiling the runtime/framework's C++ (most code under src/Native): Needs to use Emscripten and probably enable some #ifdefs
  2. Compiling the C++ parts of ILC (src/Native/jitinterface) -- we might not actually need any of these for WASM right now.
  3. Compiling the runtime/framework's C#: We can't pass a WASM architecture to the C# compiler, so it doesn't really matter what we do here as long as we get the right set of #ifs
  4. Compiling ILC's C#: Mostly the same as 3 -- if we aren't passing anything special to the C# compiler, hopefully we just get a working ILC, but we might have to track the build machine's architecture in order to get the right .NET Core and dependencies.

@morganbr morganbr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great progress toward WebAssembly build support! I've added comments on what might need to be tweaked for filling in the rest of the build, but once you respond to the comments, I'm happy with the change.

Comment thread buildscripts/build-native.cmd Outdated
exit /b 1

:BuildNativeEmscripten
:: TODO: Add a real wasm build

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you give us an idea of what you have in mind here? I know msbuild and clang might not match up too well but I'd like to understand what we'd do instead on Windows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It looks like there has been some work in the past to create an extension for VS to allow building with Emscripten, however this looks to be VS 2010 only. (http://kripken.github.io/emscripten-site/docs/getting_started/getting_started_with_emscripten_and_vs2010.html). Whilst it would be nice to keep it all under msbuild on Windows, I am not sure at the moment how easy that will be.

I have however, been able to get the same build progress as on linux by installing GNU make on Windows (http://gnuwin32.sourceforge.net/packages/make.htm).

Generation

emcmake "%CMakePath%" "-DCMAKE_TOOLCHAIN_FILE=%EMSCRIPTEN%/cmake/Modules/Platform/Emscripten.cmake" "-DCMAKE_MAKE_PROGRAM=C:\Program Files (x86)\GnuWin32\bin\make.exe" "-DCMAKE_BUILD_TYPE=Debug" -G "MinGW Makefiles" %1

Build

pushd "%__IntermediatesDir%"
emmake "C:\Program Files (x86)\GnuWin32\bin\make.exe" install
popd

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we typically use the Windows build of CMake instead of GNU make https://cmake.org/download/ . Hopefully that still fits everything else you're doing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As CMake is only generating the build system, we still need to pick a generator. The normal Windows build is using the msbuild generator, but in order to use msbuild with Emscripten, more work would be required (although this would be cleaner). At the moment, I was using the Make generator, as it fits in nicely with the Emscripten build (although adds a dependency on Make).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, sorry for the confusion. We might be able to try NMake, which comes with Visual Studio and is supposedly supported by cmake on Windows. (MS folks might have licensing problems with GNU Make; something that comes with Visual Studio or uses a different license will be a lot easier for us).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(To be clear, we'd happily take your change with the GNU Make dependency and then try to replace it as a separate change).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

NMake is a good shout. I had overlooked that. CMake has an NMake generator and it seems to work nicely. I've updated the scripts to use it.

Comment thread buildscripts/build-native.sh Outdated

# Check for additional prereqs for wasm build
if [ $__BuildArch == "wasm" ]; then
hash emcmake 2>/dev/null || { echo >&2 "Please install Emscripten before running this script"; exit 1; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do. Will also need to check for the $EMSCRIPTEN variable and advise the user to run source ./emsdk_env.sh

Comment thread buildscripts/build-native.sh Outdated
"$__ProjectRoot/src/Native/gen-buildsys-clang.sh" "$__ProjectRoot" $__ClangMajorVersion $__ClangMinorVersion $__BuildArch $__BuildType
if [ $__BuildArch == "wasm" ]; then
# TODO: Add a real wasm build
echo "Wasm build is not currently implemented"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you attempted running emcmake to see if it just works with the rest of this build? (No need to block on this, just curious if it should work already)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At the moment, I am building with:

emcmake cmake \
    "-DCMAKE_TOOLCHAIN_FILE=$EMSCRIPTEN/cmake/Modules/Platform/Emscripten.cmake" \
    "-DCMAKE_BUILD_TYPE=$build_type" \
    "$1/src/Native"

Unfortunately the rest of the build doesn't just work, but with a few tweaks to the CMakeLists.txt files, .\Native\jitinterface and .\Native\Bootstrap compile. I am currently running into issues with .\Native\Runtime\Portable with the "PalRedHawk" files, as the new target is not handled there. The assembly files for .\Native\Runtime\Full are also going to be an issue.

If you like, I could add what I have as a work in progress MR so you could see where it is at.

Comment thread buildscripts/build-native.sh Outdated
popd > /dev/null
exit 1
else
"$__ProjectRoot/src/Native/gen-buildsys-clang.sh" "$__ProjectRoot" $__ClangMajorVersion $__ClangMinorVersion $__BuildArch $__BuildType

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suspect you'll need to update gen-buildsys-clang rather than skipping it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I started off with copies of the gen-buildsys-* files, but the changes required to at least get emscripten starting the build are fairly minor, so I can update the original files with some conditionals and add to this MR.

if [ $__BuildArch == "wasm" ]; then
export __BuildOS=WebAssembly
else
# Use uname to determine what the OS is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As I mentioned in response to @jkotas's question, we'll probably need to capture the NuGet information and maybe the OS for getting the right .NET Core dependencies for ILC (but we probably need to use a different variable rather than __BuildOS)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% clear on what the final build/packaging story will be, so would welcome any suggestions here.

I anticipated that ultimately any build would need include the WASM build so that the static runtime .bc file can be packaged/embedded somewhere it can be accessed by the current platforms ILC.

The reason I went for this approach now, is it allows the WASM build to be attempted completely separately, until it is at a point where it actually works.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest starting with keeping the logic to set __NugetRuntimeId since that's required to publish ILC to the tools directory

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That sounds reasonable

@jkotas

jkotas commented Sep 27, 2017

Copy link
Copy Markdown
Member

@morganbr Agree - we need to allow for different arch of runtime/framework and the toolchain.

There are two basic choices:

  1. Allow building more flavors of the same component in one build. Is is what we have been doing for the most part so far:
  1. Each full stack build builds exactly one arch of each component.

The first plan tends to be more friendly to cost-effective CI: one leg can do basic testing of number of combinations or flavors for less. Of course, you can always split it if it starts taking too long.

@morganbr
morganbr requested a review from jkotas September 30, 2017 03:11
@morganbr

Copy link
Copy Markdown
Contributor

@jkotas , if you don't have concerns, I'd like to merge this to make experimentation easier.

@morganbr
morganbr merged commit da8c27d into dotnet:master Sep 30, 2017
@morganbr

Copy link
Copy Markdown
Contributor

Thanks, @ph1ll!

shrah pushed a commit that referenced this pull request Oct 3, 2017
* Add placeholder build option for WebAssembly

* Add Emscripten build commands

* Use Nmake in place of Make
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants