Skip to content

Commit 1f4d5ff

Browse files
kgpaimeta-codesync[bot]
authored andcommitted
fix(build): Harden the merge to main builds (#16424)
Summary: The merge to main job with resolve_dependencies fails occasionaly since it tries to get all dependencies from the internet . As we are trying to make the builds more robust and green - this change sets the job to use SYSTEM dependency. We will still test the resolve dependency module out but run it via our scheduled jobs. Pull Request resolved: #16424 Reviewed By: pratikpugalia Differential Revision: D93500381 Pulled By: kgpai fbshipit-source-id: 8ac5ccba068fef01f8455dacd925a2993c481a0d
1 parent 63f50f4 commit 1f4d5ff

File tree

6 files changed

+171
-21
lines changed

6 files changed

+171
-21
lines changed

.github/workflows/linux-build-base.yml

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,10 @@ jobs:
223223
224224
ubuntu-debug:
225225
runs-on: 16-core-ubuntu
226+
container: ghcr.io/facebookincubator/velox-dev:ubuntu-22.04
226227
# prevent errors when forks ff their main branch
227228
if: ${{ github.repository == 'facebookincubator/velox' }}
228-
name: Ubuntu debug with resolve_dependency
229+
name: Ubuntu debug with system dependencies
229230
env:
230231
CCACHE_DIR: ${{ github.workspace }}/ccache
231232
USE_CLANG: ${{ inputs.use-clang && 'true' || 'false' }}
@@ -234,6 +235,34 @@ jobs:
234235
shell: bash
235236
working-directory: velox
236237
steps:
238+
- uses: actions/checkout@v5
239+
with:
240+
fetch-depth: 2
241+
path: velox
242+
persist-credentials: false
243+
244+
- name: Fix git permissions
245+
# Usually actions/checkout does this but as we run in a container
246+
# it doesn't work
247+
run: git config --global --add safe.directory ${GITHUB_WORKSPACE}
248+
249+
- name: Install Dependencies
250+
env:
251+
VELOX_BUILD_SHARED: "OFF"
252+
VELOX_ARROW_CMAKE_PATCH: ${{ github.workspace }}/velox/CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
253+
run: |
254+
if git diff --name-only HEAD^1 HEAD | grep -q "scripts/setup-"; then
255+
# Overwrite old setup scripts with changed versions
256+
cp scripts/setup-* /
257+
258+
mkdir /tmp/build
259+
cd /tmp/build
260+
261+
USE_CLANG=false bash /setup-ubuntu.sh
262+
263+
cd /
264+
rm -rf /tmp/build # cleanup to avoid issues with disk space
265+
fi
237266
238267
- name: Get Ccache Stash
239268
uses: apache/infrastructure-actions/stash/restore@3354c1565d4b0e335b78a76aedd82153a9e144d4
@@ -246,24 +275,23 @@ jobs:
246275
run: |
247276
mkdir -p "$CCACHE_DIR"
248277
249-
- uses: actions/checkout@v5
250-
with:
251-
path: velox
252-
persist-credentials: false
253-
254-
- name: Install Dependencies
255-
run: |
256-
source scripts/setup-ubuntu.sh && install_apt_deps && install_faiss_deps
257-
258278
- name: Clear CCache Statistics
259279
run: |
260280
ccache -sz
261281
262282
- name: Make Debug Build
263283
env:
264-
VELOX_DEPENDENCY_SOURCE: BUNDLED
284+
VELOX_DEPENDENCY_SOURCE: SYSTEM
265285
ICU_SOURCE: SYSTEM
266-
MAKEFLAGS: NUM_THREADS=16 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=2
286+
# Use BUNDLED gflags to provide PIC static gflags for .so plugins.
287+
# The container's folly is built with -DGFLAGS_SHARED=FALSE so its
288+
# exported config references gflags_static which BUNDLED gflags provides.
289+
gflags_SOURCE: BUNDLED
290+
# Keep system glog (container's glog is built against the same gflags
291+
# version). Without this, BUNDLED gflags cascades to BUNDLED glog
292+
# which conflicts with system glog loaded transitively.
293+
glog_SOURCE: SYSTEM
294+
MAKEFLAGS: NUM_THREADS=16 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=3
267295
run: |
268296
EXTRA_CMAKE_FLAGS=(
269297
"-DCMAKE_LINK_LIBRARIES_STRATEGY=REORDER_FREELY"
@@ -273,15 +301,11 @@ jobs:
273301
"-DVELOX_ENABLE_GEO=ON"
274302
"-DVELOX_ENABLE_PARQUET=ON"
275303
"-DVELOX_MONO_LIBRARY=ON"
276-
"-DVELOX_BUILD_SHARED=ON"
277304
)
278305
if [[ "${USE_CLANG}" = "true" ]]; then
279306
export CC=/usr/bin/clang-15; export CXX=/usr/bin/clang++-15;
280307
else
281-
# Faiss (link issue when using Clang) is excluded for Clang compilation
282-
# and need to be added back when using GCC.
283308
EXTRA_CMAKE_FLAGS+=("-DVELOX_ENABLE_FAISS=ON")
284-
# Investigate issues with remote function service: Issue #13897
285309
EXTRA_CMAKE_FLAGS+=("-DVELOX_ENABLE_REMOTE_FUNCTIONS=ON")
286310
fi
287311
make debug
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# Copyright (c) Facebook, Inc. and its affiliates.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
# Tests the BUNDLED dependency resolution path on a plain Ubuntu runner
16+
# (no container). Triggered on PRs that change dependency scripts or
17+
# CMake modules, and on a nightly schedule.
18+
19+
name: Ubuntu Bundled Dependencies
20+
21+
on:
22+
schedule:
23+
- cron: 0 5 * * *
24+
workflow_dispatch: {}
25+
pull_request:
26+
paths:
27+
- scripts/setup-*.sh
28+
- scripts/setup-common.sh
29+
- scripts/setup-versions.sh
30+
- scripts/setup-helper-functions.sh
31+
- CMake/**
32+
- CMakeLists.txt
33+
- .github/workflows/ubuntu-bundled-deps.yml
34+
35+
permissions:
36+
contents: read
37+
38+
concurrency:
39+
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.sha }}
40+
cancel-in-progress: true
41+
42+
jobs:
43+
ubuntu-debug-bundled-deps:
44+
runs-on: 16-core-ubuntu
45+
if: ${{ github.repository == 'facebookincubator/velox' }}
46+
name: Ubuntu debug with bundled dependencies
47+
env:
48+
CCACHE_DIR: ${{ github.workspace }}/ccache
49+
defaults:
50+
run:
51+
shell: bash
52+
working-directory: velox
53+
steps:
54+
55+
- name: Get Ccache Stash
56+
uses: apache/infrastructure-actions/stash/restore@3354c1565d4b0e335b78a76aedd82153a9e144d4
57+
with:
58+
path: ${{ env.CCACHE_DIR }}
59+
key: ccache-ubuntu-debug-bundled-deps
60+
61+
- name: Ensure Stash Dirs Exists
62+
working-directory: ${{ github.workspace }}
63+
run: |
64+
mkdir -p "$CCACHE_DIR"
65+
66+
- uses: actions/checkout@v5
67+
with:
68+
path: velox
69+
persist-credentials: false
70+
71+
- name: Install Dependencies
72+
run: |
73+
source scripts/setup-ubuntu.sh && install_apt_deps && install_faiss_deps
74+
75+
- name: Clear CCache Statistics
76+
run: |
77+
ccache -sz
78+
79+
- name: Make Debug Build
80+
env:
81+
VELOX_DEPENDENCY_SOURCE: BUNDLED
82+
ICU_SOURCE: SYSTEM
83+
MAKEFLAGS: NUM_THREADS=16 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=2
84+
run: |
85+
EXTRA_CMAKE_FLAGS=(
86+
"-DCMAKE_LINK_LIBRARIES_STRATEGY=REORDER_FREELY"
87+
"-DVELOX_ENABLE_BENCHMARKS=ON"
88+
"-DVELOX_ENABLE_EXAMPLES=ON"
89+
"-DVELOX_ENABLE_ARROW=ON"
90+
"-DVELOX_ENABLE_GEO=ON"
91+
"-DVELOX_ENABLE_PARQUET=ON"
92+
"-DVELOX_MONO_LIBRARY=ON"
93+
"-DVELOX_BUILD_SHARED=ON"
94+
"-DVELOX_ENABLE_FAISS=ON"
95+
"-DVELOX_ENABLE_REMOTE_FUNCTIONS=ON"
96+
)
97+
make debug
98+
99+
- name: CCache after
100+
run: |
101+
ccache -vs
102+
103+
- uses: apache/infrastructure-actions/stash/save@3354c1565d4b0e335b78a76aedd82153a9e144d4
104+
with:
105+
path: ${{ env.CCACHE_DIR }}
106+
key: ccache-ubuntu-debug-bundled-deps
107+
108+
- name: Run Tests
109+
run: |
110+
cd _build/debug && ctest -j 8 --output-on-failure --no-tests=error

CMake/Findc-ares.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ if(c-ares_FOUND)
2020
endif()
2121

2222
find_path(C_ARES_INCLUDE_DIR NAMES ares.h PATH_SUFFIXES c-ares)
23-
find_library(C_ARES_LIBRARY NAMES c-ares)
23+
find_library(C_ARES_LIBRARY NAMES c-ares cares)
2424

2525
include(FindPackageHandleStandardArgs)
2626
find_package_handle_standard_args(c-ares DEFAULT_MSG C_ARES_LIBRARY C_ARES_INCLUDE_DIR)

CMakeLists.txt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -574,9 +574,13 @@ if(NOT TARGET gflags::gflags)
574574
endif()
575575

576576
if(${gflags_SOURCE} STREQUAL "BUNDLED")
577-
# we force glog from source to avoid issues with a system version built
578-
# against another gflags version (which is likely)
579-
set(glog_SOURCE BUNDLED)
577+
# Allow explicit glog_SOURCE override (e.g. when system glog is built
578+
# against the same gflags version as BUNDLED).
579+
if(DEFINED ENV{glog_SOURCE})
580+
set(glog_SOURCE $ENV{glog_SOURCE})
581+
else()
582+
set(glog_SOURCE BUNDLED)
583+
endif()
580584
else()
581585
set(glog_SOURCE SYSTEM)
582586
endif()

scripts/docker/centos-multi.dockerfile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ ENV UV_TOOL_BIN_DIR=/usr/local/bin \
4747
ENV CMAKE_POLICY_VERSION_MINIMUM="3.5" \
4848
VELOX_ARROW_CMAKE_PATCH=/cmake-compatibility.patch
4949

50+
# Ensure libraries installed to INSTALL_PREFIX are found at runtime (e.g.
51+
# thrift1 needs libgflags.so.2.2 when folly links gflags statically but
52+
# other tools still use shared gflags).
53+
ENV LD_LIBRARY_PATH="${INSTALL_PREFIX}/lib:${INSTALL_PREFIX}/lib64"
54+
5055
# Some CMake configs contain the hard coded prefix '/deps', we need to replace that with
5156
# the future location to avoid build errors in the base-image
5257
RUN bash /setup-centos9.sh && \

scripts/setup-common.sh

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,14 @@ function install_fmt {
4747

4848
function install_folly {
4949
wget_and_untar https://github.com/facebook/folly/archive/refs/tags/"${FB_OS_VERSION}".tar.gz folly
50-
cmake_install_dir folly -DBUILD_SHARED_LIBS="$VELOX_BUILD_SHARED" -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
50+
local FOLLY_FLAGS=(-DBUILD_SHARED_LIBS="$VELOX_BUILD_SHARED" -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON)
51+
# When folly is static, use static gflags to avoid dual gflags flag
52+
# registration when .so plugins are dlopen'd (both the binary and plugin
53+
# would register the same flags in a shared gflags registry).
54+
if [[ ${VELOX_BUILD_SHARED} != "ON" ]]; then
55+
FOLLY_FLAGS+=(-DGFLAGS_SHARED=FALSE)
56+
fi
57+
cmake_install_dir folly "${FOLLY_FLAGS[@]}"
5158
}
5259

5360
function install_fizz {

0 commit comments

Comments
 (0)