Meta+Lagom: Enable CMAKE_BUILD_WITH_INSTALL_RPATH

On macOS, CMake incorrectly tries to add and/or remove rpaths from files
that it has already processed when it performs installation. Setting the
rpaths during the build process ensures that they are only set once, and
as a bonus, makes installation slightly more performant.

Fixes #10055.

.github/workflows/libjs-test262.yml

@@ -125,7 +125,7 @@ jobs:
       - name: Run test-wasm
         working-directory: libjs-test262/Build
         run: |
-          _deps/lagom-build/test-wasm --per-file _deps/lagom-build/Userland/Libraries/LibWasm/Tests > ../../libjs-website/wasm/data/per-file-master.json || true
+          _deps/lagom-build/bin/test-wasm --per-file _deps/lagom-build/Userland/Libraries/LibWasm/Tests > ../../libjs-website/wasm/data/per-file-master.json || true
           jq -nc -f /dev/stdin <<-EOF --slurpfile previous ../../libjs-website/wasm/data/results.json --slurpfile details ../../libjs-website/wasm/data/per-file-master.json > wasm-new-results.json
             \$details[0] as \$details | \$previous[0] + [{
               "commit_timestamp": $(git -C ../.. log -1 --format=%ct),

.github/workflows/wasm.yml

@@ -90,8 +90,8 @@ jobs:
       - name: "Prepare updated REPL data"
         if: github.ref == 'refs/heads/master'
         run: |
-          cp ${{ github.workspace }}/Build/wasm/libjs.js libjs-website/repl/libjs.js
-          cp ${{ github.workspace }}/Build/wasm/libjs.wasm libjs-website/repl/libjs.wasm
+          cp ${{ github.workspace }}/Build/wasm/bin/libjs.js libjs-website/repl/libjs.js
+          cp ${{ github.workspace }}/Build/wasm/bin/libjs.wasm libjs-website/repl/libjs.wasm
           echo 'Module.SERENITYOS_COMMIT = "${{ github.sha }}";' >> libjs-website/repl/libjs.js
 
       - name: "Deploy to GitHub Pages"

Ladybird/CMakeLists.txt

@@ -167,7 +167,7 @@ include(CTest)
 if (BUILD_TESTING)
     add_test(
         NAME LibWeb
-        COMMAND ${CMAKE_CURRENT_BINARY_DIR}/headless-browser --run-tests ${SERENITY_SOURCE_DIR}/Tests/LibWeb
+        COMMAND ${CMAKE_CURRENT_BINARY_DIR}/../bin/headless-browser --run-tests ${SERENITY_SOURCE_DIR}/Tests/LibWeb
     )
     set_tests_properties(LibWeb PROPERTIES ENVIRONMENT QT_QPA_PLATFORM=offscreen)
 endif()

Meta/Azure/Lagom.yml

@@ -155,12 +155,12 @@ jobs:
           set -e
           ./Meta/check-markdown.sh
           ./Meta/lint-gml-format.sh
-          git ls-files '*.ipc' | xargs ./Meta/Lagom/Build/Tools/IPCMagicLinter/IPCMagicLinter
+          git ls-files '*.ipc' | xargs ./Meta/Lagom/Build/bin/IPCMagicLinter
         displayName: 'Run lints that require Lagom'
         workingDirectory: $(Build.SourcesDirectory)/
         env:
-          MARKDOWN_CHECK_BINARY: ./Meta/Lagom/Build/markdown-check
-          GML_FORMAT: ./Meta/Lagom/Build/gml-format
+          MARKDOWN_CHECK_BINARY: ./Meta/Lagom/Build/bin/markdown-check
+          GML_FORMAT: ./Meta/Lagom/Build/bin/gml-format
           # FIXME: enable detect_stack_use_after_return=1 #7420
           ASAN_OPTIONS: 'strict_string_checks=1:check_initialization_order=1:strict_init_order=1'
           UBSAN_OPTIONS: 'print_stacktrace=1:print_summary=1:halt_on_error=1'

Meta/Lagom/CMakeLists.txt

@@ -80,12 +80,17 @@ include(lagom_compile_options)
 
 include(GNUInstallDirs) # make sure to include before we mess w/RPATH
 
+# Mirror the structure of the installed tree to ensure that rpaths
+# always remain valid.
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}")
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}")
+
 set(CMAKE_SKIP_BUILD_RPATH FALSE)
-set(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE)
+set(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE)
 # See slide 100 of the following ppt :^)
 # https://crascit.com/wp-content/uploads/2019/09/Deep-CMake-For-Library-Authors-Craig-Scott-CppCon-2019.pdf
 if (APPLE)
-    # FIXME: This doesn't work for the full BUILD_LAGOM=ON build, see #10055
     set(CMAKE_MACOSX_RPATH TRUE)
     set(CMAKE_INSTALL_NAME_DIR "@rpath")
     set(CMAKE_INSTALL_RPATH "@loader_path/../lib")
@@ -256,7 +261,6 @@ function(lagom_test source)
         WORKING_DIRECTORY ${LAGOM_TEST_WORKING_DIRECTORY}
     )
     set_target_properties(${name} PROPERTIES LAGOM_WORKING_DIRECTORY "${LAGOM_TEST_WORKING_DIRECTORY}")
-    set_tests_properties(${name} PROPERTIES ENVIRONMENT "LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}")
 endfunction()
 
 function(serenity_test test_src sub_dir)
@@ -463,9 +467,6 @@ if (BUILD_LAGOM)
         SOURCES ${LIBGUI_SOURCES}
         LIBS LibGfx LibSyntax)
 
-    # FIXME: Why doesn't RPATH of the tests handle this properly?
-    set_target_properties(LibSoftGPU PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
-
     # FIXME: LibLocaleData is an object lib in Lagom, because the weak symbol trick we use on serenity
     #    straight up isn't supposed to work per ELF rules
     target_link_libraries(LibLocale PRIVATE LibTimeZone)
@@ -731,8 +732,8 @@ endif()
 if (NOT "$ENV{LAGOM_TARGET}" STREQUAL "")
     add_custom_target(run-lagom-target
         COMMAND "${CMAKE_COMMAND}"
-            -E env "SERENITY_SOURCE_DIR=${SERENITY_PROJECT_ROOT}" "LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}"
-            "$<TARGET_FILE:$ENV{LAGOM_TARGET}>" $ENV{LAGOM_ARGS}
+            -E env "SERENITY_SOURCE_DIR=${SERENITY_PROJECT_ROOT}" "$<TARGET_FILE:$ENV{LAGOM_TARGET}>"
+            $ENV{LAGOM_ARGS}
 
         WORKING_DIRECTORY "$<TARGET_PROPERTY:$ENV{LAGOM_TARGET},LAGOM_WORKING_DIRECTORY>"
         DEPENDS "$<TARGET_FILE:$ENV{LAGOM_TARGET}>"

Meta/check-markdown.sh

@@ -10,12 +10,12 @@ if [ -z "${MARKDOWN_CHECK_BINARY:-}" ] ; then
         echo "Directory Build/lagom/ does not exist. Skipping markdown check."
         exit 0
     fi
-    if ! [ -r Build/lagom/markdown-check ] ; then
+    if ! [ -r Build/lagom/bin/markdown-check ] ; then
         echo "Lagom executable markdown-check was not built. Skipping markdown check."
         echo "To enable this check, you may need to run './Meta/serenity.sh build lagom' first."
         exit 0
     fi
-    MARKDOWN_CHECK_BINARY="Build/lagom/markdown-check"
+    MARKDOWN_CHECK_BINARY="Build/lagom/bin/markdown-check"
 fi
 
 if [ -z "$SERENITY_SOURCE_DIR" ] ; then

Meta/lint-ci.sh

@@ -42,9 +42,9 @@ for cmd in \
     fi
 done
 
-if [ -x ./Build/lagom/Tools/IPCMagicLinter/IPCMagicLinter ]; then
+if [ -x ./Build/lagom/bin/IPCMagicLinter ]; then
     echo "Running IPCMagicLinter"
-    if time { git ls-files '*.ipc' | xargs ./Build/lagom/Tools/IPCMagicLinter/IPCMagicLinter; }; then
+    if time { git ls-files '*.ipc' | xargs ./Build/lagom/bin/IPCMagicLinter; }; then
         echo -e "[${GREEN}OK${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"
     else
         echo -e "[${RED}FAIL${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)"

Meta/lint-gml-format.sh

@@ -10,12 +10,12 @@ if [ -z "${GML_FORMAT:-}" ] ; then
         echo "Directory Build/lagom/ does not exist. Skipping GML formatting."
         exit 0
     fi
-    if ! [ -r Build/lagom/gml-format ] ; then
+    if ! [ -r Build/lagom/bin/gml-format ] ; then
         echo "Lagom executable gml-format was not built. Skipping GML formatting."
         echo "To enable this check, you may need to run './Meta/serenity.sh build lagom' first."
         exit 0
     fi
-    GML_FORMAT="Build/lagom/gml-format"
+    GML_FORMAT="Build/lagom/bin/gml-format"
 fi
 
 if [ "$#" -gt "0" ] ; then

Meta/serenity.sh

@@ -395,7 +395,7 @@ run_gdb() {
         GDB_ARGS+=( "$PASS_ARG_TO_GDB" )
     fi
     if [ "$TARGET" = "lagom" ]; then
-        gdb "$BUILD_DIR/$LAGOM_EXECUTABLE" "${GDB_ARGS[@]}"
+        gdb "$BUILD_DIR/bin/$LAGOM_EXECUTABLE" "${GDB_ARGS[@]}"
     else
         if [ -n "$KERNEL_CMD_LINE" ]; then
             export SERENITY_KERNEL_CMDLINE="$KERNEL_CMD_LINE"

Tests/LibWeb/Layout/layout_test.sh

@@ -10,13 +10,13 @@ if [[ -z "${LADYBIRD_BUILD_DIR}" ]] ; then
     exit 1
 fi
 
-BROWSER_BINARY="./headless-browser"
+BROWSER_BINARY="headless-browser"
 
 find "${SCRIPT_DIR}/input/" -type f -name "*.html" -print0 | while IFS= read -r -d '' input_html_path; do
     input_html_file=${input_html_path/${SCRIPT_DIR}"/input/"/}
     input_html_file=${input_html_file/".html"/}
 
-    output_layout_dump=$(cd "${LADYBIRD_BUILD_DIR}"; timeout 300s "${BROWSER_BINARY}" --layout-test-mode -d "${input_html_path}")
+    output_layout_dump=$(cd "${LADYBIRD_BUILD_DIR}"; timeout 300s "../bin/${BROWSER_BINARY}" --layout-test-mode -d "${input_html_path}")
     expected_layout_dump_path="${SCRIPT_DIR}/expected/${input_html_file}.txt"
 
     if cmp <(echo "${output_layout_dump}") "${expected_layout_dump_path}"; then

Tests/LibWeb/Text/text_test.sh

@@ -10,13 +10,13 @@ if [[ -z "${LADYBIRD_BUILD_DIR}" ]] ; then
     exit 1
 fi
 
-BROWSER_BINARY="./headless-browser"
+BROWSER_BINARY="headless-browser"
 
 find "${SCRIPT_DIR}/input/" -type f -name "*.html" -print0 | while IFS= read -r -d '' input_html_path; do
     input_html_file=${input_html_path/${SCRIPT_DIR}"/input/"/}
     input_html_file=${input_html_file/".html"/}
 
-    output_text_dump=$(cd "${LADYBIRD_BUILD_DIR}"; timeout 300s "${BROWSER_BINARY}" --dump-text "${input_html_path}")
+    output_text_dump=$(cd "${LADYBIRD_BUILD_DIR}"; timeout 300s "../bin/${BROWSER_BINARY}" --dump-text "${input_html_path}")
     expected_text_dump_path="${SCRIPT_DIR}/expected/${input_html_file}.txt"
 
     if cmp <(echo "${output_text_dump}") "${expected_text_dump_path}"; then

Userland/Utilities/markdown-check.cpp

@@ -8,7 +8,7 @@
  * You may want to invoke the checker like this:
  * $ ninja -C Build/lagom
  * $ export SERENITY_SOURCE_DIR=/path/to/serenity
- * $ find AK Base Documentation Kernel Meta Ports Tests Userland -type f -name '*.md' -print0 | xargs -0 Build/lagom/markdown-check README.md CONTRIBUTING.md
+ * $ find AK Base Documentation Kernel Meta Ports Tests Userland -type f -name '*.md' -print0 | xargs -0 Build/lagom/bin/markdown-check README.md CONTRIBUTING.md
  */
 
 #include <AK/Format.h>