From 57e1a4e8b44d4f39383722b7e6a76509b1b110ad Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 14:42:50 -0400 Subject: [PATCH 01/12] actions: add step for checking miral symbols map --- .github/workflows/coverage.yml | 15 ++++++++++++++- src/miral/CMakeLists.txt | 11 +++++++++++ tools/symbols_map_generator/main.py | 6 ++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 85445cbf00b..54a6bde9a18 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -81,6 +81,19 @@ jobs: -B build ${{ github.workspace }} + - name: Install LLVM and Clang for symbols check + uses: KyleMayes/install-llvm-action@v2 + with: + version: "19.0" + + - name: Set environment variables for symbols check + run: | + echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang.so.1" >> $GITHUB_ENV + echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV + + - name: Check miral symbols map + run: cmake --build build --target check-miral-symbols-map + - name: Build run: cmake --build build @@ -110,4 +123,4 @@ jobs: name: Setup upterm session uses: lhotari/action-upterm@v1 with: - limit-access-to-actor: true + limit-access-to-actor: true \ No newline at end of file diff --git a/src/miral/CMakeLists.txt b/src/miral/CMakeLists.txt index 83da698a847..2f8909a879f 100644 --- a/src/miral/CMakeLists.txt +++ b/src/miral/CMakeLists.txt @@ -153,6 +153,17 @@ add_custom_target( --output-symbols WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}") +add_custom_target( + check-miral-symbols-map + COMMAND ${PROJECT_SOURCE_DIR}/tools/symbols_map_generator/run.sh + --library-name miral + --version ${MIRAL_VERSION} + --symbols-map-path="${CMAKE_CURRENT_SOURCE_DIR}/symbols.map" + --external-headers-directory="${PROJECT_SOURCE_DIR}/include/miral" + --include-dirs "$,:>" + --diff + WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}") + configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/miral.pc.in ${CMAKE_CURRENT_BINARY_DIR}/miral.pc diff --git a/tools/symbols_map_generator/main.py b/tools/symbols_map_generator/main.py index 3e0df3e3500..2557cc67efd 100755 --- a/tools/symbols_map_generator/main.py +++ b/tools/symbols_map_generator/main.py @@ -327,6 +327,12 @@ def main(): args = parser.parse_args() logging.basicConfig(level=logging.INFO) + + if args.output_symbols: + _logger.info("symbols_map_generator is running in 'output symbols' mode") + + if args.diff: + _logger.info("symbols_map_generator is running in 'diff' mode") # Point libclang to a file on the system if 'MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH' in os.environ: From 32231570451d05663fe56327b715be745c0bb008 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 14:52:11 -0400 Subject: [PATCH 02/12] Installing llvm via the script' --- .github/workflows/coverage.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 54a6bde9a18..66746f6de57 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -82,9 +82,10 @@ jobs: ${{ github.workspace }} - name: Install LLVM and Clang for symbols check - uses: KyleMayes/install-llvm-action@v2 - with: - version: "19.0" + run: | + wget https://apt.llvm.org/llvm.sh + chmod +x llvm.sh + sudo ./llvm.sh 19 - name: Set environment variables for symbols check run: | From 2b2d5eea0855c9a6d4c7d2b1ef603289f92f8462 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 14:59:18 -0400 Subject: [PATCH 03/12] actions for miroil symbols too --- .github/workflows/coverage.yml | 14 ++++++++------ src/miroil/CMakeLists.txt | 11 +++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 66746f6de57..13e93c9b8d7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -81,19 +81,21 @@ jobs: -B build ${{ github.workspace }} - - name: Install LLVM and Clang for symbols check + - name: Setup symbols check run: | wget https://apt.llvm.org/llvm.sh chmod +x llvm.sh sudo ./llvm.sh 19 - - - name: Set environment variables for symbols check - run: | echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang.so.1" >> $GITHUB_ENV echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV - - name: Check miral symbols map - run: cmake --build build --target check-miral-symbols-map + - name: Check symbols + run: | + echo "Checking miral symbols..." + cmake --build build --target check-miral-symbols-map + + echo "Checking miroil symbols..." + cmake --build build --target check-miroil-symbols-map - name: Build run: cmake --build build diff --git a/src/miroil/CMakeLists.txt b/src/miroil/CMakeLists.txt index 09f124846e0..7c9fc3d04a1 100644 --- a/src/miroil/CMakeLists.txt +++ b/src/miroil/CMakeLists.txt @@ -100,6 +100,17 @@ add_custom_target( --output-symbols WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}") +add_custom_target( + check-miroil-symbols-map + COMMAND ${PROJECT_SOURCE_DIR}/tools/symbols_map_generator/run.sh + --library-name miroil + --version ${MIROIL_VERSION} + --symbols-map-path="${CMAKE_CURRENT_SOURCE_DIR}/symbols.map" + --external-headers-directory="${PROJECT_SOURCE_DIR}/include/miroil" + --include-dirs "$,:>" + --diff + WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}") + install(TARGETS miroil LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}") install(DIRECTORY ${CMAKE_SOURCE_DIR}/include/miroil DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") install(FILES ${CMAKE_CURRENT_BINARY_DIR}/miroil.pc DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig") From b1af0684fad51f2a38d1270ff1e371ad98bef19f Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 15:00:46 -0400 Subject: [PATCH 04/12] Improve output on symbols --- tools/symbols_map_generator/main.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/symbols_map_generator/main.py b/tools/symbols_map_generator/main.py index 2557cc67efd..33ba08de5e0 100755 --- a/tools/symbols_map_generator/main.py +++ b/tools/symbols_map_generator/main.py @@ -266,7 +266,7 @@ def print_symbols_diff(previous_symbols: list[Symbol], new_symbols: set[str], is added_symbols = get_added_symbols(previous_symbols, new_symbols, is_internal) print("") - print("\033[1mNew Symbols 🟒🟒🟒\033[0m") + print(" \033[1mNew Symbols 🟒🟒🟒\033[0m") for s in added_symbols: print(f"\033[92m {s}\033[0m") @@ -280,7 +280,7 @@ def print_symbols_diff(previous_symbols: list[Symbol], new_symbols: set[str], is deleted_symbols.sort() print("") - print("\033[1mDeleted Symbols πŸ”»πŸ”»πŸ”»\033[0m") + print(" \033[1mDeleted Symbols πŸ”»πŸ”»πŸ”»\033[0m") for s in deleted_symbols: print(f"\033[91m {s}\033[0m") @@ -384,7 +384,11 @@ def main(): previous_symbols = read_symbols_from_file(args.symbols_map_path, library) if args.diff: + print("External Symbols Diff:") print_symbols_diff(previous_symbols, external_symbols, False) + + print("") + print("Internal Symbols Diff:") print_symbols_diff(previous_symbols, internal_symbols, True) print("") From 4d3eba4e89e07ff8467f2864e5689c173232930f Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 15:35:44 -0400 Subject: [PATCH 05/12] New symbols check file --- .github/workflows/coverage.yml | 18 +---------- .github/workflows/symbols-check.yml | 48 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/symbols-check.yml diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 13e93c9b8d7..85445cbf00b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -81,22 +81,6 @@ jobs: -B build ${{ github.workspace }} - - name: Setup symbols check - run: | - wget https://apt.llvm.org/llvm.sh - chmod +x llvm.sh - sudo ./llvm.sh 19 - echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang.so.1" >> $GITHUB_ENV - echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV - - - name: Check symbols - run: | - echo "Checking miral symbols..." - cmake --build build --target check-miral-symbols-map - - echo "Checking miroil symbols..." - cmake --build build --target check-miroil-symbols-map - - name: Build run: cmake --build build @@ -126,4 +110,4 @@ jobs: name: Setup upterm session uses: lhotari/action-upterm@v1 with: - limit-access-to-actor: true \ No newline at end of file + limit-access-to-actor: true diff --git a/.github/workflows/symbols-check.yml b/.github/workflows/symbols-check.yml new file mode 100644 index 00000000000..14604acd69d --- /dev/null +++ b/.github/workflows/symbols-check.yml @@ -0,0 +1,48 @@ +name: Symbols Check + +on: + push: + branches: + - main + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.number && format('pr{0}', github.event.number) || github.ref_name }} + cancel-in-progress: true + +jobs: + Run: + runs-on: ubuntu-latest + + timeout-minutes: 10 + steps: + - name: Check out code + uses: actions/checkout@v4 + with: + # So we can determine the merge base + fetch-depth: 0 + + - name: Install dependencies + run: | + sudo apt-get build-dep ./ + + - name: Configure + run: > + cmake -B build ${{ github.workspace }} + + - name: Install clang + run: | + wget https://apt.llvm.org/llvm.sh + chmod +x llvm.sh + sudo ./llvm.sh 19 + echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang.so.1" >> $GITHUB_ENV + echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV + + - name: Check miral symbols + run: | + cmake --build build --target check-miral-symbols-map + + - name: Check miroil symbols + run: | + cmake --build build --target check-miroil-symbols-map \ No newline at end of file From f392770a36f72a1b6284821b496af4ce741c1e05 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 15:41:57 -0400 Subject: [PATCH 06/12] reporting changed symbols with an exit code --- tools/symbols_map_generator/main.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/symbols_map_generator/main.py b/tools/symbols_map_generator/main.py index 33ba08de5e0..ed39231f71b 100755 --- a/tools/symbols_map_generator/main.py +++ b/tools/symbols_map_generator/main.py @@ -259,7 +259,7 @@ def get_added_symbols(previous_symbols: list[Symbol], new_symbols: set[str], is_ return added_symbols -def print_symbols_diff(previous_symbols: list[Symbol], new_symbols: set[str], is_internal: bool): +def report_symbols_diff(previous_symbols: list[Symbol], new_symbols: set[str], is_internal: bool): """ Prints the diff between the previous symbols and the new symbols. """ @@ -284,6 +284,8 @@ def print_symbols_diff(previous_symbols: list[Symbol], new_symbols: set[str], is for s in deleted_symbols: print(f"\033[91m {s}\033[0m") + return len(deleted_symbols) > 0 or len(added_symbols) > 0 + def main(): parser = argparse.ArgumentParser(description="This tool parses the header files of a provided in the Mir project " @@ -383,13 +385,14 @@ def main(): previous_symbols = read_symbols_from_file(args.symbols_map_path, library) + has_changed_symbols = False if args.diff: print("External Symbols Diff:") - print_symbols_diff(previous_symbols, external_symbols, False) + has_changed_symbols = report_symbols_diff(previous_symbols, external_symbols, False) print("") print("Internal Symbols Diff:") - print_symbols_diff(previous_symbols, internal_symbols, True) + has_changed_symbols = has_changed_symbols or report_symbols_diff(previous_symbols, internal_symbols, True) print("") if args.output_symbols: @@ -476,8 +479,10 @@ def main(): f.write(output_str) - - exit(0) + if has_changed_symbols: + exit(1) + else: + exit(0) From 7169146e1894952ea8aed14342ca1dbdf2cb2825 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 16:06:12 -0400 Subject: [PATCH 07/12] Quick test! --- include/miral/miral/output.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/miral/miral/output.h b/include/miral/miral/output.h index a0920ccdb1d..90effa874c3 100644 --- a/include/miral/miral/output.h +++ b/include/miral/miral/output.h @@ -64,6 +64,8 @@ class Output /// The type of the output. auto type() const -> Type; + auto get_x() const -> int; + /// The physical size of the output. auto physical_size_mm() const -> PhysicalSizeMM; From fa49ce1d2516c42fd8ea4f2e8071a67665b6c8f2 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 18 Apr 2024 16:08:09 -0400 Subject: [PATCH 08/12] Revert "Quick test!" This reverts commit 7169146e1894952ea8aed14342ca1dbdf2cb2825. --- include/miral/miral/output.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/miral/miral/output.h b/include/miral/miral/output.h index 90effa874c3..a0920ccdb1d 100644 --- a/include/miral/miral/output.h +++ b/include/miral/miral/output.h @@ -64,8 +64,6 @@ class Output /// The type of the output. auto type() const -> Type; - auto get_x() const -> int; - /// The physical size of the output. auto physical_size_mm() const -> PhysicalSizeMM; From 0ab1bdb3d123f3500eba6b13953b53e83fd3287b Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 19 Apr 2024 10:03:12 -0400 Subject: [PATCH 09/12] Getting llvm from the repo --- .github/workflows/symbols-check.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/symbols-check.yml b/.github/workflows/symbols-check.yml index 14604acd69d..78a3b13d799 100644 --- a/.github/workflows/symbols-check.yml +++ b/.github/workflows/symbols-check.yml @@ -33,10 +33,10 @@ jobs: - name: Install clang run: | - wget https://apt.llvm.org/llvm.sh - chmod +x llvm.sh - sudo ./llvm.sh 19 - echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang.so.1" >> $GITHUB_ENV + wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key|sudo apt-key add - + sudo add-apt-repository "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy main" + sudo apt install libclang1-19 + echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang-19.so.1" >> $GITHUB_ENV echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV - name: Check miral symbols From 0fcd40b96075a90e1f58ee492b191e524649e65d Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 19 Apr 2024 10:57:45 -0400 Subject: [PATCH 10/12] style: combining args.diff with has_changed_symbols check when returning exit(1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: MichaΕ‚ Sawicz --- tools/symbols_map_generator/main.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/symbols_map_generator/main.py b/tools/symbols_map_generator/main.py index ed39231f71b..5933cc87010 100755 --- a/tools/symbols_map_generator/main.py +++ b/tools/symbols_map_generator/main.py @@ -479,10 +479,8 @@ def main(): f.write(output_str) - if has_changed_symbols: + if args.diff and has_changed_symbols: exit(1) - else: - exit(0) From 7bd31704d4d532dc00aa41e84dd008892738b9fe Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 19 Apr 2024 10:59:17 -0400 Subject: [PATCH 11/12] actions: rearranging various parts --- .github/workflows/symbols-check.yml | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/.github/workflows/symbols-check.yml b/.github/workflows/symbols-check.yml index 78a3b13d799..3dd21867e49 100644 --- a/.github/workflows/symbols-check.yml +++ b/.github/workflows/symbols-check.yml @@ -19,30 +19,23 @@ jobs: steps: - name: Check out code uses: actions/checkout@v4 - with: - # So we can determine the merge base - fetch-depth: 0 - name: Install dependencies run: | sudo apt-get build-dep ./ - - - name: Configure - run: > - cmake -B build ${{ github.workspace }} - - - name: Install clang - run: | wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key|sudo apt-key add - sudo add-apt-repository "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy main" sudo apt install libclang1-19 echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang-19.so.1" >> $GITHUB_ENV echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV - - name: Check miral symbols - run: | - cmake --build build --target check-miral-symbols-map + - name: Configure + run: > + cmake -B build ${{ github.workspace }} - - name: Check miroil symbols + - name: Check symbols run: | - cmake --build build --target check-miroil-symbols-map \ No newline at end of file + RET=0 + cmake --build build --target check-miral-symbols-map || RET=$? + cmake --build build --target check-miroil-symbols-map || RET=$? + exit $RET \ No newline at end of file From f0062ba462c279c3a7fb5dac4c4bbcc3cd84b6a2 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 19 Apr 2024 11:23:44 -0400 Subject: [PATCH 12/12] minor: move apt-get build-dep after llvm install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: MichaΕ‚ Sawicz --- .github/workflows/symbols-check.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/symbols-check.yml b/.github/workflows/symbols-check.yml index 3dd21867e49..a02d7b049ce 100644 --- a/.github/workflows/symbols-check.yml +++ b/.github/workflows/symbols-check.yml @@ -28,6 +28,7 @@ jobs: sudo apt install libclang1-19 echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang-19.so.1" >> $GITHUB_ENV echo "MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib" >> $GITHUB_ENV + sudo apt-get build-dep ./ - name: Configure run: >