serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Meta: This makes it possible to build SerenityOs on WSL2 again

Open wertzuz opened this issue 2 years ago • 7 comments

This was broken since commit 71b184a, Because CMAKE_SKIP_RPATH has to be true.

wertzuz avatar Jun 12 '23 22:06 wertzuz

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

BuggieBot avatar Jun 12 '23 22:06 BuggieBot

Can you explain a bit more what you mean with "broken"? A lot of people, including me, are using WSL2 and can build and run it just fine.

fdellwing avatar Jun 13 '23 04:06 fdellwing

I have installed arch on WSL2 and got following errors when trying to build:

  The install of the DynlibA target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:13 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibA target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:13 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibB target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:14 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibB target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:14 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibC target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:16 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibC target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:16 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibD target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:18 (add_dlopen_lib)


CMake Error at Tests/LibELF/CMakeLists.txt:3 (add_library):
  The install of the DynlibD target requires changing an RPATH from the build
  tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:18 (add_dlopen_lib)


CMake Error at Meta/CMake/utils.cmake:121 (add_executable):
  The install of the test-elf target requires changing an RPATH from the
  build tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:28 (serenity_test)


CMake Error at Meta/CMake/utils.cmake:121 (add_executable):
  The install of the TestDlOpen target requires changing an RPATH from the
  build tree, but this is not supported with the Ninja generator unless on an
  ELF-based or XCOFF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH
  variable may be set to avoid this relinking step.
Call Stack (most recent call first):
  Tests/LibELF/CMakeLists.txt:28 (serenity_test)

This happened since the mentioned commit. Setting CMAKE_SKIP_RPATH to TRUE fixed the issue and made it possible for me to boot again.

wertzuz avatar Jun 13 '23 19:06 wertzuz

This change is not correct. Those libraries must have their rpath in place. Your change breaks at least one test on serenity. https://dev.azure.com/SerenityOS/SerenityOS/_build/results?buildId=33372&view=logs&j=1427867c-05b9-544b-63a5-81f0b096b4ff&t=e99ee61b-bd91-5005-d29b-1e01d815fdb7&l=427

Previously when we've seen that error, a full rebuild or a deletion of the CMake cache has fixed it.

ADKaster avatar Jun 14 '23 22:06 ADKaster

I even went to the point where i completely recloned the serenity repository, but without this change it won't boot for me. I as well rechecked my CMake, clang and build setup. But well the difference seems to be at another point, ig.

wertzuz avatar Jun 15 '23 14:06 wertzuz

image image

I looked at the CMAKE_EXECUTABLE_FORMAT variable, which controls whether this check errors out or not, and it seems that on WSL2, it's incorrectly marked as Unknown rather than ELF, as it is on my MacBook Pro. This seems to be a CMake error. Not sure if it's upstream or in our CMake toolchain files.

ADKaster avatar Jun 18 '23 13:06 ADKaster

I had the issue disappear after doing a sudo apt update and sudo apt upgrade in my wsl2 instance, as well as removing stale clang-12, clang-13 and clang-14 packages and installing clang-15. Can you verify whether doing these steps fixes the problem for you?

I added

variable_watch(CMAKE_EXECUTABLE_FORMAT)
variable_watch(CMAKE_EXECUTABLE_MAGIC)

to the top of the top-level CMakeLists.txt, before any other lines in the file and saw that the CMAKE_EXECUTABLE_FORMAT was correctly set to "ELF" instead of "Unknown" after doing those steps from a clean build.

ADKaster avatar Jun 18 '23 13:06 ADKaster