bnoordhuis / v8-cmake

The V8 JavaScript engine, but built with CMake instead of GN - WIP
BSD 3-Clause "New" or "Revised" License
189 stars 54 forks source link

feat: upgrade to v8 11.6 #66

Closed gengjiawen closed 8 months ago

gengjiawen commented 1 year ago

fix https://github.com/bnoordhuis/v8-cmake/issues/65 fix https://github.com/bnoordhuis/v8-cmake/issues/42

bnoordhuis commented 1 year ago

"The system cannot execute the specified program."

But it's not saying what program and I can't really divine it from the logs... I'm guessing it's builtins-generated from cmake/GenerateBuiltinsList.cmake but not 100% sure.

gengjiawen commented 1 year ago

"The system cannot execute the specified program."

But it's not saying what program and I can't really divine it from the logs... I'm guessing it's builtins-generated from cmake/GenerateBuiltinsList.cmake but not 100% sure.

More like the long path issue on windows. Need a new way to handle torque.

bnoordhuis commented 1 year ago

Long path or long argument list? torque.exe ends up D:/a/v8-cmake/build, right?

gengjiawen commented 1 year ago

Long path or long argument list? torque.exe ends up D:/a/v8-cmake/build, right?

long argument list

bnoordhuis commented 1 year ago

That's what I thought. CMake has a bunch of response file variables (like CMAKE_CXX_USE_RESPONSE_FILE_FOR_OBJECTS) to get around invoking the compiler or linker with long argument lists but nothing (as far as I know) for using response files with arbitrary executables.

Torque doesn't support reading from response files but that's easy to add.

bnoordhuis commented 1 year ago

Forgot to mention the obvious: invoking torque multiple times with shorter argument lists - but I don't know if that actually works. Easy enough to test though.

barracuda156 commented 11 months ago

Edited: ah, I did not notice, it has been on CI at least.

I will give it a go locally then.

gengjiawen commented 11 months ago

Edited: ah, I did not notice, it has been on CI at least.

I will give it a go locally then.

macOS and linux should works. windows still has the path issues.

barracuda156 commented 11 months ago

@gengjiawen Great.

P. S. Then I just need to find someone who can help sorting out powerpc assembler a bit :)

I have 8.3.110.13 building on macOS ppc, but a) something still needs to be fixed for it to work correctly and b) from 8.4.x+ upstream has broken ppc somewhat further.

gengjiawen commented 11 months ago

on a windows enable long path, build works CleanShot 2023-12-25 at 14 15 21@2x

bnoordhuis commented 11 months ago

How did you get around the long argument list issue?

FWIW, the internet suggests long paths are enabled on GHA Windows runners but git needs to opt in, presumably like so:

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 89498826..cfed0378 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -18,7 +18,9 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       - name: setup
-        run: cmake -E make_directory ${{runner.workspace}}/build
+        run: |
+          git config core.longpaths true
+          cmake -E make_directory ${{runner.workspace}}/build
       - name: configure
         run: cmake ${{runner.workspace}}/v8-cmake
         working-directory: ${{runner.workspace}}/build

Maybe cmake needs to opt in too, somehow.

barracuda156 commented 11 months ago

How did you get around the long argument list issue?

Maybe cmake needs to opt in too, somehow.

Kinda on a side-note, but on macOS long paths can also be a problem. Example: https://github.com/ALSparse/ALSparse/issues/19

gengjiawen commented 11 months ago

How did you get around the long argument list issue?

I turned on long-path support on my windows. Not sure if it's GHA Windows revert long path support ?

bnoordhuis commented 11 months ago

Hm, still failing with this:

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'D:\a\v8-cmake\build\CMakeFiles\3caf4c045984d86c8d0e2deebc748fc9\builtins.rule;D:\a\v8-cmake\build\CMakeFiles\a8f7d1923a331b4b50796a7a07bdcbe8\aggregate-error-tq-csa.cc.rule;D:\a\v8-cmake\build\CMakeFiles\184467c103758efcc2c8ca2fe9d02f8b\builtins-generated.rule;D:\a\v8-cmake\build\CMakeFiles\d3ae68cc38a20a9bf244124ff4705ebf\bytecodes-builtins-list.h.rule;D:\a\v8-cmake\v8-cmake\CMakeLists.txt' exited with code 9020. [D:\a\v8-cmake\build\v8_torque_generated.vcxproj]

Unclear what exit code 9020 signifies.

gengjiawen commented 11 months ago

Looks GA windows not enabled long path anymore, at least now. Screenshot from runs-on: windows-2022

CleanShot 2023-12-27 at 10 29 54@2x

gengjiawen commented 11 months ago

Can this done per file (sacrifice perf) on windows ?

bnoordhuis commented 11 months ago

Hm, that's weird. https://github.com/actions/runner-images/blob/62a46d0fd8/images/windows/scripts/build/Configure-BaseImage.ps1#L73 suggests it's (still?) enabled on 2022 runners. It was definitely enabled on 2019 runners.

gengjiawen commented 10 months ago

Looks still a messy for windows 2019. They break it for somehow.

gengjiawen commented 10 months ago

@bnoordhuis The problem part is torque_outputs is too long, I tried do it in foreach, but still failed. Any suggestions ?

add_custom_command(
  COMMAND
    torque
    -o ${PROJECT_BINARY_DIR}/torque-generated
    -v8-root ${PROJECT_SOURCE_DIR}/v8
    ${torque_files}
  DEPENDS
    torque
    ${torque_dirs}
    ${torque_files_abs}
  OUTPUT
    ${torque-outputs}
    ${torque_outputs}
)

foreach(file IN LISTS torque_outputs)
  add_custom_command(
    OUTPUT ${file}
    COMMAND ${CMAKE_COMMAND} -E touch ${file}
    DEPENDS
      torque
      ${torque_dirs}
      ${torque_files_abs}
  )
endforeach()
bnoordhuis commented 10 months ago

The problem part is torque_outputs is too long

How do you figure? I went through the CI results for the last four commits but I'm not sure how you came to that conclusion. I do see a lot of warnings like these:

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): warning MSB8065: Custom build for item "D:\a\v8-cmake\build\CMakeFiles\a8f7d1923a331b4b50796a7a07bdcbe8\aggregate-error-tq-csa.cc.rule" succeeded, but specified output "d:\a\v8-cmake\build\torque-generated\objects-printer-tq.cc" has not been created. This may cause incremental build to work incorrectly.

Culminating - eventually - in this error:

c1xx : fatal error C1083: Cannot open source file: 'D:\a\v8-cmake\build\torque-generated\objects-printer-tq.cc': No such file or directory [D:\a\v8-cmake\build\v8_torque_generated.vcxproj]

gengjiawen commented 10 months ago

The problem part is torque_outputs is too long

How do you figure? I went through the CI results for the last four commits but I'm not sure how you came to that conclusion. I do see a lot of warnings like these:

Normally win build failed in 2m. When I skip torque_outputs, it failed at 29mins. https://github.com/bnoordhuis/v8-cmake/actions/runs/7406169157/job/20150198402?pr=66

bnoordhuis commented 8 months ago

@gengjiawen can you rebase on top of master? I just merged a commit that - with some luck - fixes the Windows build.

gengjiawen commented 8 months ago

@bnoordhuis Looks like build passed!

gengjiawen commented 8 months ago

@bnoordhuis Let's merge this ?

bnoordhuis commented 8 months ago

Yes sorry, lots of things going on at the moment :)

barracuda156 commented 8 months ago

@bnoordhuis Could this be released in tags?

bnoordhuis commented 8 months ago

Done: https://github.com/bnoordhuis/v8-cmake/tree/11.6.189.4

barracuda156 commented 8 months ago

Thank you!