From 71b184accf5e94e24cdd9cba23f70a62cd5852e6 Mon Sep 17 00:00:00 2001 From: implicitfield <114500360+implicitfield@users.noreply.github.com> Date: Sat, 18 Mar 2023 19:23:35 +0400 Subject: [PATCH] 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 | 2 +- .github/workflows/wasm.yml | 4 ++-- Ladybird/CMakeLists.txt | 2 +- Meta/Azure/Lagom.yml | 6 +++--- Meta/Lagom/CMakeLists.txt | 17 +++++++++-------- Meta/check-markdown.sh | 4 ++-- Meta/lint-ci.sh | 4 ++-- Meta/lint-gml-format.sh | 4 ++-- Meta/serenity.sh | 2 +- Tests/LibWeb/Layout/layout_test.sh | 4 ++-- Tests/LibWeb/Text/text_test.sh | 4 ++-- Userland/Utilities/markdown-check.cpp | 2 +- 12 files changed, 28 insertions(+), 27 deletions(-) diff --git a/.github/workflows/libjs-test262.yml b/.github/workflows/libjs-test262.yml index deaf134d4a5..6b6a10dbc47 100644 --- a/.github/workflows/libjs-test262.yml +++ b/.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), diff --git a/.github/workflows/wasm.yml b/.github/workflows/wasm.yml index e5bce318d4b..4bb046df0ef 100644 --- a/.github/workflows/wasm.yml +++ b/.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" diff --git a/Ladybird/CMakeLists.txt b/Ladybird/CMakeLists.txt index 410714c1478..7e66e4c8c9a 100644 --- a/Ladybird/CMakeLists.txt +++ b/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() diff --git a/Meta/Azure/Lagom.yml b/Meta/Azure/Lagom.yml index 3a40bab4d9e..7fb5bd2dfec 100644 --- a/Meta/Azure/Lagom.yml +++ b/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' diff --git a/Meta/Lagom/CMakeLists.txt b/Meta/Lagom/CMakeLists.txt index 3f710a8be01..641c03ab75f 100644 --- a/Meta/Lagom/CMakeLists.txt +++ b/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}" - "$" $ENV{LAGOM_ARGS} + -E env "SERENITY_SOURCE_DIR=${SERENITY_PROJECT_ROOT}" "$" + $ENV{LAGOM_ARGS} WORKING_DIRECTORY "$" DEPENDS "$" diff --git a/Meta/check-markdown.sh b/Meta/check-markdown.sh index 6132ca303b7..925d7f31d77 100755 --- a/Meta/check-markdown.sh +++ b/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 diff --git a/Meta/lint-ci.sh b/Meta/lint-ci.sh index 19784253c78..ee5938d0ec4 100755 --- a/Meta/lint-ci.sh +++ b/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)" diff --git a/Meta/lint-gml-format.sh b/Meta/lint-gml-format.sh index c23b45242a4..85f51238f06 100755 --- a/Meta/lint-gml-format.sh +++ b/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 diff --git a/Meta/serenity.sh b/Meta/serenity.sh index 35c16795216..a81fcaec40f 100755 --- a/Meta/serenity.sh +++ b/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" diff --git a/Tests/LibWeb/Layout/layout_test.sh b/Tests/LibWeb/Layout/layout_test.sh index e52103ac497..7b08006394d 100755 --- a/Tests/LibWeb/Layout/layout_test.sh +++ b/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 diff --git a/Tests/LibWeb/Text/text_test.sh b/Tests/LibWeb/Text/text_test.sh index 5a661bf036b..2abeb345edc 100755 --- a/Tests/LibWeb/Text/text_test.sh +++ b/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 diff --git a/Userland/Utilities/markdown-check.cpp b/Userland/Utilities/markdown-check.cpp index 8f4bea46794..2d19bc1a326 100644 --- a/Userland/Utilities/markdown-check.cpp +++ b/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