Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

actions: add step for checking miral and miroil symbols map #3336

Merged
merged 12 commits into from
Apr 19, 2024
42 changes: 42 additions & 0 deletions .github/workflows/symbols-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
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

- name: Install dependencies
run: |
sudo apt-get build-dep ./
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the expensive step :)

Suggested change
sudo apt-get build-dep ./

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want the build deps so that 3rd party libraries resolve in clang, right?

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
mattkae marked this conversation as resolved.
Show resolved Hide resolved
sudo apt-get build-dep ./

- name: Configure
run: >
cmake -B build ${{ github.workspace }}

- name: Check symbols
run: |
RET=0
cmake --build build --target check-miral-symbols-map || RET=$?
cmake --build build --target check-miroil-symbols-map || RET=$?
exit $RET
11 changes: 11 additions & 0 deletions src/miral/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$<JOIN:$<TARGET_PROPERTY:miral,INCLUDE_DIRECTORIES>,:>"
--diff
WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}")

configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/miral.pc.in
${CMAKE_CURRENT_BINARY_DIR}/miral.pc
Expand Down
11 changes: 11 additions & 0 deletions src/miroil/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$<JOIN:$<TARGET_PROPERTY:miroil,INCLUDE_DIRECTORIES>,:>"
--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")
27 changes: 20 additions & 7 deletions tools/symbols_map_generator/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,14 +259,14 @@ 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.
"""
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")

Expand All @@ -280,10 +280,12 @@ 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")

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 "
Expand Down Expand Up @@ -327,6 +329,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:
Expand Down Expand Up @@ -377,9 +385,14 @@ def main():

previous_symbols = read_symbols_from_file(args.symbols_map_path, library)

has_changed_symbols = False
if args.diff:
print_symbols_diff(previous_symbols, external_symbols, False)
print_symbols_diff(previous_symbols, internal_symbols, True)
print("External Symbols Diff:")
has_changed_symbols = report_symbols_diff(previous_symbols, external_symbols, False)

print("")
print("Internal Symbols Diff:")
has_changed_symbols = has_changed_symbols or report_symbols_diff(previous_symbols, internal_symbols, True)
print("")

if args.output_symbols:
Expand Down Expand Up @@ -466,8 +479,8 @@ def main():

f.write(output_str)


exit(0)
if args.diff and has_changed_symbols:
exit(1)



Expand Down
Loading