From 04404ea4828312941c4cae33578443c8b189590c Mon Sep 17 00:00:00 2001 From: Edward Chen <18449977+edgchen1@users.noreply.github.com> Date: Mon, 14 Oct 2024 09:24:38 -0700 Subject: [PATCH] Fix Xcode 16 iOS build issues (#22379) - Work around Xcode 16 iOS test build issue: `error: Multiple commands produce '.../PlugIns'`. - Fix link error in iOS static framework test. - Update build.py to check for the right kind of build before running iOS tests on the simulator. - Update Xcode 16 build images to 'macos-15' because that's the only image that will have Xcode 16 soon. See https://github.com/actions/runner-images/issues/10703. --- .github/workflows/mac.yml | 19 +++-- cmake/onnxruntime.cmake | 19 ++++- cmake/onnxruntime_unittests.cmake | 20 ++++- tools/ci_build/build.py | 135 +++++++++++++++++------------- 4 files changed, 122 insertions(+), 71 deletions(-) diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index d1a4366da45e2..b36b0aa555940 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -20,7 +20,7 @@ env: jobs: ARM64-Xcode16: - runs-on: macos-14 + runs-on: macos-15 env: xcode_version: 16 @@ -60,12 +60,16 @@ jobs: --use_xnnpack \ --use_binskim_compliant_compile_flags - ARM64-Xcode16-targeting-iphonesimulator-x86_64: - runs-on: macos-14 + ARM64-Xcode16-targeting-iphonesimulator: + runs-on: macos-15 env: xcode_version: 16 + strategy: + matrix: + target_arch: [x86_64, arm64] + timeout-minutes: 60 steps: @@ -87,16 +91,14 @@ jobs: - uses: actions/checkout@v4 - # Note: Setting onnxruntime_BUILD_UNIT_TESTS=OFF as a workaround for - # https://github.com/microsoft/onnxruntime/issues/22245. - - name: Build + - name: Build for iphonesimulator ${{ matrix.target_arch }} shell: bash run: | python ./tools/ci_build/build.py \ --build_dir ./build \ --update \ --build --parallel \ - --skip_tests \ + --test \ --build_apple_framework \ --use_xcode \ --use_coreml \ @@ -105,8 +107,7 @@ jobs: --ios \ --apple_deploy_target=13.0 \ --apple_sysroot=iphonesimulator \ - --osx_arch=x86_64 \ - --cmake_extra_defines=onnxruntime_BUILD_UNIT_TESTS=OFF + --osx_arch=${{ matrix.target_arch }} Vcpkg: runs-on: macos-13 diff --git a/cmake/onnxruntime.cmake b/cmake/onnxruntime.cmake index 08b8ca0cb66de..9602e54f3bc2d 100644 --- a/cmake/onnxruntime.cmake +++ b/cmake/onnxruntime.cmake @@ -393,8 +393,23 @@ if(onnxruntime_BUILD_APPLE_FRAMEWORK) list(APPEND lib_and_dependencies ${cur_target}) - get_target_property(link_libraries ${cur_target} LINK_LIBRARIES) - foreach(dependency ${link_libraries}) + set(all_link_libraries) + + get_property(link_libraries_set TARGET ${cur_target} PROPERTY LINK_LIBRARIES SET) + if(link_libraries_set) + get_target_property(link_libraries ${cur_target} LINK_LIBRARIES) + list(APPEND all_link_libraries ${link_libraries}) + endif() + + get_property(interface_link_libraries_set TARGET ${cur_target} PROPERTY INTERFACE_LINK_LIBRARIES SET) + if(interface_link_libraries_set) + get_target_property(interface_link_libraries ${cur_target} INTERFACE_LINK_LIBRARIES) + list(APPEND all_link_libraries ${interface_link_libraries}) + endif() + + list(REMOVE_DUPLICATES all_link_libraries) + + foreach(dependency ${all_link_libraries}) if(TARGET ${dependency}) process(${dependency}) endif() diff --git a/cmake/onnxruntime_unittests.cmake b/cmake/onnxruntime_unittests.cmake index 619c3a784d5f9..217ee7cab12d5 100644 --- a/cmake/onnxruntime_unittests.cmake +++ b/cmake/onnxruntime_unittests.cmake @@ -169,7 +169,25 @@ function(AddTest) MACOSX_BUNDLE_SHORT_VERSION_STRING ${ORT_VERSION} XCODE_ATTRIBUTE_ENABLE_BITCODE "NO") - xctest_add_test(xctest.${_UT_TARGET} ${_UT_TARGET}_xc) + # This is a workaround for an Xcode 16 / CMake issue: + # error: Multiple commands produce '/Debug/Debug-iphonesimulator/onnxruntime_test_all.app/PlugIns' + # note: CreateBuildDirectory /Debug/Debug-iphonesimulator/onnxruntime_test_all.app/PlugIns + # note: Target 'onnxruntime_test_all' (project 'onnxruntime') has create directory command with output + # '/Debug/Debug-iphonesimulator/onnxruntime_test_all.app/PlugIns' + # + # It seems related to the test target (e.g., onnxruntime_test_all_xc) LIBRARY_OUTPUT_DIRECTORY property getting set + # to "$/PlugIns" in xctest_add_bundle(): + # https://github.com/Kitware/CMake/blob/9c4a0a9ff09735b847bbbc38caf6da7f6c7238f2/Modules/FindXCTest.cmake#L159-L168 + # + # This is the related CMake issue: https://gitlab.kitware.com/cmake/cmake/-/issues/26301 + # + # Unsetting LIBRARY_OUTPUT_DIRECTORY avoids the build error. + set_property(TARGET ${_UT_TARGET}_xc PROPERTY LIBRARY_OUTPUT_DIRECTORY) + + # Don't bother calling xctest_add_test() because we don't use CTest to run tests on iOS. + # Instead, we can call 'xcodebuild test-without-building' and specify a '-destination' referring to an iOS + # simulator or device. + # xctest_add_test(xctest.${_UT_TARGET} ${_UT_TARGET}_xc) else() if (CMAKE_SYSTEM_NAME STREQUAL "Emscripten") # We might have already executed the following "find_program" code when we build ORT nodejs binding. diff --git a/tools/ci_build/build.py b/tools/ci_build/build.py index ed87640b9e0d8..384569997b9b6 100644 --- a/tools/ci_build/build.py +++ b/tools/ci_build/build.py @@ -1870,6 +1870,10 @@ def setup_rocm_build(args): def run_android_tests(args, source_dir, build_dir, config, cwd): + if args.android_abi != "x86_64": + log.info(f"--android_abi ({args.android_abi}) is not x86_64, skipping running of Android tests on emulator.") + return + sdk_tool_paths = android.get_sdk_tool_paths(args.android_sdk_path) device_dir = "/data/local/tmp" @@ -1891,72 +1895,85 @@ def run_adb_shell(cmd): else: adb_shell(f"cd {device_dir} && {cmd}") - if args.android_abi == "x86_64": - with contextlib.ExitStack() as context_stack: - if args.android_run_emulator: - avd_name = "ort_android" - system_image = f"system-images;android-{args.android_api};default;{args.android_abi}" - - android.create_virtual_device(sdk_tool_paths, system_image, avd_name) - emulator_proc = context_stack.enter_context( - android.start_emulator( - sdk_tool_paths=sdk_tool_paths, - avd_name=avd_name, - extra_args=["-partition-size", "2047", "-wipe-data"], - ) + with contextlib.ExitStack() as context_stack: + if args.android_run_emulator: + avd_name = "ort_android" + system_image = f"system-images;android-{args.android_api};default;{args.android_abi}" + + android.create_virtual_device(sdk_tool_paths, system_image, avd_name) + emulator_proc = context_stack.enter_context( + android.start_emulator( + sdk_tool_paths=sdk_tool_paths, + avd_name=avd_name, + extra_args=["-partition-size", "2047", "-wipe-data"], ) - context_stack.callback(android.stop_emulator, emulator_proc) + ) + context_stack.callback(android.stop_emulator, emulator_proc) + + adb_push("testdata", device_dir, cwd=cwd) + adb_push(os.path.join(source_dir, "cmake", "external", "onnx", "onnx", "backend", "test"), device_dir, cwd=cwd) + adb_push("onnxruntime_test_all", device_dir, cwd=cwd) + adb_shell(f"chmod +x {device_dir}/onnxruntime_test_all") + adb_push("onnx_test_runner", device_dir, cwd=cwd) + adb_shell(f"chmod +x {device_dir}/onnx_test_runner") + run_adb_shell(f"{device_dir}/onnxruntime_test_all") + + # remove onnxruntime_test_all as it takes up a _lot_ of space and can cause insufficient storage errors + # when we try to copy the java app to the device. + adb_shell(f"rm {device_dir}/onnxruntime_test_all") + + if args.build_java: + # use the gradle wrapper under /java + gradle_executable = os.path.join(source_dir, "java", "gradlew.bat" if is_windows() else "gradlew") + android_test_path = os.path.join(cwd, "java", "androidtest", "android") + run_subprocess( + [ + gradle_executable, + "--no-daemon", + f"-DminSdkVer={args.android_api}", + "clean", + "connectedDebugAndroidTest", + ], + cwd=android_test_path, + ) - adb_push("testdata", device_dir, cwd=cwd) - adb_push( - os.path.join(source_dir, "cmake", "external", "onnx", "onnx", "backend", "test"), device_dir, cwd=cwd + if args.use_nnapi: + run_adb_shell(f"{device_dir}/onnx_test_runner -e nnapi {device_dir}/test") + else: + run_adb_shell(f"{device_dir}/onnx_test_runner {device_dir}/test") + + # run shared_lib_test if necessary + if args.build_shared_lib: + adb_push("libonnxruntime.so", device_dir, cwd=cwd) + adb_push("onnxruntime_shared_lib_test", device_dir, cwd=cwd) + adb_push("libcustom_op_library.so", device_dir, cwd=cwd) + adb_push("libcustom_op_get_const_input_test_library.so", device_dir, cwd=cwd) + adb_push("onnxruntime_customopregistration_test", device_dir, cwd=cwd) + adb_shell(f"chmod +x {device_dir}/onnxruntime_shared_lib_test") + adb_shell(f"chmod +x {device_dir}/onnxruntime_customopregistration_test") + run_adb_shell(f"LD_LIBRARY_PATH=$LD_LIBRARY_PATH:{device_dir} {device_dir}/onnxruntime_shared_lib_test") + run_adb_shell( + f"LD_LIBRARY_PATH=$LD_LIBRARY_PATH:{device_dir} {device_dir}/onnxruntime_customopregistration_test" ) - adb_push("onnxruntime_test_all", device_dir, cwd=cwd) - adb_shell(f"chmod +x {device_dir}/onnxruntime_test_all") - adb_push("onnx_test_runner", device_dir, cwd=cwd) - adb_shell(f"chmod +x {device_dir}/onnx_test_runner") - run_adb_shell(f"{device_dir}/onnxruntime_test_all") - - # remove onnxruntime_test_all as it takes up a _lot_ of space and can cause insufficient storage errors - # when we try to copy the java app to the device. - adb_shell(f"rm {device_dir}/onnxruntime_test_all") - - if args.build_java: - # use the gradle wrapper under /java - gradle_executable = os.path.join(source_dir, "java", "gradlew.bat" if is_windows() else "gradlew") - android_test_path = os.path.join(cwd, "java", "androidtest", "android") - run_subprocess( - [ - gradle_executable, - "--no-daemon", - f"-DminSdkVer={args.android_api}", - "clean", - "connectedDebugAndroidTest", - ], - cwd=android_test_path, - ) - if args.use_nnapi: - run_adb_shell(f"{device_dir}/onnx_test_runner -e nnapi {device_dir}/test") - else: - run_adb_shell(f"{device_dir}/onnx_test_runner {device_dir}/test") - # run shared_lib_test if necessary - if args.build_shared_lib: - adb_push("libonnxruntime.so", device_dir, cwd=cwd) - adb_push("onnxruntime_shared_lib_test", device_dir, cwd=cwd) - adb_push("libcustom_op_library.so", device_dir, cwd=cwd) - adb_push("libcustom_op_get_const_input_test_library.so", device_dir, cwd=cwd) - adb_push("onnxruntime_customopregistration_test", device_dir, cwd=cwd) - adb_shell(f"chmod +x {device_dir}/onnxruntime_shared_lib_test") - adb_shell(f"chmod +x {device_dir}/onnxruntime_customopregistration_test") - run_adb_shell(f"LD_LIBRARY_PATH=$LD_LIBRARY_PATH:{device_dir} {device_dir}/onnxruntime_shared_lib_test") - run_adb_shell( - f"LD_LIBRARY_PATH=$LD_LIBRARY_PATH:{device_dir} {device_dir}/onnxruntime_customopregistration_test" - ) +def run_ios_tests(args, source_dir, config, cwd): + is_targeting_iphone_simulator = "iphonesimulator" in args.apple_sysroot.lower() + if not is_targeting_iphone_simulator: + log.info( + f"Could not detect iphonesimulator target from --apple_sysroot ({args.apple_sysroot}), " + "skipping running of iOS tests on simulator." + ) + return + host_arch = platform.machine() + if host_arch != args.osx_arch: + log.info( + f"Host arch ({host_arch}) and --osx_arch ({args.osx_arch}) mismatch, " + "skipping running of iOS tests on simulator." + ) + return -def run_ios_tests(args, source_dir, config, cwd): simulator_device_info = subprocess.check_output( [ sys.executable,