Build errors with Protobuf 23 #4055

jkhsjdhjs · 2023-07-08T16:42:37Z

Hey,

I recently created a package of the p4 compiler for the Arch Linux User Repository: https://aur.archlinux.org/packages/p4lang-p4c
It was already a bit complicated to build it, but I eventually got it working by using CMAKE_UNITY_BUILD=ON (for some reason the compilation error-ed without it) and by specifying PROTOBUF_LIBRARY=protobuf, because it would complain about the Protobuf version or not find it while linking (see https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=p4lang-p4c#n50).

Recently, the Arch Linux developers updated protobuf from 21.12 to 23.3 (don't ask me about these version numbers, but that seems to correspond to library version 4.23.3, while the previous one was 3.21.12). Anyway, this protobuf version depends on the abseil cpp libraries, and since CMakeLists.txt currently only links against Protobuf directly, this leaves undefined references that cause the build to fail. I found a possible solution to that problem here: protocolbuffers/protobuf#12292 (comment)
By adding CONFIG to the find_package call, CMake should load the respective protobuf cmake files, which would then also link the program correctly against the abseil libraries and other possible dependencies of protobuf.

However, since adding this (and changing the version number from 3.0.0 to 23.3.0, because it complained), the build fails at the Genering protobuf dependencies step with:

[ 47%] Generating protobuf files
/bin/sh: line 1: -I: command not found
make[2]: *** [control-plane/CMakeFiles/controlplane-gen.dir/build.make:78: control-plane/google/rpc/status.pb.cc] Error 127
make[1]: *** [CMakeFiles/Makefile2:1871: control-plane/CMakeFiles/controlplane-gen.dir/all] Error 2
make: *** [Makefile:166: all] Error 2

Running make with -d for debug mode shows the following:

[ 47%] Generating protobuf files
Reaping winning child 0x561bb9b00500 PID 192167 
cd /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/build/control-plane && -I /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane/p4runtime/proto -I/home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane --cpp_out /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/build/control-plane --python_out /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/build/control-plane /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane/google/rpc/status.proto /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane/p4runtime/proto/p4/config/v1/p4info.proto /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane/p4runtime/proto/p4/config/v1/p4types.proto /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane/p4runtime/proto/p4/v1/p4runtime.proto /home/jkhsjdhjs/aur/p4lang-p4c/src/p4lang-p4c/control-plane/p4runtime/proto/p4/v1/p4data.proto
Live child 0x561bb9b00500 (control-plane/google/rpc/status.pb.cc) PID 192168 
/bin/sh: line 1: -I: command not found
Reaping losing child 0x561bb9b00500 PID 192168 
make[2]: *** [control-plane/CMakeFiles/controlplane-gen.dir/build.make:78: control-plane/google/rpc/status.pb.cc] Error 127
Removing child 0x561bb9b00500 PID 192168 from chain.
Reaping losing child 0x55a33b04adc0 PID 192166 
make[1]: *** [CMakeFiles/Makefile2:1871: control-plane/CMakeFiles/controlplane-gen.dir/all] Error 2
Removing child 0x55a33b04adc0 PID 192166 from chain.
Reaping losing child 0x55ed0f977d70 PID 192061 
make: *** [Makefile:166: all] Error 2
Removing child 0x55ed0f977d70 PID 192061 from chain.

The third line shows, that the command executed if the cd command succeeds is simply -I, so I assume that some variable containing the path to some executable (maybe the protoc executable), is now no longer defined.

Do you have any idea what might be going wrong here? Thanks in advance!

The text was updated successfully, but these errors were encountered:

fruffy · 2023-07-08T16:47:28Z

Protobuf has been a persistent source of pain because Google consistently introduces breaking changes. Wish we did not depend on it...

Let me check in the next couple of days. We have never tested Protobuf beyond version 21 because 22 introduces Abseil etc.

We should fix the Protobuf dependency using CMake's FetchContent, that could get rid of a lot of these problems.

davidbolvansky · 2023-07-08T17:05:19Z

Exactly :) In other project I have quite good experience with ExternalProject_Add / FetchContent and fixed version of Protobuf.

fruffy · 2023-07-09T20:56:45Z

Exactly :) In other project I have quite good experience with ExternalProject_Add / FetchContent and fixed version of Protobuf.

Is this project public? Do you build from scratch or do you use prebuilt binaries? I'd prefer the latter since protobuf is big and slow to build...

davidbolvansky · 2023-07-09T21:29:49Z

Not a public one. We use prebuilt binaries as well, good direction :)

fruffy · 2023-07-09T21:56:33Z

Not a public one. We use prebuilt binaries as well, good direction :)

Were do you get prebuilt libprotobuf(-lite) files from? Iirc they are not supplied with the Github zips for some reason.

davidbolvansky · 2023-07-09T22:02:33Z

Well yeah, we build it manually (not only protobuf..) and store prebuilt libraries.

Maybe p4c could follow same steps..

fruffy · 2023-07-10T15:25:53Z

@jkhsjdhjs Could you try this PR #4056 to check whether it helps your case?

jkhsjdhjs · 2023-07-10T18:13:49Z

@fruffy Thanks for your work on this! I just tested it, and it builds fine using the downloaded protobuf version. I did see 2 CMake warnings though:

Fetching Protobuf version 21.12 for P4C...
CMake Warning (dev) at /usr/share/cmake/Modules/FetchContent.cmake:1282 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  cmake/Protobuf.cmake:33 (fetchcontent_declare)
  CMakeLists.txt:136 (p4c_obtain_protobuf)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at /usr/share/cmake/Modules/FetchContent.cmake:1282 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  cmake/Protobuf.cmake:51 (fetchcontent_declare)
  CMakeLists.txt:136 (p4c_obtain_protobuf)
This warning is for project developers.  Use -Wno-dev to suppress it.

If I try to install the build package however, I see that the package including protobuf is 80MB larger than without it, and that the included protobuf files currently conflict with the system protobuf:

resolving dependencies...
looking for conflicting packages...

Packages (1) p4lang-p4c-1.2.4.0-1

Total Installed Size:  435,17 MiB
Net Upgrade Size:       80,55 MiB

:: Proceed with installation? [Y/n] 
(1/1) checking keys in keyring                     [#####################################] 100%
(1/1) checking package integrity                   [#####################################] 100%
(1/1) loading package files                        [#####################################] 100%
(1/1) checking for file conflicts                  [#####################################] 100%
error: failed to commit transaction (conflicting files)
p4lang-p4c: /usr/include/google/protobuf/any.h exists in filesystem (owned by protobuf)
p4lang-p4c: /usr/include/google/protobuf/any.pb.h exists in filesystem (owned by protobuf)
p4lang-p4c: /usr/include/google/protobuf/any.proto exists in filesystem (owned by protobuf)
p4lang-p4c: /usr/include/google/protobuf/api.pb.h exists in filesystem (owned by protobuf)
p4lang-p4c: /usr/include/google/protobuf/api.proto exists in filesystem (owned by protobuf)
[...]

Not sure what can be done about that.

Because I'd prefer using the system protobuf anyway, I also tried building with 21.12 installed and P4C_USE_PREINSTALLED_PROTOBUF=ON, but that version doesn't seem to be accepted by CMake for some reason, despite having a higher version than the minimum required:

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Protobuf (missing: Protobuf_LIBRARIES) (found suitable
  version "3.21.12", minimum required is "3.0.0")
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake/Modules/FindProtobuf.cmake:650 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  cmake/Protobuf.cmake:14 (find_package)
  CMakeLists.txt:136 (p4c_obtain_protobuf)

Even if I remove the version requirement entirely, it still doesn't work:

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Protobuf (missing: Protobuf_LIBRARIES) (found version
  "3.21.12")
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake/Modules/FindProtobuf.cmake:650 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  cmake/Protobuf.cmake:14 (find_package)
  CMakeLists.txt:136 (p4c_obtain_protobuf)

This can be worked around my adding -DPROTOBUF_LIBRARY=protobuf, but maybe there's also something that can be done in the CMake script about this.

I also have a feeling it would work with Protobuf 23, since the only problem I had was linking with abseil. But I can understand if you don't wanna work on this, it really is a PITA.

fruffy · 2023-07-10T18:31:40Z

@fruffy Thanks for your work on this! I just tested it, and it builds fine using the downloaded protobuf version. I did see 2 CMake warnings though:

Great to hear the FetchContent version builds. I believe the warnings come from newer version of CMake. They are easy to fix but older versions may complain.

This looks like a CMake<-->Arch problem to me. I am unsure how find_package could break here. Does this happen with a clean CMakeCache?

This is hard to fix since I currently do not run Arch. How does the package conflict happen, are you running make install?

jkhsjdhjs · 2023-07-10T19:08:15Z

I believe the warnings come from newer version of CMake. They are easy to fix but older versions may complain.

Yeah, you're right. DOWNLOAD_EXTRACT_TIMESTAMP was only recently added in 3.24, so older versions will probably complain.

This looks like a CMake<-->Arch problem to me. I am unsure how find_package could break here. Does this happen with a clean CMakeCache?

Yep, happens with a clean CMake cache. I just ran cmake with --debug-find and noticed that it only considers static libraries (.a), that aren't provided by the protobuf package on Arch Linux. Adding -DENABLE_PROTOBUF_STATIC=OFF resolves this issue.

This is hard to fix since I currently do not run Arch. How does the package conflict happen, are you running make install?

Bascially what I'm doing is creating a package from the make install command, see the last few lines of this file: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=p4lang-p4c
The PKGBUILD file is just an instruction on how to create the package, which can be executed by makepkg on Arch Linux. It first runs the prepare() function, build(), check() and finally package(). The result is a tar archive, that can be installed via the package manager. The conflict happens during install via the package manager, since the protobuf libraries included with p4c are also installed to /usr/include/google/protobuf during make install, just like protobuf provided by this package, that I currently have installed: https://archlinux.org/packages/extra/x86_64/protobuf/

fruffy · 2023-07-10T19:31:30Z

Yeah, you're right. DOWNLOAD_EXTRACT_TIMESTAMP was only recently added in 3.24, so older versions will probably complain.

I think this is fixable but the workaround is a little awkward.

Yep, happens with a clean CMake cache. I just ran cmake with --debug-find and noticed that it only considers static libraries (.a), that aren't provided by the protobuf package on Arch Linux. Adding -DENABLE_PROTOBUF_STATIC=OFF resolves this issue.

Great, are you able to build P4C now?

Bascially what I'm doing is creating a package from the make install command, see the last few lines of this file:

I see, I did not consider that. Maybe it is possible to choose separate install path for the local P4C version.

jkhsjdhjs · 2023-07-10T19:50:14Z

Great, are you able to build P4C now?

I can build it with Protobuf 21.12 without the -DPROTOBUF_LIBRARY=protobuf flag now, but with Protobuf 23 I still get undefined abseil references. If I try to add CONFIG to the find_package () call, as suggested in protocolbuffers/protobuf#12292 (comment), and change the version requirement to 23.3.0, I get /bin/sh: line 1: -I: command not found as in the original issue description.

I see, I did not consider that. Maybe it is possible to choose separate install path for the local P4C version.

Would be great!

fruffy · 2023-07-10T20:37:11Z

I can build it with Protobuf 21.12 without the -DPROTOBUF_LIBRARY=protobuf flag now, but with Protobuf 23 I still get undefined abseil references. If I try to add CONFIG to the find_package () call, as suggested in protocolbuffers/protobuf#12292 (comment), and change the version requirement to 23.3.0, I get /bin/sh: line 1: -I: command not found as in the original issue description.

I was able to get Protobuf 23 to build with the FetchContent setup, but it is finicky. The Protobuf/Abseil CMake installation still seems a bit broken. So I would prefer to keep things below 22.x for now. Although at some point we definitely have to migrate.

Would be great!

Okay, let me look into that.

fruffy · 2023-07-10T21:09:18Z

@jkhsjdhjs I pushed a new commit that (hopefully) excludes Protobuf from the make install. Things seem to work alright but I am in untested waters here. Can you confirm it works on your end?

jkhsjdhjs · 2023-07-10T21:41:55Z

Yep, that works, thanks for your work on this! Since all the executables are now quite a bit larger, I assume the protobuf library is now baked directly into them, right?

BTW: Did you do anything special to build p4c with protobuf 23? If not, this may as well be an issue with the protobuf installation on Arch Linux. In this case I might check back with the Arch Linux developers in the next days and see if they have an idea what might be going on.

fruffy · 2023-07-11T13:33:20Z

Yep, that works, thanks for your work on this! Since all the executables are now quite a bit larger, I assume the protobuf library is now baked directly into them, right?

Yes, a lot of the compiler libraries including Protobuf are linked statically. Although I am surprised this was not the case before here.

Did you do anything special to build p4c with protobuf 23?

Iirc no. I checked out the version on Github and built it by adding Protobuf as a CMake subdirectory. I had to add the includes/libraries manually, but that's it.

I think this issue is very much related, maybe CMake is simply broken here?

jkhsjdhjs · 2023-07-12T17:11:37Z

Yes, a lot of the compiler libraries including Protobuf are linked statically. Although I am surprised this was not the case before here.

Ah, this is probably done to keep the binaries as portable as possible or to avoid issues with breaking changes of dependencies.
Linking dynamically would save quite some space, but I can understand that you don't want to take care of issues titled Build errors with <dependency> <version> all the time :D

Also regarding that: Currently, this repository already has three submodules. If protobuf is compiled from source anyway, wouldn't it make sense to add that as a submodule as well, instead of downloading it via CMake? This way, the build would not requiring internet access, and keep the dependency management consistent with the other dependencies.
On Arch Linux, the build scripts are also divided into several steps -- as I shortly elaborated in a previous comment -- and using protobuf as a submodule would keep the steps cleanly separated, as the sources could be acquired in one go, with the build step not downloading additional sources.

Iirc no. I checked out the version on Github and built it by adding Protobuf as a CMake subdirectory. I had to add the includes/libraries manually, but that's it.

I think this issue is very much related, maybe CMake is simply broken here?

Yeah, that's the issue I also linked. A comment there suggests to add CONFIG to the find_package () call, which allows the build scripts to generate successfully, but the actual compilation with make fails with /bin/sh: line 1: -I: command not found at the Generating protobuf files step.

fruffy · 2023-07-12T20:32:57Z

Also regarding that: Currently, this repository already has three submodules. If protobuf is compiled from source anyway, wouldn't it make sense to add that as a submodule as well, instead of downloading it via CMake? This way, the build would not requiring internet access, and keep the dependency management consistent with the other dependencies. On Arch Linux, the build scripts are also divided into several steps -- as I shortly elaborated in a previous comment -- and using protobuf as a submodule would keep the steps cleanly separated, as the sources could be acquired in one go, with the build step not downloading additional sources.

There is actually a PR to consolidate these submodules: #3970
FetchContent can be a thin wrapper around submodules. Since it integrates fetching and dependency management into CMake it removes one extra unnecessary step and is much more convenient to manage. It should also work without internet access in the same way git submodule update --init --recursive does. For the Arch Linux setup, I would separate the cmake .. and make step, if possible.

Yeah, that's the issue I also linked. A comment there suggests to add CONFIG to the find_package () call, which allows the build scripts to generate successfully, but the actual compilation with make fails with /bin/sh: line 1: -I: command not found at the Generating protobuf files step.

Hmm, it looks like there is no clear fix available right now. This might require some trial and error.

fruffy · 2023-07-25T22:34:51Z

I merged #4056 but will leave this one open until the CMake (or Protobuf?) package is fixed.

Google introduced abseil as a new dependency of Protobuf, but CMake's FindProtobuf hasn't been updated yet to link with abseil. Thus, CMake is now advised to use the config provided by Protobuf instead of its own. In case no config can be found, we fall back to the old behavior. The Protobuf version constraint is removed since it only applies with `P4C_USE_PREINSTALLED_PROTOBUF=ON` anyway, in which case we'll want to use whatever preinstalled version is available. Furthermore, the variables `PROTOBUF_LIBRARY` and `PROTOBUF_PROTOC_EXECUTABLE` are set correctly so executing `protoc` and linking with Protobuf doesn't fail. Fix p4lang#4055 See also protocolbuffers/protobuf#12292

Google introduced abseil as a new dependency of Protobuf, but CMake's FindProtobuf hasn't been updated yet to link with abseil. Thus, CMake is now advised to use the config provided by Protobuf instead of its own. In case no config can be found, we fall back to the old behavior. The Protobuf version constraint is removed since it only applies with `P4C_USE_PREINSTALLED_PROTOBUF=ON` anyway, in which case we'll want to use whatever preinstalled version is available. Furthermore, the variables `PROTOBUF_LIBRARY` and `PROTOBUF_PROTOC_EXECUTABLE` are set correctly so executing `protoc` and linking with Protobuf doesn't fail. Fix #4055 See also protocolbuffers/protobuf#12292

fruffy added the bug This behavior is unintended and should be fixed. label Jul 8, 2023

fruffy self-assigned this Jul 8, 2023

fruffy added the control-plane Topics related to the control-plane or P4Runtime. label Jul 8, 2023

fruffy mentioned this issue Jul 10, 2023

Install Protobuf using FetchContent. #4056

Merged

jkhsjdhjs mentioned this issue Jul 12, 2023

C++: Undefine reference to functions defined in absl::lts_20230125 protocolbuffers/protobuf#12292

Closed

jkhsjdhjs mentioned this issue Aug 12, 2023

Fix linking with Protobuf 23 #4104

Merged

fruffy closed this as completed in #4104 Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build errors with Protobuf 23 #4055

Build errors with Protobuf 23 #4055

jkhsjdhjs commented Jul 8, 2023

fruffy commented Jul 8, 2023

davidbolvansky commented Jul 8, 2023

fruffy commented Jul 9, 2023

davidbolvansky commented Jul 9, 2023

fruffy commented Jul 9, 2023

davidbolvansky commented Jul 9, 2023

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 10, 2023 •

edited

Loading

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 11, 2023

jkhsjdhjs commented Jul 12, 2023

fruffy commented Jul 12, 2023

fruffy commented Jul 25, 2023

Build errors with Protobuf 23 #4055

Build errors with Protobuf 23 #4055

Comments

jkhsjdhjs commented Jul 8, 2023

fruffy commented Jul 8, 2023

davidbolvansky commented Jul 8, 2023

fruffy commented Jul 9, 2023

davidbolvansky commented Jul 9, 2023

fruffy commented Jul 9, 2023

davidbolvansky commented Jul 9, 2023

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 10, 2023 • edited Loading

fruffy commented Jul 10, 2023

jkhsjdhjs commented Jul 10, 2023

fruffy commented Jul 11, 2023

jkhsjdhjs commented Jul 12, 2023

fruffy commented Jul 12, 2023

fruffy commented Jul 25, 2023

fruffy commented Jul 10, 2023 •

edited

Loading