8000 Avoid breakages for WITH_LLVM builds with libprotobuf < 3.19.0 by dhoekwater · Pull Request #200 · google/autofdo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Avoid breakages for WITH_LLVM builds with libprotobuf < 3.19.0 #200

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 15, 2024
Merged

Avoid breakages for WITH_LLVM builds with libprotobuf < 3.19.0 #200

merged 1 commit into from
Jul 15, 2024

Conversation

dhoekwater
Copy link
Collaborator
@dhoekwater dhoekwater commented Jul 12, 2024 •
edited
Loading

Recent commit 623c777 introduced spe_tid_pid_provider, which includes
repeated_ptr_field.h (RepeatedPtrField was only moved to its own
header in 3.19.0).

repeated_field.h transitively includes repeated_ptr_field in new
versions of libprotobuf for backwards compatibility reasons, so we can
include that instead.

@dhoekwater dhoekwater changed the title master Avoid breakages for WITH_LLVM builds with libprotobuf < 3.19.0 Jul 12, 2024
@dhoekwater
Copy link
Collaborator Author

This fixes #193 for WITH_LLVM builds.

Tested on Ubuntu 20.04 with libprotoc 3.6.1 and on Debian 6.6.15 with libprotoc 3.21.12.

I did have to install zstd, so we should add this to the dependencies list.

@dhoekwater dhoekwater mentioned this pull request Jul 12, 2024
Closed
@dhoekwater
Avoid breakages for WITH_LLVM builds with libprotobuf < 3.19.0
3977fd1 
Recent commit 623c777 introduced spe_tid_pid_provider, which includes
`repeated_ptr_field.h` (`RepeatedPtrField` was only moved to its own
header in 3.19.0).

`repeated_field.h` transitively includes `repeated_ptr_field` in new
versions of libprotobuf for backwards compatibility reasons, so we can
include that instead.
@snehasish
Copy link
Collaborator

Using the WITH_LLVM option on Ubuntu 20.04, I am unable to build with the cmake installed from the repository.

CMake Error at third_party/llvm-project/llvm/CMakeLists.txt:3 (cmake_minimum_required):
  CMake 3.20.0 or higher is required.  You are running version 3.16.3

Can you confirm the version of cmake on your Ubuntu 20.04 machine?

Also should we bump the supported system to Ubuntu 22.04?

@shenhanc78
Copy link
Collaborator
shenhanc78 commented Jul 12, 2024 •
edited
Loading

Yes, cmake package is stuck with 3.16.3 for ubuntu 20.04. And there is no easy way around it except to build/install by oneself.

Tried build with 22.04 LTS, it builds successfully. The protobuf installed is libprotobuf-dev 3.12.4.

Also should we bump the supported system to Ubuntu 22.04?

Since this is a restriction enforced by llvm and we are building on top of llvm, I think it is reasonable to align with llvm's decision.

Thanks for the fix.

@dhoekwater
Copy link
Collaborator Author
dhoekwater commented Jul 12, 2024 •
edited
Loading

The version of cmake I used was 3.29.6. The default on Ubuntu 20.04 is 3.16.3, but now that Kitware maintains APT repositories for different Ubuntu versions, upgrading is as simple as:

# Follow steps in https://askubuntu.com/questions/355565/how-do-i-install-the-latest-version-of-cmake-from-the-command-line
$ sudo apt purge --auto-remove cmake
$ wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc 2>/dev/null | gpg --dearmor - | sudo tee /etc/apt/trusted.gpg.d/kitware.gpg >/dev/null
$  sudo apt-add-repository 'deb https://apt.kitware.com/ubuntu/ focal main'     
$  sudo apt update && sudo apt install cmake

I'm good with bumping the officially supported version of Ubuntu to 22.04, but it would be helpful for 20.04 users to know that all they have to do is update cmake.

@snehasish snehasish mentioned this pull request Jul 12, 2024
Merged
@snehasish
Copy link
Collaborator

I was able to verify the fix on my Ubuntu 22.04 VM. I see that the addr2cu_test target fails unless libzstd-dev is installed. Thanks for pointing that out, I'll add it to the README.

@shenhanc78 I think we should make the failure clearer by setting set (LLVM_ENABLE_ZSTD, FORCE_ON) in the updated cmake file. Can you incorporate this change in #199 too?

Kitware maintains APT repositories for different Ubuntu versions

Good idea, I'll note this for Ubuntu 20.04 systems when I update the README. I'll go ahead and merge this.

@snehasish snehasish merged commit fb8ed8b into google:master Jul 15, 2024
1 check passed
@dhoekwater dhoekwater mentioned this pull request Apr 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhoekwater @snehasish @shenhanc78
0