-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Mac: build debug and release versions of protobuf #2042
Conversation
@kevinwilfong has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why is this not a problem for Linux builds.
scripts/setup-macos.sh
Outdated
@@ -109,6 +109,16 @@ function install_re2 { | |||
cmake_install -DRE2_BUILD_TESTING=OFF | |||
} | |||
|
|||
function install_protobuf { | |||
# Protobuf's optimized build and debug build are incompatible when linked with code from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what causes Velox build to list against both build and debug versions of the protobuf. CC: @rui-mo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why is this not a problem for Linux builds.
I mentioned in the description, I suspect it's because we use the protobuf-devel package which I'm guessing includes the debug build. (Lots of assumptions there, I don't know for sure)
@kevinwilfong It seems the protobuf 3.21.2 still have problems building from source. Shall we install a previous version? |
It works on my personal Mac... let me try playing with it a bit. We're hitting an issue running protoc now. |
9e989b9
to
86003a1
Compare
It looks like the Protobuf folks gave in and allowed a commit to fix this. They just cut a new release and the version is getting updated in HomeBrew Once that's done the original issue should be fixed, and I can abandon this. For reference, for whatever reason in CI if I used protoc built with Debug mode it would fail with and if I tried to build the Protobuf libraries in Debug mode and built protoc in Release mode, cmake wouldn't be able to find it. All of this worked on my personal Mac, so I'm still not sure what the problem was. |
Closing this out as homebrew was updated |
Protobuf recently made a change that made it so debug builds of Velox on Mac broke. See
#2029
This seems like a conscious tradeoff on the Protobuf side, building debug code linking agains release builds of Protobuf
was never supported, it just happened to work according to the comments here
protocolbuffers/protobuf#9947
To address this, I've changed setup-macos.sh to checkout the latest (as of writing) release of Protobuf and build/install
both optimized and debug versions of the library. cmake seems to be smart enough to figure out when to use which.
I also tried continuing to use brew to install the optimized version, but that seemed to break find_package(Protobuf 3.0.0
REQUIRED) it would only find the optimized version, not the debug version, even though I built and installed that
separately.
I suspect this works in Linux builds because we use the protobuf-devel package which I'm guessing includes the debug
build, but I didn't dig into it too much.
I verified
make debug
runs on my Mac now, I'm verifyingmake release
finishes successfully as well.