Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install Protobuf using FetchContent. #4056

Merged
merged 5 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions .github/workflows/ci-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,9 @@ jobs:
echo "/usr/local/opt/bison/bin" >> $GITHUB_PATH
- name: Build (MacOS)
# for some reason, brew does not install protobuf in a way cmake would be
# able to find it. Also, the library needs to be specified manually in
# order for the compiler to be correctly linked with protobuf.
# Also, there are no static protobuf libs in homebrew.
# See also https://github.com/Homebrew/homebrew-core/pull/122277
run: |
PROTOBUF=`brew --prefix --installed protobuf@21`
./bootstrap.sh -DENABLE_GC=ON -DCMAKE_BUILD_TYPE=RELEASE \
-DCMAKE_UNITY_BUILD=ON \
-DProtobuf_INCLUDE_DIR=$PROTOBUF/include \
-DProtobuf_LIBRARY=$PROTOBUF/lib/libprotobuf.dylib \
-DENABLE_PROTOBUF_STATIC=OFF
-DCMAKE_UNITY_BUILD=ON
make -Cbuild -j2
- name: Run tests (MacOS)
Expand Down
19 changes: 9 additions & 10 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ endif()
if (CMAKE_UNITY_BUILD)
set(CMAKE_UNITY_BUILD_BATCH_SIZE 10 CACHE UNINITIALIZED "Set the unity build batch size.")
endif ()

# Always build position-independent code. This is important when linking with Protobuf.
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

# set the required options for a static release build
if (BUILD_STATIC_RELEASE)
message(STATUS "Building static release binaries")
Expand All @@ -128,19 +132,14 @@ if (BUILD_STATIC_RELEASE)
add_definitions(-DP4C_STATIC_BUILD)
endif ()

# required tools and libraries
# Required tools and libraries.
find_package (PythonInterp 3 REQUIRED)
find_package (FLEX 2.0 REQUIRED)
find_package (BISON 3.0 REQUIRED)
if (ENABLE_PROTOBUF_STATIC)
set(SAVED_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES})
set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
endif ()
find_package (Protobuf 3.0.0 REQUIRED)
if (ENABLE_PROTOBUF_STATIC)
set(CMAKE_FIND_LIBRARY_SUFFIXES ${SAVED_CMAKE_FIND_LIBRARY_SUFFIXES})
endif ()
# The boost graph headers are optional and only required by the graphs backend
include(Protobuf)
p4c_obtain_protobuf()

# The boost graph headers are optional and only required by the graphs back end.
find_package (Boost QUIET COMPONENTS graph)
if (Boost_FOUND)
set (HAVE_LIBBOOST_GRAPH 1)
Expand Down
26 changes: 11 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,9 @@ sudo dpkg -i /path/to/package.deb
library. Default is ON.
- `-DENABLE_GTESTS=ON|OFF`. Enable building and running GTest unit tests.
Default is ON.
- `-DENABLE_PROTOBUF_STATIC=ON|OFF`. Enable the use of static
protobuf libraries. Default is ON.
- `-DP4C_USE_PREINSTALLED_PROTOBUF=ON|OFF`. Try to find a system version of Protobuf instead of a CMake version.
- `-DENABLE_PROTOBUF_STATIC=ON|OFF`. Enable the use of static protobuf libraries. Default is ON.
Only has an effect when `P4C_USE_PREINSTALLED_PROTOBUF` is enabled.
- `-DENABLE_MULTITHREAD=ON|OFF`. Use multithreading. Default is
OFF.
- `-DBUILD_LINK_WITH_GOLD=ON|OFF`. Use Gold linker for build if available.
Expand Down Expand Up @@ -271,18 +272,16 @@ For documentation building:

`p4c` also depends on Google Protocol Buffers (Protobuf). `p4c` requires version
3.0 or higher, so the packaged version provided in Ubuntu 20.04 **should**
work. However, all our CI testing is done with a more recent version of Protobuf
(at the moment, 3.18.1), which we install from source. If you are experiencing
issues with the Protobuf version shipped with your OS distribution, we recommend
that we install Protobuf 3.18.1 from source. You can find instructions
[here](https://github.com/protocolbuffers/protobuf/blob/v3.18.1/src/README.md).
After cloning Protobuf and before you build, check-out version 3.18.1:
work. However, P4C typically installs its own version of Protobuf using CMake's Fetchcontent module
(at the moment, 3.21.12). If you are experiencing issues with the Protobuf version shipped with your OS distribution, we recommend that to install Protobuf 3.21.12 from source. You can find instructions
[here](https://github.com/protocolbuffers/protobuf/blob/v3.21.12/src/README.md).
After cloning Protobuf and before you build, check-out version 3.21.12:

`git checkout v3.18.1`
`git checkout v3.21.12`

Please note that while all Protobuf versions newer than 3.0 should work for
`p4c` itself, you may run into trouble with some extensions and other p4lang
projects unless you install version 3.18.1.
projects unless you install version 3.21.12.

### CMake
p4c requires a CMake version of at least 3.16.3 or higher. On older systems, a newer version of CMake can be installed using `pip3 install --user cmake==3.16.3`. We have a CI test on Ubuntu 18.04 that uses this option, but there is no guarantee that this will lead to a successful build.
Expand All @@ -292,7 +291,7 @@ p4c requires a CMake version of at least 3.16.3 or higher. On older systems, a n
```bash
sudo dnf install -y cmake g++ git automake libtool gc-devel bison flex \
libfl-devel gmp-devel boost-devel boost-iostreams boost-graph llvm pkg-config \
python3 python3-pip tcpdump protobuf-devel protobuf-static
python3 python3-pip tcpdump

sudo pip3 install -r requirements.txt
```
Expand Down Expand Up @@ -343,10 +342,7 @@ Installing on macOS:
```
Homebrew offers a `protobuf` formula. It installs version 3.2, which should
work for p4c itself but may cause problems with some extensions. It's
preferable to install Protocol Buffers 3.0 from source using the instructions
[here](https://github.com/google/protobuf/blob/master/src/README.md). Check
out the newest tag in the 3.0 series (`v3.0.2` as of this writing) before you
build.
preferable to use the version of Protobuf which is supplied with CMake's fetchcontent (3.21.12).

The `protobuf` formula requires the following CMake variables to be set,
otherwise CMake does not find the libraries or fails in linking. It is likely
Expand Down
2 changes: 1 addition & 1 deletion backends/bmv2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ set (BMV2_BACKEND_COMMON_HDRS

set (IR_DEF_FILES ${IR_DEF_FILES} ${CMAKE_CURRENT_SOURCE_DIR}/bmv2.def PARENT_SCOPE)

add_library(bmv2backend ${P4C_STATIC_BUILD} ${BMV2_BACKEND_COMMON_SRCS})
add_library(bmv2backend STATIC ${BMV2_BACKEND_COMMON_SRCS})
add_dependencies(bmv2backend ir-generated frontend)

add_executable(p4c-bm2-ss ${BMV2_SIMPLE_SWITCH_SRCS})
Expand Down
3 changes: 3 additions & 0 deletions backends/dpdk/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ add_custom_command(
)

add_library(dpdk_runtime STATIC ${DPDK_P4RUNTIME_INFO_GEN_SRCS})
target_include_directories(dpdk_runtime
SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
)

set(P4C_DPDK_SOURCES
../bmv2/common/lower.cpp
Expand Down
10 changes: 4 additions & 6 deletions backends/p4tools/modules/testgen/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,15 @@ FetchContent_Declare(
GIT_TAG 3741c73ba78babd2ed88f2acf2fcd6dafdb878e8
GIT_PROGRESS TRUE
)
FetchContent_MakeAvailable(inja)
fetchcontent_makeavailable_but_exclude_install(inja)

# Testgen libraries.
# Testgen libraries and includes. Defined here such that targets can extend them.
set(
TESTGEN_LIBS
PRIVATE p4tools-common
PUBLIC inja
)
set(TESTGEN_INCLUDES)

file(GLOB testgen_targets RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}/targets ${CMAKE_CURRENT_SOURCE_DIR}/targets/*)
foreach(ext ${testgen_targets})
Expand All @@ -104,17 +105,14 @@ endforeach(ext)
# Propagate def files set by target extensions upwards.
set(IR_DEF_FILES ${IR_DEF_FILES} PARENT_SCOPE)

# Convert the list of files into #includes
foreach(include_file ${include_files})
endforeach()

# Fill the template
configure_file(register.h.in register.h)

# The library.
add_p4tools_library(testgen ${TESTGEN_SOURCES})
# Make sure the testgen library and anything that links to it see both the Z3 and Inja includes.
target_link_libraries(testgen PUBLIC p4tools-common inja)
target_include_directories(testgen ${TESTGEN_INCLUDES})

# The executable.
add_p4tools_executable(p4testgen main.cpp)
Expand Down
6 changes: 6 additions & 0 deletions backends/p4tools/modules/testgen/targets/bmv2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,9 @@ set(
TESTGEN_LIBS ${TESTGEN_LIBS}
PARENT_SCOPE
)

set(
TESTGEN_INCLUDES ${TESTGEN_INCLUDES}
SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
PARENT_SCOPE
)
9 changes: 9 additions & 0 deletions cmake/P4CUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -405,3 +405,12 @@ macro(p4c_add_xfail_reason tag reason)
endif()
endforeach()
endmacro(p4c_add_xfail_reason)

# Effectively emulates fetchcontent_makeavailable but does not add the module to install.
macro(fetchcontent_makeavailable_but_exclude_install content)
FetchContent_GetProperties(${content})
if(NOT ${content}_POPULATED)
FetchContent_Populate(${content})
add_subdirectory(${${content}_SOURCE_DIR} ${${content}_BINARY_DIR} EXCLUDE_FROM_ALL)
endif()
endmacro(fetchcontent_makeavailable_but_exclude_install)
76 changes: 76 additions & 0 deletions cmake/Protobuf.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
macro(p4c_obtain_protobuf)
option(
P4C_USE_PREINSTALLED_PROTOBUF
"Look for a preinstalled version of Protobuf in the system instead of installing a prebuilt binary using FetchContent."
OFF
)

# If P4C_USE_PREINSTALLED_PROTOBUF is ON just try to find a preinstalled version of Protobuf.
if(P4C_USE_PREINSTALLED_PROTOBUF)
if(ENABLE_PROTOBUF_STATIC)
set(SAVED_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES})
set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
endif()
find_package(Protobuf 3.0.0 REQUIRED)
if(ENABLE_PROTOBUF_STATIC)
set(CMAKE_FIND_LIBRARY_SUFFIXES ${SAVED_CMAKE_FIND_LIBRARY_SUFFIXES})
endif()
else()
# Google introduced another breaking change with protobuf 22.x by adding abseil as a new
# dependency. https://protobuf.dev/news/2022-08-03/#abseil-dep We do not want abseil, so we stay
# with 21.x for now.
set(P4C_PROTOBUF_VERSION "21.12")
message("Fetching Protobuf version ${P4C_PROTOBUF_VERSION} for P4C...")

# Print out download state while setting up Protobuf.
set(FETCHCONTENT_QUIET_PREV ${FETCHCONTENT_QUIET})
set(FETCHCONTENT_QUIET OFF)
set(protobuf_BUILD_TESTS OFF CACHE BOOL "Build tests.")
set(protobuf_BUILD_PROTOC_BINARIES OFF CACHE BOOL "Build libprotoc and protoc compiler.")
# Only ever build the static library. It is not safe to link with a local dynamic version.
set(protobuf_BUILD_SHARED_LIBS OFF CACHE BOOL "Build Shared Libraries")
# Unity builds do not work for Protobuf...
set(SAVED_CMAKE_UNITY_BUILD ${CMAKE_UNITY_BUILD})
set(CMAKE_UNITY_BUILD OFF)
fetchcontent_declare(
protobuf
URL https://github.com/protocolbuffers/protobuf/releases/download/v${P4C_PROTOBUF_VERSION}/protobuf-cpp-3.${P4C_PROTOBUF_VERSION}.tar.gz
URL_HASH SHA256=4eab9b524aa5913c6fffb20b2a8abf5ef7f95a80bc0701f3a6dbb4c607f73460
USES_TERMINAL_DOWNLOAD TRUE
GIT_PROGRESS TRUE
)
# Pull a different protoc binary for MacOS.
# TODO: Should we build from scratch?
if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
fetchcontent_declare(
protoc
URL https://github.com/protocolbuffers/protobuf/releases/download/v${P4C_PROTOBUF_VERSION}/protoc-${P4C_PROTOBUF_VERSION}-osx-x86_64.zip
USES_TERMINAL_DOWNLOAD TRUE
GIT_PROGRESS TRUE
)
else()
fetchcontent_declare(
protoc
URL https://github.com/protocolbuffers/protobuf/releases/download/v${P4C_PROTOBUF_VERSION}/protoc-${P4C_PROTOBUF_VERSION}-linux-x86_64.zip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fruffy This seems to break the build for architectures other than x86_64 (e.g., aarch64).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Afaik we do not support or make guarantees for any architecture other than x86_64. But if there is a need this should be easily patchable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p4c is also used on ARM64 platforms like P4EDGE and P4Pi. It would be great if we could also enable support for the aarch64 architecture.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the option P4C_USE_PREINSTALLED_PROTOBUF=ON will use the system's Protobuf version. In that case things should compile as usual until we have a patch for these types of architectures in place.

Copy link
Member

@rst0git rst0git Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Fabian!

Some platforms are not connected to the internet and FetchContent would fail when building p4c. As a workaround, we can run cmake on a system with internet access and include build/_deps with the source code.

Do you have any advice on how to enable building p4c offline?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think that building without internet is a supported usecase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are ways to run FetchContent fully disconnected https://cmake.org/cmake/help/latest/module/FetchContent.html#command:fetchcontent_populate
At least I have been in scenarios where I did not have internet and was able to build as usual.

Now, populating build/deps in the beginning is a different story. But that is no different that running git submodule init --update. I do not have a good suggestion there yet other than using the PREINSTALLED options.

URL_HASH SHA256=3a4c1e5f2516c639d3079b1586e703fc7bcfa2136d58bda24d1d54f949c315e8
USES_TERMINAL_DOWNLOAD TRUE
GIT_PROGRESS TRUE
)
endif()

fetchcontent_makeavailable(protoc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should protoc be installed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just downloads the binary packages, the installation of protoc takes a bit. make install will not include protoc since it is not a CMake package.

# Exclude Protobuf from the main make install step. We only want to use it locally.
fetchcontent_makeavailable_but_exclude_install(protobuf)

# Reset unity builds to the previous state...
set(CMAKE_UNITY_BUILD ${SAVED_CMAKE_UNITY_BUILD})
set(FETCHCONTENT_QUIET ${FETCHCONTENT_QUIET_PREV})

set(PROTOBUF_PROTOC_EXECUTABLE ${protoc_SOURCE_DIR}/bin/protoc CACHE STRING
"Protoc executable."
)
set(Protobuf_INCLUDE_DIR ${protobuf_SOURCE_DIR}/src/ CACHE STRING "Protobuf include directory.")
set(PROTOBUF_LIBRARY libprotobuf)
message("Done with setting up Protobuf for P4C.")
endif()
endmacro(p4c_obtain_protobuf)
6 changes: 5 additions & 1 deletion control-plane/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,15 @@ set (CONTROLPLANE_HDRS
)

add_library(controlplane-gen OBJECT ${P4RUNTIME_GEN_SRCS})
target_include_directories(controlplane-gen
SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
)
target_link_libraries (controlplane-gen PUBLIC ${PROTOBUF_LIBRARY})

include_directories (${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR})
# Silence -Warray-bounds as the root issue is out of our control: https://github.com/protocolbuffers/protobuf/issues/7140
set_source_files_properties (${P4RUNTIME_GEN_SRCS} PROPERTIES COMPILE_FLAGS "-Wno-unused-parameter -Wno-array-bounds")

add_library (controlplane STATIC ${CONTROLPLANE_SRCS} )
target_link_libraries (controlplane ${PROTOBUF_LIBRARY} ${CMAKE_THREAD_LIBS_INIT} controlplane-gen)
target_link_libraries (controlplane ${CMAKE_THREAD_LIBS_INIT} controlplane-gen)
add_dependencies (controlplane mkP4configdir ir-generated controlplane-gen frontend-parser-gen)
2 changes: 1 addition & 1 deletion tools/ci-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ if [[ "${DISTRIB_RELEASE}" == "18.04" ]] || [[ "$(which simple_switch 2> /dev/nu
# Use GCC 9 from https://launchpad.net/~ubuntu-toolchain-r/+archive/ubuntu/test
sudo apt-get update && sudo apt-get install -y software-properties-common
sudo add-apt-repository -uy ppa:ubuntu-toolchain-r/test
P4C_DEPS+=" libprotobuf-dev protobuf-compiler gcc-9 g++-9"
P4C_DEPS+=" gcc-9 g++-9"
export CC=gcc-9
export CXX=g++-9
else
Expand Down
2 changes: 0 additions & 2 deletions tools/install_fedora_deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ sudo dnf install -y -q \
openssl-devel \
pkg-config \
procps-ng \
protobuf-devel \
protobuf-static \
python3 \
python3-pip \
python3-thrift \
Expand Down
11 changes: 3 additions & 8 deletions tools/install_mac_deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,15 @@ if [[ ! -x $BREW ]]; then
/usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
fi

# Google introduced another breaking change with protobuf 22.x by adding abseil as a new dependency.
# https://protobuf.dev/news/2022-08-03/#abseil-dep
# We do not want abseil, so we stay with 21.x for now.
PROTOBUF_LIB="protobuf@21"
BOOST_LIB="[email protected]"

$BREW update
$BREW install autoconf automake bdw-gc bison ${BOOST_LIB} ccache cmake \
libtool openssl pkg-config python coreutils grep ${PROTOBUF_LIB}
libtool openssl pkg-config python coreutils grep

# Prefer Homebrew's bison, grep, and protobuf over the macOS-provided version
$BREW link --force bison grep ${PROTOBUF_LIB} ${BOOST_LIB}
# Prefer Homebrew's bison and grep over the macOS-provided version
$BREW link --force bison grep ${BOOST_LIB}
echo 'export PATH="/usr/local/opt/bison/bin:$PATH"' >> ~/.bash_profile
echo 'export PATH="/usr/local/opt/${PROTOBUF_LIB}/bin:$PATH"' >> ~/.bash_profile
echo 'export PATH="/usr/local/opt/grep/libexec/gnubin:$PATH"' >> ~/.bash_profile
export PATH="/usr/local/opt/bison/bin:$PATH"
export PATH="/usr/local/opt/grep/libexec/gnubin:$PATH"
Expand Down