Skip to content

Conversation

@vespakoen
Copy link
Contributor

@phkahler @ruevs This worked on my fork, don't ask me why the first attempt didn't work, I don't feel like looking into it too deep.
The last attempt (using CMAKE_CXX_STANDARD) on my machine, also stopped working after clearing my cache, so I guess either emcmake ignores the CMAKE_CXX_STANDARD somehow, or maybe EMSCRIPTEN isn't set for the web build?

Please let me know if you are fine with this or just merge it ;)

@ruevs ruevs marked this pull request as draft January 2, 2026 22:27
@ruevs
Copy link
Member

ruevs commented Jan 2, 2026

I do not like it :-(
An alternative attempt currently running the CD on my fork here. Let's see...

https://github.com/ruevs/solvespace/actions/runs/20667998716

Update: it failed :-( Off... I really did not want to setup Emscripten on this system... I guess I'll have to... tomorrow...
Good night and Happy New Year!

@ruevs
Copy link
Member

ruevs commented Jan 2, 2026

Here is what the LLM says...

Details
The change to C++17 for Emscripten was made in commit 37484dc. This update modified the CMake configuration to set CMAKE_CXX_STANDARD to 17 when the EMSCRIPTEN condition is true. However, despite these changes, the build failed to use C++17, leading to compilation errors.

The issue lies in the following:

    Top-level CMake configuration: The changed code sets the C++ standard based on the EMSCRIPTEN condition:
    CMake

    if(EMSCRIPTEN)
      set(CMAKE_CXX_STANDARD 17)
    else()
      set(CMAKE_CXX_STANDARD 11)
    endif()
    set(CMAKE_CXX_STANDARD_REQUIRED YES)

    This should ensure that Emscripten builds use at least C++17. However, in the build logs, Emscripten was not applying this configuration.

    Scoping problem: The CMAKE_CXX_STANDARD declaration applies globally but might be overridden elsewhere in project-specific or subdirectory CMakeLists.txt files. For example:
        The code in /bench, /exposed, or other directories may override or not inherit the correct C++ standard.
        These subdirectories may explicitly set target_compile_features or other settings conflicting with the top-level configuration.

    This can happen if subdirectory CMakeLists.txt files are missing explicit inheritance of the C++ standard set at the root level.

    Emscripten detection issue: If Emscripten's toolchain file isn't included early in the CMake configuration, the EMSCRIPTEN condition might not be evaluated correctly.

Fix Recommendations:

    Enforce C++17 globally for Emscripten: Use set_property to enforce the C++ standard for all targets, ensuring subdirectories inherit the configuration:
    CMake

    if(EMSCRIPTEN)
      set(CMAKE_CXX_STANDARD 17)
      set(CMAKE_CXX_STANDARD_REQUIRED YES)
      set_property(GLOBAL PROPERTY CXX_STANDARD 17)
    else()
      set(CMAKE_CXX_STANDARD 11)
    endif()

    Audit subdirectory configurations:
        Check subdirectory CMakeLists.txt files (like /bench/CMakeLists.txt) for target_compile_features or custom compiler flags. Ensure they do not conflict with the global configuration.

    Include the Emscripten toolchain file early: Make sure the Emscripten toolchain is loaded correctly so the EMSCRIPTEN condition is evaluated. If the condition is not recognized, add include(Platform/Emscripten) to ensure proper detection.

Let me know if you'd like assistance identifying conflicts in your subdirectory configuration files!

And all the places trying to set the standard:

Details
Code	File	Line	Column
set(CMAKE_CXX_STANDARD 17)	C:\PAR\Projects\Solvespace\git\extlib\mimalloc\test\CMakeLists.txt	5	11
  set(CMAKE_CXX_STANDARD 17)	C:\PAR\Projects\Solvespace\git\CMakeLists.txt	17	13
  set(CMAKE_CXX_STANDARD 11)	C:\PAR\Projects\Solvespace\git\CMakeLists.txt	19	13
set(CMAKE_CXX_STANDARD_REQUIRED YES)	C:\PAR\Projects\Solvespace\git\CMakeLists.txt	21	11
  set(CMAKE_CXX_STANDARD_COMPUTED_DEFAULT 98)	C:\PAR\Projects\Solvespace\git\cmake\Platform\Emscripten.cmake	152	13
set(CMAKE_CXX_STANDARD 11)	C:\PAR\Projects\Solvespace\git\extlib\angle\CMakeLists.txt	5	11
set(CMAKE_CXX_STANDARD_REQUIRED YES)	C:\PAR\Projects\Solvespace\git\extlib\angle\CMakeLists.txt	6	11
  set_property(TARGET btl_blaze PROPERTY CXX_STANDARD 14)	C:\PAR\Projects\Solvespace\git\extlib\eigen\bench\btl\libs\blaze\CMakeLists.txt	9	42
  get_target_property(targetCxxStandard ${SDK_BUILD_IR_TARGET} CXX_STANDARD)	C:\PAR\Projects\Solvespace\git\extlib\eigen\cmake\FindComputeCpp.cmake	251	64
set(CMAKE_CXX_STANDARD 17)	C:\PAR\Projects\Solvespace\git\extlib\eigen\cmake\FindTriSYCL.cmake	65	11
set(CXX_STANDARD_REQUIRED ON)	C:\PAR\Projects\Solvespace\git\extlib\eigen\cmake\FindTriSYCL.cmake	66	5
  set(CMAKE_CXX_STANDARD 11)	C:\PAR\Projects\Solvespace\git\extlib\eigen\CMakeLists.txt	80	13
  #set(CMAKE_CXX_STANDARD 03)	C:\PAR\Projects\Solvespace\git\extlib\eigen\CMakeLists.txt	86	14
  set(CMAKE_CXX_STANDARD 17)	C:\PAR\Projects\Solvespace\git\extlib\eigen\unsupported\doc\examples\SYCL\CMakeLists.txt	6	13
    set(CMAKE_CXX_STANDARD 14)	C:\PAR\Projects\Solvespace\git\extlib\eigen\unsupported\doc\examples\SYCL\CMakeLists.txt	11	15
    set(CMAKE_CXX_STANDARD 11)	C:\PAR\Projects\Solvespace\git\extlib\eigen\unsupported\doc\examples\SYCL\CMakeLists.txt	14	15
      set(CMAKE_CXX_STANDARD 17)	C:\PAR\Projects\Solvespace\git\extlib\eigen\unsupported\test\CMakeLists.txt	167	17
        set(CMAKE_CXX_STANDARD 14)	C:\PAR\Projects\Solvespace\git\extlib\eigen\unsupported\test\CMakeLists.txt	172	19
        set(CMAKE_CXX_STANDARD 11)	C:\PAR\Projects\Solvespace\git\extlib\eigen\unsupported\test\CMakeLists.txt	175	19
set(CMAKE_CXX_STANDARD 17)	C:\PAR\Projects\Solvespace\git\extlib\mimalloc\CMakeLists.txt	5	11

Perhaps we should go with your way, but at least put it in if(EMSCRIPTEN)... I'll try it now...

@ruevs ruevs marked this pull request as ready for review January 2, 2026 22:55
@ruevs ruevs merged commit 6c0273f into solvespace:master Jan 2, 2026
4 checks passed
@ruevs
Copy link
Member

ruevs commented Jan 2, 2026

Off I'm too sleepy; you did put it in the EMSCRIPTEN section. Thank you. Merged.

Edit: and it worked!
https://github.com/solvespace/solvespace/actions/runs/20668360975
Just like in your fork.

@ruevs
Copy link
Member

ruevs commented Jan 3, 2026

@vespakoen @phkahler I also fixed the failing WASM library compilation here d14609f

The build now works: https://github.com/solvespace/solvespace/actions/runs/20681760812
but the "publish WASM library" part of the action is still broken:
https://github.com/solvespace/solvespace/actions/runs/20681760812/job/59376987328#step:6:135

npm notice Access token expired or revoked. Please try logging in again.
npm ERR! code E404
npm ERR! 404 Not Found - PUT https://registry.npmjs.org/slvs - Not found
npm ERR! 404 
npm ERR! 404  '[email protected]' is not in the npm registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

I have no clue what to do about it.

@ruevs ruevs mentioned this pull request Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants