Skip to content

Commit

Permalink
Bump protobuf min version and relax version specifier (facebookincuba…
Browse files Browse the repository at this point in the history
…tor#10134)

Summary:
Fixes: facebookincubator#3136 and improves issues with the EXACT version called out in facebookincubator#10113

Pull Request resolved: facebookincubator#10134

Reviewed By: kgpai

Differential Revision: D58825444

Pulled By: bikramSingh91

fbshipit-source-id: be81ba00b5fdce54419216edbf9c35d8d36edaaf
  • Loading branch information
assignUser authored and facebook-github-bot committed Jun 21, 2024
1 parent 0fe2e62 commit 2259b9e
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 10 deletions.
1 change: 1 addition & 0 deletions .github/workflows/linux-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ jobs:
env:
CCACHE_DIR: "/__w/velox/velox/.ccache"
VELOX_DEPENDENCY_SOURCE: SYSTEM
Protobuf_SOURCE: BUNDLED # can be removed after #10134 is merged
simdjson_SOURCE: BUNDLED
xsimd_SOURCE: BUNDLED
CUDA_VERSION: "12.4"
Expand Down
17 changes: 12 additions & 5 deletions .github/workflows/macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,18 @@ jobs:
run: |
brew install \
bison boost ccache double-conversion flex fmt gflags glog \
icu4c libevent libsodium lz4 lzo ninja openssl range-v3 simdjson \
snappy thrift xz xsimd zstd
icu4c libevent libsodium lz4 lzo ninja openssl protobuf@21 \
range-v3 simdjson snappy thrift xz xsimd zstd
echo "NJOBS=`sysctl -n hw.ncpu`" >> $GITHUB_ENV
brew unlink protobuf || echo "protobuf not installed"
brew link --force protobuf@21
- name: Cache ccache
uses: actions/cache@v4
uses: assignUser/stash/restore@v1
with:
path: '${{ env.CCACHE_DIR }}'
key: ccache-macos-${{ matrix.os }}-${{ hashFiles('velox/*') }}
restore-keys: ccache-macos-${{ matrix.os }}
key: ccache-macos-${{ matrix.os }}

- name: Configure Build
env:
Expand All @@ -92,6 +93,12 @@ jobs:
run: |
cmake --build _build/debug -j $NJOBS
ccache -s
- uses: assignUser/stash/save@v1
with:
path: '${{ env.CCACHE_DIR }}'
key: ccache-macos-${{ matrix.os }}

- name: Run Tests
if: false
run: ctest -j $NJOBS --test-dir _build/debug --output-on-failure
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ by Velox. See details on bundling below.
| xz | default | No |
| zstd | default | No |
| openssl | default | No |
| protobuf | 21 (exact) | Yes |
| protobuf | 21.7 >= x < 22 | Yes |
| boost | 1.77.0 | Yes |
| flex | 2.5.13 | No |
| bison | 3.0.4 | No |
Expand Down
4 changes: 2 additions & 2 deletions CMake/resolve_dependency_modules/protobuf.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
# limitations under the License.
include_guard(GLOBAL)

set(VELOX_PROTOBUF_BUILD_VERSION 21.4)
set(VELOX_PROTOBUF_BUILD_VERSION 21.8)
set(VELOX_PROTOBUF_BUILD_SHA256_CHECKSUM
6c5e1b0788afba4569aeebb2cfe205cb154aa01deacaba0cd26442f3b761a836)
83ad4faf95ff9cbece7cb9c56eb3ca9e42c3497b77001840ab616982c6269fb6)
string(
CONCAT
VELOX_PROTOBUF_SOURCE_URL
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ if(${VELOX_BUILD_MINIMAL_WITH_DWIO}

# Locate or build protobuf.
set_source(Protobuf)
resolve_dependency(Protobuf 3.21.4 EXACT)
resolve_dependency(Protobuf 3.21.7...<4)
include_directories(${Protobuf_INCLUDE_DIRS})
endif()

Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-centos8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function install_fmt {
}

function install_protobuf {
wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf
wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.8/protobuf-all-21.8.tar.gz protobuf
(
cd protobuf
./configure --prefix=/usr
Expand Down

0 comments on commit 2259b9e

Please sign in to comment.