Mizux
commented
1 month ago Custom Protobuf
cloning Protobuf locally and playing with SHA1/patch
git clone git@github.com:protocolbuffers/protobuf.git
cd protobuf
# reset to the commit to test
git reset --hard d21425d334f965650c3c66362b0d827797f059db
# Change -dev version suffix using v27.0 commit otherwise at runtime protobuf check version suffix and refuse to start
# we use tig to select the commit to cherry pick
tig v27.0
# inside tig press `C` to cherrypick, `Y` to confirm, then quit with `q`
# fix patch using mergetool
git mergetool
# remove .orig
git clean -n
git clean -f
# Patch CMake stuff
# note: could be protobuf-v27.patch
git apply --3way ~/work/main/patches/protobuf-v26.patchmodify or-tools
diff --git a/cmake/dependencies/CMakeLists.txt b/cmake/dependencies/CMakeLists.txt
index a5cfdeee13..fbd75fe544 100644
--- a/cmake/dependencies/CMakeLists.txt
+++ b/cmake/dependencies/CMakeLists.txt
@@ -106,13 +106,21 @@ if(BUILD_Protobuf)
#set(protobuf_BUILD_LIBUPB ON)
FetchContent_Declare(
Protobuf
- GIT_REPOSITORY "https://github.com/protocolbuffers/protobuf.git"
+ SOURCE_DIR "${CMAKE_SOURCE_DIR}/pb"
- GIT_SHALLOW TRUE
+ #GIT_SHALLOW TRUE
GIT_SUBMODULES ""
- PATCH_COMMAND git apply --ignore-whitespace "${CMAKE_CURRENT_LIST_DIR}/../../patches/protobuf-v27.patch")
+ #PATCH_COMMAND git apply --ignore-whitespace "${CMAKE_CURRENT_LIST_DIR}/../../patches/protobuf-v27.patch"
+ #PATCH_COMMAND git apply --ignore-whitespace "${CMAKE_CURRENT_LIST_DIR}/../../patches/protobuf-v26.patch"
+ )
FetchContent_MakeAvailable(Protobuf)
list(POP_BACK CMAKE_MESSAGE_INDENT)
message(CHECK_PASS "fetched")note: I've use a symbolic link in the root dir of my or-tools local repo.
ln -s <path_to_hacked_protobuf> pb
In or-tools:
# clean protobuf build
rm -rf build/_deps/protobuf-build build/_deps/protobuf-src build/_deps/protobuf-subbuild
# rebuild
cmake --build build -j 8
# test
source build/python/venv/bin/activate
python ortools/routing/python/model_test.pyDev Note
Rename the 'includingDefaultValueWithoutPresenceFields' and 'always_print_without_presence_fields' to 'alwaysPrintFieldsWithNoPresence':
You also, may need to revert this commit e0a4dcf5a082e7f90b73708fc7ff4a5e4760ed85 or apply this diff
diff --git a/ortools/util/file_util.cc b/ortools/util/file_util.cc
index db638ca8f9..fcb661fcf1 100644
--- a/ortools/util/file_util.cc
+++ b/ortools/util/file_util.cc
@@ -165,7 +165,7 @@ absl::Status WriteProtoToFile(absl::string_view filename,
case ProtoWriteFormat::kJson: {
google::protobuf::util::JsonPrintOptions options;
options.add_whitespace = true;
- options.always_print_fields_with_no_presence = true;
+ options.always_print_primitive_fields = true;
options.preserve_proto_field_names = true;
if (!google::protobuf::util::MessageToJsonString(proto, &output_string,
options)if you are before this commit:
Fun Fact: While release v26.1 contains this patch, it is NOT available in the v26-dev tag...
https://github.com/protocolbuffers/protobuf/blob/v26.1/src/google/protobuf/json/json.h#L46
https://github.com/protocolbuffers/protobuf/blob/v26-dev/src/google/protobuf/json/json.h#L46
See only commit between a and b[
To only list commit [a, b[ you can use:
tig a...bResults
Using a bisect approach to find the first borken commit(s)...
| Protobuf Version | CMake Patch | Test | Date |
|---|---|---|---|
v27.2 |
protobuf-v27.patch | FAIL | 2024 |
v27.1 |
protobuf-v27.patch | FAIL | 2024 |
v27.0 |
protobuf-v27.patch | FAIL | 2024/05/22 |
| ... | ... | ... | ... |
<v27-dev> |
protobuf-v27.patch | FAIL | 2024/04/17 |
| ... | ... | ... | ... |
01312f9c34a779f2a4e3327822b33d85f7a64002 |
protobuf-v26.patch | FAIL | 2024/02/06 |
| ... | ... | ... | ... |
4687ef335f29736fa0ff83d6421ea2bbdbdf1aa5 |
protobuf-v26.patch | FAIL | 2024/02/01 |
| ... | ... | ... | ... |
21ab7459ee43c0d5079e9f7708d453f68a5fd0e5 |
protobuf-v26.patch | FAIL | 2024/01/31 |
| ... | ... | ... | ... |
6f1d88107f268b8ebdad6690d116e74c403e366e |
protobuf-v26.patch | FAIL | 2024/01/30 |
| ... | ... | ... | ... |
4df6e042eca08159ee395853d8c250469a35db72 |
protobuf-v26.patch | FAIL | 2024/01/30 |
| ... | ... | ... | ... |
f4511fda5a0ad44e84848077da83f2ae9b21830e |
protobuf-v26.patch | FAIL | 2024/01/30 |
| ... | ... | ... | ... |
49c83ab799977592744f50df93957ddec1e12e70 |
protobuf-v26.patch | FAIL | 2024/01/30 |
abc9bae7c19d9fff4b0be0ffd3a01f947d3b487a |
protobuf-v26.patch | NA(0) | 2024/01/30 |
58baeb4c3b664f8918d24cef5151083d9da9767c |
protobuf-v26.patch | PASS | 2024/01/30 |
| ... | ... | ... | ... |
d21425d334f965650c3c66362b0d827797f059db |
protobuf-v26.patch | PASS | 2024/01/30 |
| ... | ... | ... | ... |
9146ce6ddba3b74ad16a0f440f9cf9d73ba282c7 |
protobuf-v26.patch | PASS | 2024/01/25 |
| ... | ... | ... | ... |
<v26-dev> |
protobuf-v26.patch | NA(1) |
- https://github.com/protocolbuffers/protobuf/commit/49c83ab799977592744f50df93957ddec1e12e70 contains auto generated code after the commit https://github.com/protocolbuffers/protobuf/commit/abc9bae7c19d9fff4b0be0ffd3a01f947d3b487a so I guess this one is not atomic and can't be tested...
- compile fail didn't manage to generate a binary, but v26-dev is not in the v26.1 which contains commit after this tag
magneticflux-
Tests impacted
here the list of impacted tests...
To test:
note: using Protobuf 58baeb4c3b664f8918d24cef5151083d9da9767c everything pass... Regression seem to have been introduced by http://cl/602725967
Investigation
Testing one broken test
Trace:
https://github.com/google/or-tools/blob/f9adb26d1df687ce2362e864ee713f2db00897bc/ortools/routing/python/model_test.py#L648-L651
continue to investigate...
ref: https://github.com/protocolbuffers/protobuf/blob/63def39e881afa496502d9c410f4ea948e59490d/src/google/protobuf/generated_message_reflection.cc#L3616-L3620