From 72a1f7864711ab4fa3fa59818b29ac4cb3d4b293 Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Fri, 12 May 2023 11:22:21 +0200 Subject: [PATCH] Run iOS size test on CI through Bazel (#1122) --- .github/workflows/ios-ci.yml | 28 ++++- ...-results.yml => pr-render-test-result.yml} | 4 +- .github/workflows/pr-size-test-result.yml | 85 ++++++++++++++ BUILD.bazel | 10 ++ bazel/flags.bzl | 5 +- platform/ios/.gitignore | 1 + platform/ios/BUILD.bazel | 105 +++++++++++++----- platform/ios/bazel/macros.bzl | 20 ++++ src/mbgl/util/version.cpp | 4 + 9 files changed, 226 insertions(+), 36 deletions(-) rename .github/workflows/{upload-render-test-results.yml => pr-render-test-result.yml} (97%) create mode 100644 .github/workflows/pr-size-test-result.yml create mode 100644 platform/ios/bazel/macros.bzl diff --git a/.github/workflows/ios-ci.yml b/.github/workflows/ios-ci.yml index fbc97455f..e3e5c2fb7 100644 --- a/.github/workflows/ios-ci.yml +++ b/.github/workflows/ios-ci.yml @@ -114,11 +114,13 @@ jobs: ccache --max-size=2G ccache --show-stats - - name: Install iOS packaging dependencies - run: | - ./platform/ios/scripts/install-packaging-dependencies.sh + - name: Save PR number + if: github.event_name == 'pull_request' + working-directory: ./ env: - CILAUNCH: true + PR_NUMBER: ${{ github.event.number }} + run: | + echo $PR_NUMBER > ./pr_number - name: Check debug symbols run: make darwin-check-public-symbols @@ -132,6 +134,24 @@ jobs: - name: Running ios UI tests run: make ios-uitest + # size test + + - name: Build app for size test & output size + working-directory: ./ + run: | + bazelisk build --compilation_mode=opt //platform/ios:size + cp $(bazel cquery --compilation_mode=opt --output=files //platform/ios:size) ./size + + - name: Upload size test result + uses: actions/upload-artifact@v3 + with: + name: size-test-result + path: | + ./size + ./pr_number + + # --- + - name: Build and run SDK unit tests with thread and undefined behavior sanitizers run: make ios-sanitize diff --git a/.github/workflows/upload-render-test-results.yml b/.github/workflows/pr-render-test-result.yml similarity index 97% rename from .github/workflows/upload-render-test-results.yml rename to .github/workflows/pr-render-test-result.yml index 7c1d1daf3..af25bfa72 100644 --- a/.github/workflows/upload-render-test-results.yml +++ b/.github/workflows/pr-render-test-result.yml @@ -1,4 +1,4 @@ -name: upload-render-test-results +name: pr-upload-render-test-result permissions: pull-requests: write # This is required to leave a comment on the PR @@ -11,7 +11,7 @@ on: - completed jobs: - upload-render-test-results: + upload-render-test-result: runs-on: ubuntu-22.04 if: github.event.workflow_run.event == 'pull_request' steps: diff --git a/.github/workflows/pr-size-test-result.yml b/.github/workflows/pr-size-test-result.yml new file mode 100644 index 000000000..dfa1b384e --- /dev/null +++ b/.github/workflows/pr-size-test-result.yml @@ -0,0 +1,85 @@ +name: pr-size-test-result + +on: + workflow_run: + workflows: [ios-ci] + types: + - completed + +jobs: + comment-on-pr: + runs-on: ubuntu-22.04 + steps: + # https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run + # TODO: de-duplicate with re-usable action + - name: 'Download size-test-result artifact' + uses: actions/github-script@v6 + with: + script: | + let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.payload.workflow_run.id, + }); + let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => { + return artifact.name == "size-test-result" + })[0]; + let download = await github.rest.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: matchArtifact.id, + archive_format: 'zip', + }); + let fs = require('fs'); + fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/size-test-result.zip`, Buffer.from(download.data)); + + - name: 'Unzip render-test-result artifact' + run: unzip size-test-result.zip + + - name: Show directory structure + run: ls -R + + # Move ./size artifact to ./size_main and store as cache + + - run: mv size size_main + if: github.event.workflow_run.event != 'pull_request' + + - name: Save size + if: github.event.workflow_run.event != 'pull_request' + uses: actions/cache/save@v3 + with: + path: | + size_main + key: ios-size + + # Restore cache so we can output the difference + + - name: Restore size + if: github.event.workflow_run.event == 'pull_request' + uses: actions/cache/restore@v3 + with: + path: | + size_main + key: ios-size + + - run: | + echo pr_number="$(cat ./pr_number)" >> "$GITHUB_ENV" + echo old_size="$(cat ./size_main)" >> "$GITHUB_ENV" + echo new_size="$(cat ./size)" >> "$GITHUB_ENV" + echo percentage_change=$(awk 'NR==FNR{a=$0;next}{printf "%+.2f%%\n", 100*($0-a)/a}' $new_size $old_size) >> "$GITHUB_ENV" + if: github.event.workflow_run.event == 'pull_request' + + - name: 'Leave comment on PR with test results' + if: github.event.workflow_run.event == 'pull_request' + uses: marocchino/sticky-pull-request-comment@v2 + with: + number: ${{ env.pr_number }} + hide_and_recreate: true + hide_classify: "OUTDATED" + message: | + ### Size test result + + Old size: ${{ env.old_size }} bytes + New size: ${{ env.new_size }} bytes + Difference: **${{ env.percentage_change }}** + diff --git a/BUILD.bazel b/BUILD.bazel index ce601ad5a..4304c42d6 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -45,4 +45,14 @@ cc_library( ], ) +genrule( + name = "git_hash", + local = True, + outs = ["hash"], + visibility = ["//visibility:public"], + cmd = """ + git rev-parse HEAD > $@ + """ +) + exports_files(["LICENSE.md"], visibility = ["//visibility:public"]) diff --git a/bazel/flags.bzl b/bazel/flags.bzl index 3a0d33bca..677739c7e 100644 --- a/bazel/flags.bzl +++ b/bazel/flags.bzl @@ -25,11 +25,10 @@ Compilation flags used for all .cpp and .mm targets. CPP_FLAGS = WARNING_FLAGS + [ "-fexceptions", - "-frtti", + "-fno-rtti", "-ftemplate-depth=1024", - "-std=c++17", + "-std=c++17" ] - """ Compilation flags related to the Maplibre codebase. Relevant for all .cpp .mm and .m code - src/* diff --git a/platform/ios/.gitignore b/platform/ios/.gitignore index f554384c3..b2d1f64c9 100644 --- a/platform/ios/.gitignore +++ b/platform/ios/.gitignore @@ -37,6 +37,7 @@ test/fixtures/storage/assets.zip /vendor/.cache buck-out .buckd +MapLibre.xcodeproj # Visual Studio Code .vscode diff --git a/platform/ios/BUILD.bazel b/platform/ios/BUILD.bazel index 39815036a..ae1333b98 100644 --- a/platform/ios/BUILD.bazel +++ b/platform/ios/BUILD.bazel @@ -7,6 +7,7 @@ load( "top_level_target", "xcodeproj", ) +load("bazel/macros.bzl", "info_plist") # This list of public headers is from what Mapbox vendors with their dynamic framework # If we forget one the build will fail, and if we add too many there will be a warning @@ -105,6 +106,18 @@ apple_static_xcframework( deps = ["sdk"], ) +info_plist( + name = "info_plist", + out = "Info.plist", + base_info_plist = "platform/ios/framework/Info.plist", +) + +info_plist( + name = "info_static_plist", + out = "Info-static.plist", + base_info_plist = "platform/ios/framework/Info-static.plist" +) + apple_xcframework( name = "Mapbox.dynamic", bundle_id = "com.maplibre.mapbox", @@ -113,7 +126,7 @@ apple_xcframework( "platform/ios/resources/*.lproj/**", "platform/ios/resources/*.xcassets/**", ]), - infoplists = ["build/Info.plist"], + infoplists = ["info_plist"], ios = { "simulator": [ "x86_64", @@ -128,14 +141,12 @@ apple_xcframework( deps = ["sdk-dynamic"], ) -# This target allows you to link the SDK to emulate linking it in the app -# This should help us find issues with missing symbols sooner. ios_framework( - name = "Mapbox.link", + name = "MapLibre.link", bundle_id = "com.maplibre.link", families = ["iphone"], - infoplists = ["build/Info.plist"], - linkopts = ["-all_load"], + infoplists = ["info_plist"], + linkopts = [""], minimum_os_version = "12.0", deps = ["sdk"], ) @@ -187,7 +198,7 @@ apple_resource_bundle( name = "resources", bundle_id = "com.maplibre.mapbox", bundle_name = "Mapbox", - infoplists = ["build/Info-static.plist"], + infoplists = ["info_static_plist"], resources = glob([ "platform/ios/resources/*.lproj/**", "platform/ios/resources/*.xcassets/**", @@ -367,6 +378,11 @@ objc_library( ], ) +objc_library( + name = "sdk-headers", + hdrs = ["platform/ios/src/Mapbox.h"] + _PUBLIC_HEADERS, +) + objc_library( name = "iosapp", srcs = [ @@ -385,7 +401,8 @@ objc_library( "platform/ios/app/MBXOrnamentsViewController.m", "platform/ios/app/MBXStateManager.m", "platform/darwin/app/LimeGreenStyleLayer.m", - "platform/darwin/app/LimeGreenStyleLayer.h" + "platform/darwin/app/LimeGreenStyleLayer.h", + "platform/ios/app/MBXCustomCalloutView.h", ], hdrs = [ "platform/ios/app/MBXAppDelegate.h", @@ -393,7 +410,6 @@ objc_library( "platform/ios/app/MBXCustomLocationViewController.h", "platform/ios/app/MBXSnapshotsViewController.h", "platform/ios/app/MBXFrameTimeGraphView.h", - "platform/ios/app/MBXCustomCalloutView.h", "platform/ios/app/MBXViewController.h", "platform/ios/app/MBXUserLocationAnnotationView.h", "platform/ios/app/MBXAnnotationView.h", @@ -404,28 +420,19 @@ objc_library( "platform/ios/app/MLNMapView_Experimental.h" ], includes = [ - "platform/darwin/app" - ], - deps = [ - "sdk" + "platform/darwin/app", + "platform/ios/src", + "platform/darwin/src" ], sdk_frameworks = [ "GLKit" # needed for LimeGreenStyleLayer - ] + ], + deps = [ + "sdk-headers" + ], ) - -ios_application( - name = "App", - bundle_id = "com.maplibre.app", - families = ["iphone", "ipad"], - infoplists = ["platform/ios/app/Info.plist"], - minimum_os_version = "12.0", - visibility = ["//visibility:public"], - deps = [ - "iosapp", - ], - resources = [ +_IOS_APPLICATION_RESOURCES = [ "platform/ios/app/numeric_filter_style.json", "platform/ios/app/missing_icon.json", "platform/ios/app/points.geojson", @@ -439,8 +446,52 @@ ios_application( "platform/ios/app/Assets.xcassets", "platform/darwin/test/amsterdam.geojson" ] + glob([ - "platform/ios/app/*.lproj/**" + "platform/ios/app/*.lproj/**", ]) + +ios_application( + name = "App", + bundle_id = "com.maplibre.app", + families = ["iphone", "ipad"], + infoplists = ["platform/ios/app/Info.plist"], + minimum_os_version = "12.0", + visibility = ["//visibility:public"], + deps = [ + "sdk", + "iosapp", + ], + resources = _IOS_APPLICATION_RESOURCES +) + +ios_application( + name = "AppDynamicLib", + bundle_id = "com.maplibre.app", + families = ["iphone", "ipad"], + infoplists = ["platform/ios/app/Info.plist"], + minimum_os_version = "12.0", + visibility = ["//visibility:public"], + deps = [ + "iosapp", + ], + frameworks = [ + "MapLibre.link" + ], + resources = _IOS_APPLICATION_RESOURCES +) + +# Outputs a file with the size of MapLibre as a dynamic library in bytes +genrule( + name = "size", + srcs = [ + "AppDynamicLib" + ], + outs = [ + "size-out" + ], + cmd = """ + unzip $(location AppDynamicLib) + stat -f%z ./Payload/AppDynamicLib.app/Frameworks/MapLibre.link.framework/MapLibre.link > $@ + """, ) xcodeproj( diff --git a/platform/ios/bazel/macros.bzl b/platform/ios/bazel/macros.bzl new file mode 100644 index 000000000..b1406a664 --- /dev/null +++ b/platform/ios/bazel/macros.bzl @@ -0,0 +1,20 @@ +def info_plist(name, base_info_plist, out, **kwargs): + native.genrule( + name = name, + srcs = [ + base_info_plist, + "//:git_hash" + ], + outs = [ + out + ], + cmd = """ + cp $(location {}) $@ + plutil -replace MLNCommitHash -string $$(cat $(location //:git_hash)) $@ + + # TODO: move to file + sem_version=0.0.0 + plutil -replace MLNSemanticVersionString -string $$sem_version $@ + """.format(base_info_plist), + **kwargs +) \ No newline at end of file diff --git a/src/mbgl/util/version.cpp b/src/mbgl/util/version.cpp index fcb31f0b7..67bf5fe90 100644 --- a/src/mbgl/util/version.cpp +++ b/src/mbgl/util/version.cpp @@ -3,7 +3,11 @@ namespace mbgl { namespace version { +#ifdef MBGL_VERSION_REV const char* revision = MBGL_VERSION_REV; +#else +const char* revision = "NOT SET"; +#endif } // namespace version } // namespace mbgl