Add cmake build support #1308

grg · 2025-06-02T00:51:00Z

Cleaning up PR #1303 from sudhanshu112233shukla.

Items that need to be addressed in future PRs:

~~Replicate the Thrift Python file handling. (The Python files are generated, but any additional steps are not performed, such as installing.)~~ DONE - 6/6/25.
~~Verify that make install installs all necessary files in the correct locations.~~ DONE - 6/6/25.
Review visibility of include and library usage (PUBLIC/PRIVATE/...)
Review generation of static vs shared libraries.
Add header files to source lists. This is not needed by cmake, but is apparently used by some IDEs.

Detailed status is in the file CMAKE_IMPLEMENTATION_SUMMARY.md

Updates:

6/6/25 - Pulled in the lpm_trie changes (Convert bf_lpm_trie.c into C++ style #1297) from main. Updated cmake files to reflect changes.

sudhanshu112233shukla · 2025-06-02T17:14:43Z

Hi 👋

I've thoroughly reviewed your excellent work on cleaning up and completing the CMake implementation. This is a fantastic transformation from the initial foundation into a production-ready build system. Here's my detailed review:
Outstanding Improvements
Build System Completeness
Excellent work on converting all remaining Makefile.am files to CMakeLists.txt
Perfect implementation of Thrift generation system with proper dependency handling
Smart solution for handling Thrift output differences with the touch workaround
Great attention to detail in matching autoconf and cmake systems behavior
Test Infrastructure
Brilliant fix separating gRPC tests into different groups to avoid parallel execution issues
Proper test dependency management - building before running ctest
Excellent debugging with --output-on-failure for better CI visibility
Smart handling of undeterministic tests with proper conditionals
Dependency & Linking
Critical fixes for platform-specific issues (atomic library for clang, pthread for bmapps)
Proper conditional linking (e.g., only linking psaswitch_thrift when Thrift enabled)
Excellent dependency ordering - including test directory later to avoid circular deps
Great variable configuration for configure_file templates
CI/CD Pipeline
Perfect CI fixes - timezone handling, script permissions, shell selection
Smart parallelization with matrix builds for install tests
Excellent error handling and debugging output
Production-ready Ubuntu 22.04 compatibility
ling edge cases, platform compatibility, and CI/CD integration is outstanding.

grg · 2025-06-02T21:29:26Z

@fruffy / @jafingerhut Here's an updated version of @sudhanshu112233shukla's PR (#1303). They've reviewed it and seem happy.

I've asked them to complete the DCO sign-off process on their original PR as I pull some of their commits. Once that's done, I'll re-pull those commits and re-push. I believe that all my commits have the sign-off.

fruffy · 2025-06-02T22:43:26Z

Hi 👋

I've thoroughly reviewed your excellent work on cleaning up and completing the CMake implementation. This is a fantastic transformation from the initial foundation into a production-ready build system. Here's my detailed review: Outstanding Improvements Build System Completeness Excellent work on converting all remaining Makefile.am files to CMakeLists.txt Perfect implementation of Thrift generation system with proper dependency handling Smart solution for handling Thrift output differences with the touch workaround Great attention to detail in matching autoconf and cmake systems behavior Test Infrastructure Brilliant fix separating gRPC tests into different groups to avoid parallel execution issues Proper test dependency management - building before running ctest Excellent debugging with --output-on-failure for better CI visibility Smart handling of undeterministic tests with proper conditionals Dependency & Linking Critical fixes for platform-specific issues (atomic library for clang, pthread for bmapps) Proper conditional linking (e.g., only linking psaswitch_thrift when Thrift enabled) Excellent dependency ordering - including test directory later to avoid circular deps Great variable configuration for configure_file templates CI/CD Pipeline Perfect CI fixes - timezone handling, script permissions, shell selection Smart parallelization with matrix builds for install tests Excellent error handling and debugging output Production-ready Ubuntu 22.04 compatibility ling edge cases, platform compatibility, and CI/CD integration is outstanding.

Please do not use AI tools blindly, that is basically spam. Use your own judgement on the output and only comment what you think makes sense.

grg · 2025-06-06T19:14:09Z

Rebased to current main to pull in the lpm trie changes. (Originally merged, but decided to rebase to make it easier later to pull in the signed-off versions of the first two commits.)

fruffy

Some initial comments.

PR_DESCRIPTION.md

README.md

fruffy · 2025-06-07T12:23:47Z

ci/install-thrift.sh

@@ -12,7 +12,12 @@ set -e
 git clone -b 0.13.0 https://github.com/apache/thrift.git thrift-0.13.0
 cd thrift-0.13.0
 ./bootstrap.sh
-./configure --with-cpp=yes --with-c_glib=no --with-java=no --with-ruby=no --with-erlang=no --with-go=no --with-nodejs=no
+./configure --with-as3=no --with-c_glib=no --with-csharp=no --with-cpp=yes \


fwiw, Thrift also has a CMake-build now, which can be used instead of CMake. See this setup in the Tofino studio:
https://github.com/p4lang/open-p4studio/blob/main/p4studio/dependencies/source/install_thrift.py#L52

I'll look at switching 8000 to the cmake version.

I tried switching to using the cmake thrift build but then ran into issues building bmv2. I suggest we leave as is now and then switch to the cmake build later. (I don't think the switch for thrift has to be done at the same time as the switch for bmv2.)

cmake/GenerateThrift.cmake

Signed-off-by: sudhanshu shukla <161225935+sudhanshu112233shukla@users.noreply.github.com>

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Add additional CMakeLists.txt files neeeded by the ubuntu-22 tests Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Also attempt to fix a dependency issue Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Improve python generation and installation Adjust install rules to better match autoconf setup. Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

More information from CMAKE_IMPLEMENTATION_SUMMARY.md into README.md. Update the description in README.md to reflec the current status. Remove uneeded PR_DESCRIPTION.md file. Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

fruffy

Approving, with some nits. Unclear to me whether the install step is working the same as autoconf here, but we can fix that later. Nice to see CMake support finally!

services/CMakeLists.txt

fruffy · 2025-06-18T01:20:16Z

tests/CMakeLists.txt

+  test_actions
+  test_ageing
+  test_assert_assume
+  test_bm_apps


Only works with nanomsg?

You're probably right that this test should only be built if nanomsg is enabled. There may be others as well.

The same issue appears to exist with the autotools build. The tests dir is included when thrift is enabled, and Makefile.am in tests does not have anything conditional on nanomsg.

I've filed #1310 so that this doesn't get forgotten about.

src/bm_apps/CMakeLists.txt

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg · 2025-06-19T16:22:37Z

@fruffy Can you merge this PR please? GitHub says I'm not authorized to push to that branch.

antoninbas · 2025-06-23T17:56:32Z

@grg @fruffy CI ha been failing on main since this PR was merged

grg · 2025-06-23T20:32:17Z

@grg @fruffy CI ha been failing on main since this PR was merged

Ugh 🙁 Looking into now.

Fix breakage to the build-no-pi job introduced by p4lang#1308

Fix breakage to the build-no-pi job introduced by p4lang#1308 Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg · 2025-06-23T21:15:59Z

@grg @fruffy CI ha been failing on main since this PR was merged

Ugh 🙁 Looking into now.

Fix in #1311.

Fix breakage to the build-no-pi job introduced by p4lang#1308 Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg force-pushed the gleng/cmake_support branch 2 times, most recently from c37746a to 7315eae Compare June 2, 2025 02:41

grg force-pushed the gleng/cmake_support branch 2 times, most recently from 084ea46 to 281eae8 Compare June 2, 2025 20:58

grg marked this pull request as ready for review June 2, 2025 21:26

grg force-pushed the gleng/cmake_support branch 3 times, most recently from 6edced4 to a7c4990 Compare June 6, 2025 19:13

fruffy reviewed Jun 7, 2025

View reviewed changes

sudhanshu112233shukla and others added 17 commits June 16, 2025 13:38

Add cmake

45a10ac

Signed-off-by: sudhanshu shukla <161225935+sudhanshu112233shukla@users.noreply.github.com>

builds/files

cfdccad

Signed-off-by: sudhanshu shukla <161225935+sudhanshu112233shukla@users.noreply.github.com>

update cmake tests to ubuntu-latest

979ca9d

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

invoke install scripts with sh

417675c

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

get a cmake build compiling from the command line

5061b4b

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

switch to bash instead of sh for install scripts

6878d95

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

debug: adjust thrift build

a9a48fc

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

disable non-nanomsg test

d31ac8a

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

psa_switch/pna_nic binaries require thrift

e65e41f

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

add thrift generation dependencies

e0dedb2

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

additional cmake files

2645324

Add additional CMakeLists.txt files neeeded by the ubuntu-22 tests Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

replace "make check" with ctest

307a19f

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

remove find package for absl

9cd7bcf

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

build before running ctest

926f1f2

Also attempt to fix a dependency issue Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

build fixes

6f134b1

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

don't prompt for timezone

9bc0f59

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

fall back to chmod for cmake < 3.19

d54ef5c

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg added 15 commits June 16, 2025 13:38

add cmake details to README.md

04e75c1

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

update implementation status doc

720b8b7

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

run old and new tests

33fd05b

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

tweak use of if statement in Dockerfile

5544d6e

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

use fi instead of endif in shell scrtipt :)

dd2628d

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

handle old/new build system in workflow

614b5d8

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

fix

312f423

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

fix if statement

86a0c6b

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

move things into the corect spot in if statements

e45d32e

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

env section for test run

cc69321

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

temporarily revert tests to autoconf

f62d7e6

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

python + install fixes

70b5a2c

Improve python generation and installation Adjust install rules to better match autoconf setup. Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

update cmake status; remove README.cmake.md

a7f6af5

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

install bmpi library

c2515d3

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

update to match removed trie code

6bab58e

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg force-pushed the gleng/cmake_support branch from 6b10adf to 92c48c0 Compare June 16, 2025 20:39

update readme; remove unneeded md files

902b240

More information from CMAKE_IMPLEMENTATION_SUMMARY.md into README.md. Update the description in README.md to reflec the current status. Remove uneeded PR_DESCRIPTION.md file. Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg force-pushed the gleng/cmake_support branch from 92c48c0 to 902b240 Compare June 16, 2025 21:28

fruffy approved these changes Jun 18, 2025

View reviewed changes

address PR feedback

2acd745

Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg merged commit 0887960 into p4lang:main Jun 19, 2025
10 checks passed

grg deleted the gleng/cmake_support branch June 19, 2025 16:47

grg added a commit to ai-fabrics/behavioral-model-public that referenced this pull request Jun 23, 2025

fix CI job: build-no-pi

03b5ccf

Fix breakage to the build-no-pi job introduced by p4lang#1308

grg mentioned this pull request Jun 23, 2025

fix CI job: build-no-pi #1311

Merged

grg added a commit to ai-fabrics/behavioral-model-public that referenced this pull request Jun 23, 2025

fix CI job: build-no-pi

e958eaf

Fix breakage to the build-no-pi job introduced by p4lang#1308 Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

grg added a commit to ai-fabrics/behavioral-model-public that referenced this pull request Jun 23, 2025

fix CI job: build-no-pi

360ab0b

Fix breakage to the build-no-pi job introduced by p4lang#1308 Signed-off-by: Glen Gibb <gleng@ai-fabrics.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add cmake build support #1308

Add cmake build support #1308

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add cmake build support #1308

Add cmake build support #1308

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!