github-actions[bot] commented on code in PR #65796: URL: https://github.com/apache/doris/pull/65796#discussion_r3620690437
########## .github/workflows/be-ut-mac.yml: ########## @@ -19,6 +19,7 @@ name: BE UT (macOS) on: pull_request: + types: [opened, synchronize] Review Comment: [P1] Revalidate the current merge result when reopening Required checks are attached to the head SHA, but a `pull_request` workflow checks out `refs/pull/<n>/merge`, as [GitHub documents](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request). If head `H` passed against base `B1`, the PR is closed, the base advances to `B2`, and the same head is reopened, the merge ref is now `B2+H` while the old green check still belongs to `H`. Dropping the default `reopened` event skips validation of that current merge result. Please retain `reopened`, or only skip after matching a successful check to the exact current merge SHA. ########## be/test/runtime/runtime_profile_test.cpp: ########## @@ -441,7 +441,7 @@ TEST(RuntimeProfileTest, DerivedCounters) { RuntimeProfile::Counter* bytes_counter = profile.add_counter("bytes", TUnit::BYTES); RuntimeProfile::Counter* ticks_counter = profile.add_counter("ticks", TUnit::TIME_NS); // set to 1 sec - ticks_counter->set(1000L * 1000L * 1000L); + ticks_counter->set(int64_t(1000L * 1000L * 1000L)); Review Comment: [P1] Finish removing the libc++ `<ranges>` conflict This translation unit still includes `common/object_pool.h`, which itself includes `<ranges>` and uses `std::ranges::reverse_view`. Because `doris_be_test` applies `-fno-access-control` target-wide, this still reaches the same libc++ access-redeclaration failure that motivated the direct `<ranges>` removals in this PR. The production-only workflow does not compile this path. Please remove the transitive conflict (for example, use reverse iterators in `ObjectPool` or scope the test flag more narrowly), then compile, link, and run `doris_be_test` on Darwin. ########## .github/workflows/be-ut-mac.yml: ########## @@ -80,21 +81,50 @@ jobs: 'openjdk@11' 'maven' 'node' - 'llvm@16' + 'llvm@20' + 'libomp' ) brew install "${cellars[@]}" || true + # macos-15 runners are Apple Silicon (arm64), so download the arm64 + # prebuilt thirdparty. Using the x86_64 archive makes the linker ignore + # every thirdparty static library ("found architecture 'x86_64', + # required architecture 'arm64'") and the final BE link fails with + # thousands of undefined symbols. pushd thirdparty branch="${{ github.base_ref }}" if [[ -z "${branch}" ]] || [[ "${branch}" == 'master' ]]; then - curl -L https://github.com/apache/doris-thirdparty/releases/download/automation/doris-thirdparty-prebuilt-darwin-x86_64.tar.xz \ - -o doris-thirdparty-prebuilt-darwin-x86_64.tar.xz + curl -L https://github.com/apache/doris-thirdparty/releases/download/automation/doris-thirdparty-prebuilt-darwin-arm64.tar.xz \ + -o doris-thirdparty-prebuilt-darwin-arm64.tar.xz else - curl -L "https://github.com/apache/doris-thirdparty/releases/download/automation-${branch/branch-/}/doris-thirdparty-prebuilt-darwin-x86_64.tar.xz"; \ - -o doris-thirdparty-prebuilt-darwin-x86_64.tar.xz + curl -L "https://github.com/apache/doris-thirdparty/releases/download/automation-${branch/branch-/}/doris-thirdparty-prebuilt-darwin-arm64.tar.xz"; \ + -o doris-thirdparty-prebuilt-darwin-arm64.tar.xz fi - tar -xvf doris-thirdparty-prebuilt-darwin-x86_64.tar.xz + tar -xvf doris-thirdparty-prebuilt-darwin-arm64.tar.xz popd - export JAVA_HOME="${JAVA_HOME_17_X64%\/}" - ./run-be-ut.sh --run -j "$(nproc)" --clean + # macos-15 runners are Apple Silicon (arm64), so the JDK env var is + # JAVA_HOME_17_arm64. Fall back to the x64 variable for Intel runners. + JAVA_HOME="${JAVA_HOME_17_arm64:-${JAVA_HOME_17_X64}}" + export JAVA_HOME="${JAVA_HOME%\/}" + + # AppleClang ships no OpenMP runtime, so CMake's find_package(OpenMP) + # (e.g. in contrib/openblas) fails with "Could NOT find OpenMP_C" and + # "'omp.h' file not found". Point the compiler/linker at Homebrew's + # libomp. be/CMakeLists.txt resets CMAKE_C_FLAGS/CMAKE_CXX_FLAGS, so + # EXTRA_CXX_FLAGS is also needed to pass the include path to the BE build. + LIBOMP_PREFIX="$(brew --prefix libomp)" + export CPPFLAGS="-I${LIBOMP_PREFIX}/include ${CPPFLAGS:-}" + export CFLAGS="-I${LIBOMP_PREFIX}/include ${CFLAGS:-}" + export CXXFLAGS="-I${LIBOMP_PREFIX}/include ${CXXFLAGS:-}" + export LDFLAGS="-L${LIBOMP_PREFIX}/lib ${LDFLAGS:-}" + export EXTRA_CXX_FLAGS="-I${LIBOMP_PREFIX}/include ${EXTRA_CXX_FLAGS:-}" + + # Only verify that the BE compiles on macOS; do NOT compile be/test. + # The unit tests are built and run locally, not in this job. build.sh + # configures the BE with -DMAKE_TEST=OFF, so nothing under be/test is + # compiled. Skip the Java extensions and cdc client (not relevant to a + # macOS C++ build check), and pass -j explicitly because build.sh + # otherwise defaults to only ~nproc/4 jobs. + DISABLE_BE_JAVA_EXTENSIONS=ON DISABLE_BE_CDC_CLIENT=ON \ + ./build.sh --be -j "$(nproc)" Review Comment: [P1] Keep compiling and running the macOS unit tests This replaces `run-be-ut.sh --run` with `build.sh --be`, but the latter hard-codes `MAKE_TEST=OFF` in `build.sh`. As a result, this check no longer builds `doris_be_test` or executes any BE unit test; the successful macOS run confirms `make test: OFF`. That also means none of this PR's Darwin-specific fixes under `be/test/**` are validated here. Please keep the arm64/JDK/OpenMP setup while restoring `run-be-ut.sh --run`, or otherwise configure `MAKE_TEST=ON` and execute the test binary. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
