HPCC-34104 Add wasm32 build target #19868
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a wasm32 build target to support Emscripten compilation within the HPCC project, as described in HPCC-34104. Key changes include updates to various CMake files to enable EMSCRIPTEN‐specific build flags, modifications to dockerfiles and build scripts to support the wasm32 target, and adjustments to configuration files such as package.json and vcpkg settings.
Reviewed Changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| roxie/ccd/CMakeLists.txt | Added EMSCRIPTEN-specific compile flag for the ccd library. |
| plugins/py3embed/py3embed.cpp | Included EMSCRIPTEN in the conditional inclusion for execinfo backtrace support. |
| plugins/nlp/nlp_eng.cpp | Extended build condition to define LINUX for EMSCRIPTEN; consider if this conflation is appropriate. |
| package.json | Changed module type from ES module to CommonJS to align with the wasm32 target requirements. |
| fs/dafilesrv/dafilesrv.cpp | Added a stub daemon function for EMSCRIPTEN builds. |
| esp/scm/*.cmake | Updated custom commands to prepend COMMAND_NODE_PREFIX for EMSCRIPTEN builds. |
| ecl/hql/hqlstack.hpp & CMakeLists.txt | Introduced EMSCRIPTEN-specific definitions and compile flags. |
| dockerfiles/* & dockerfiles/build.sh | Updated build scripts and Dockerfiles to support a wasm32-emscripten build. |
| cmake_modules/* | Adjusted multiple CMake modules (vcpkg, plugins, options, commonSetup) for EMSCRIPTEN compatibility. |
| .github/workflows/build-vcpkg.yml | Added a new GitHub workflow job to build the wasm32-emscripten Docker image. |
Comments suppressed due to low confidence (2)
package.json:4
- Switching from ES module to CommonJS might affect module resolution and dependency loading. Please ensure that all downstream scripts and package dependencies are compatible with CommonJS.
"type":"commonjs"
cmake_modules/plugins.cmake:25
- [nitpick] The addition of 'emscripten' to VCPKG_INCLUDE and similarly to VCPKG_SUPPRESS should be verified against the actual dependency requirements for EMSCRIPTEN. Documenting this change in the project’s build documentation may help clarify intended behavior.
set(VCPKG_INCLUDE "(windows | osx | linux | emscripten)")
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-34104 Jirabot Action Result: |
There was a problem hiding this comment.
@GordonSmith looks good. One cleanup comment.
Is master best, or will the changes to make/docker files cause upmerge conflicts?
There was a problem hiding this comment.
Pull Request Overview
Adds a new WebAssembly build target by integrating Emscripten into the existing build system and CI.
- Enable EMSCRIPTEN compilation paths in source files and headers
- Extend CMake, vcpkg, and build scripts to recognize
wasm32-emscriptenand set appropriate flags - Provide a Dockerfile and CI job to build and test the new target
Reviewed Changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| roxie/ccd/ccdfile.cpp | Include <sys/mman.h> when compiling under Emscripten |
| roxie/ccd/CMakeLists.txt | Suppress memcall errors for EMSCRIPTEN target |
| plugins/py3embed/py3embed.cpp | Enable backtrace includes under Emscripten |
| package.json | Switch package type from module to commonjs |
| fs/dafilesrv/dafilesrv.cpp | Stub out daemon() on Emscripten |
| esp/scm/smcscm.cmake | Prefix SCM commands with COMMAND_NODE_PREFIX |
| esp/scm/ncmscm.cmake | Prefix NCM commands with COMMAND_NODE_PREFIX |
| esp/scm/espscm.cmake | Prefix ESD (ESP SCM) commands similarly |
| esp/scm/additional.cmake | Prefix additional SCM commands |
| ecl/hql/hqlstack.hpp | Define stack alignment for Emscripten |
| ecl/hql/CMakeLists.txt | Add compile flags for Emscripten on hql |
| dockerfiles/wasm32-emscripten.dockerfile | New Dockerfile for wasm32-emscripten builds |
| dockerfiles/build.sh | Adjust build script to support wasm32 target |
| devdoc/.vitepress/config.mts | Exclude third-party/** from doc builds |
| cmake_modules/vcpkg.cmake | Configure vcpkg triplets and toolchain for EMSCRIPTEN |
| cmake_modules/plugins.cmake | Include emscripten in plugin host/suppress filters |
| cmake_modules/options.cmake | Accept MinSizeRel build type and disable features under EMSCRIPTEN |
| cmake_modules/commonSetup.cmake | Parse Emscripten compiler version string |
| .github/workflows/build-vcpkg.yml | Add CI job for wasm32-emscripten build |
If you would rather a diff branch to target it might help upmerge conflicts, but I don't think they would be too bad? |
Signed-off-by: Gordon Smith <[email protected]>
Fix windows build failing
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for building the HPCC platform for the wasm32 target using Emscripten by updating source files, CMake modules, and Docker build scripts.
- Extended platform-specific conditions and target properties to include EMSCRIPTEN.
- Updated CMake configuration and build scripts (including Dockerfiles) to handle EMSCRIPTEN-specific build flags and options.
- Modified package.json and documentation settings to align with the wasm32 build target.
Reviewed Changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| roxie/ccd/ccdfile.cpp | Added EMSCRIPTEN to platform check for system header inclusion. |
| roxie/ccd/CMakeLists.txt | Set EMSCRIPTEN-specific compile flag on the ccd library. |
| plugins/py3embed/py3embed.cpp | Extended platform check to include EMSCRIPTEN for backtrace setup. |
| package.json | Changed module type from ES modules to CommonJS. |
| fs/dafilesrv/dafilesrv.cpp | Added a stub for daemon() on EMSCRIPTEN. |
| esp/scm/* cmake files | Updated custom commands to include COMMAND_NODE_PREFIX for EMSCRIPTEN. |
| ecl/hql/hqlstack.hpp & CMakeLists.txt | Added EMSCRIPTEN-specific adjustments for compilation flags. |
| dockerfiles/wasm32-emscripten.dockerfile | Added a Dockerfile for building using wasm32 Emscripten. |
| dockerfiles/build.sh | Modified build scripts to handle wasm32-emscripten configuration. |
| devdoc/.vitepress/config.mts | Updated source exclusion paths to omit third-party directories. |
| cmake_modules/* | Added EMSCRIPTEN-specific configuration and toolchain settings. |
| .github/workflows/build-vcpkg.yml | Added CI workflow support for the wasm32-emscripten build. |
Comments suppressed due to low confidence (2)
package.json:4
- Changing the module type from 'module' to 'commonjs' can affect module resolution and imports; please ensure that all dependent code and tooling are updated accordingly.
"type":"commonjs",
esp/scm/smcscm.cmake:56
- Ensure that the variable COMMAND_NODE_PREFIX is defined in the build configuration or environment so that the injected command works as expected across all relevant platforms.
COMMAND ${ASAN_LEAK_SUPPRESS_PREFIX} ${COMMAND_NODE_PREFIX} $<TARGET_FILE:hidl> ${ESPSCM_SOURCE_DIR}/${result}.ecm ${ESPSCM_GENERATED_DIR}
| @@ -94,16 +95,19 @@ function doBuild() { | |||
|
|
|||
| mkdir -p $ROOT_DIR/build/$1 | |||
| CMAKE_OPTIONS_EXTRA="-DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache" | |||
| if [ "$1" == "wasm32-emscripten" ]; then | |||
There was a problem hiding this comment.
[nitpick] Consider adding a brief comment here to explain why the build type is switched to 'MinSizeRel' and the EMSCRIPTEN flag is added for wasm32 builds to help future maintainers.
Copilot uses AI. Check for mistakes.
Type of change:
Checklist:
Smoketest:
Testing: