protobuf: restore 3.17.1 + improve robustness of test_v1_package conanfile.py #15518

SpaceIm · 2023-01-28T22:38:35Z

restore 3.17.1: Address review in protobuf: Update to new CMake integrations #14851 (comment). Maintenance of this version has been removed during conan v2 migration, but it's still widely used in other CCI recipes (like OpenCV), which can't depend on newer versions than this one, therefore it's not a good thing to not have migrated this version to conan v2 helpers nor to drop its maintenance.
test_v1_pacage: Inspired from test_v1_package conanfile of capnproto. I've experimented a lot both 1 & 2 profiles while migrating capnproto.
The idea is to avoid a patch in Add protobuf 3.5.1.1 version #15202 due to this error in test v1 package: Add protobuf 3.5.1.1 version #15202 (comment), which comes from a lack of robustness in test v1 package.

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

ghost · 2023-01-28T23:12:40Z

I detected other pull requests that are modifying protobuf/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

github-actions · 2023-01-30T14:49:22Z

Hooks produced the following warnings for commit b0b9a7f

protobuf/3.19.6

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotocd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotoc.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobufd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf-lited.so' links to system library 'm' but it is not in cpp_info.system_libs.

protobuf/3.21.9

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotoc.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotocd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobufd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf-lited.so' links to system library 'm' but it is not in cpp_info.system_libs.

protobuf/3.17.1

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotoc.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotocd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobufd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf-lited.so' links to system library 'm' but it is not in cpp_info.system_libs.

protobuf/3.19.4

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotocd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotoc.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobufd.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprotobuf-lited.so' links to system library 'm' but it is not in cpp_info.system_libs.

jcar87 · 2023-01-30T15:38:25Z

(like OpenCV), which can't depend on newer versions than this one,

Is there an OpenCV issue or documentation that specifies the upper bound for the protobuf version?

SpaceIm · 2023-01-30T15:55:04Z

(like OpenCV), which can't depend on newer versions than this one,

Is there an OpenCV issue or documentation that specifies the upper bound for the protobuf version?

No, but in #15193 I've tried to bump protobuf and build failed with any version greater than 3.17.1

In OpenCV, they allow to build a vendored protobuf. It was 3.5.2 for a long time, and has been updated to 3.19.1 in OpenCV 4.5.5 but with some patches to keep compatibility I think: opencv/opencv#20998. It has not been backported to OpenCV 3.x branch anyway.

jcar87 · 2023-01-30T16:02:52Z

On which platform/compiler were the issues experienced? I can't locally reproduce this - I'm able to build OpenCV 4.x with the most recent protobuf.

SpaceIm · 2023-01-30T16:10:01Z

MacOS, all shared.

jcar87 · 2023-01-30T16:22:20Z

Thanks! I'm still unable to reproduce the issue on macOS. I'm building OpenCV 4.5.5 against Protobuf 3.21.4 with everything shared - is there a link to a build that failed on CI when trying to bump the version?

The vendored-in version of protobuf in OpenCV needs to be anchored to the exact same version as the vendored-in .pb.cc and .pb.h files. Since we pass PROTOBUF_UPDATE_FILES unconditionally (given that we provide Protobuf), those files are re-generated and I can see this working correctly as far as I can tell.

jcar87

waiting for feedback related to issues with versions newer than 3.17.1 -

SpaceIm · 2023-01-30T17:02:11Z

I've tried again because I've tested 2 or 3 weeks ago. Indeed it works fine in 4.5.5, but it fails in OpenCV 4.5.0 if protobuf is greater than 3.17.1 (and I guess for all OpenCV before 4.5.5):

conan create . opencv/4.5.0@ -o "*":shared=True -tf None

[276/561] Building CXX object modules/dnn/CMakeFiles/opencv_dnn.dir/src/caffe/caffe_io.cpp.o
FAILED: modules/dnn/CMakeFiles/opencv_dnn.dir/src/caffe/caffe_io.cpp.o
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DCVAPI_EXPORTS -DHAVE_PROTOBUF=1 -DOPENCV_DNN_EXTERNAL_PROTOBUF=1 -D_USE_MATH_DEFINES -D__OPENCV_BUILD=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/src/modules/dnn/include -I/Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/build/Release/modules/dnn -I/Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/src/modules/core/include -I/Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/src/modules/imgproc/include -isystem /Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/build/Release -isystem /Users/spaceim/.conan/data/protobuf/3.21.4/_/_/package/ebb41598f88669366da6070619c3f6d43d0d809f/include -isystem /Users/spaceim/.conan/data/eigen/3.3.9/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/eigen3 -isystem /Users/spaceim/.conan/data/zlib/1.2.13/_/_/package/8a659aeafbc64e1549799b2ee77375f43ddc5111/include -m64 -stdlib=libc++   -fsigned-char -W -Wall -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -Wformat -Werror=format-security -Wstrict-prototypes -Wundef -Winit-self -Wpointer-arith -Wsign-promo -Wuninitialized -Winit-self -Wno-delete-non-virtual-dtor -Wno-unnamed-type-template-args -Wno-comment -Wno-deprecated-enum-enum-conversion -Wno-deprecated-anon-enum-enum-conversion -fdiagnostics-show-option -Wno-long-long -Qunused-arguments -Wno-semicolon-before-method-body -ffunction-sections -fdata-sections  -fvisibility=hidden -fvisibility-inlines-hidden -Wno-deprecated -Wno-missing-prototypes -Wno-missing-declarations -Wno-shadow -Wno-unused-parameter -Wno-sign-compare -Wno-invalid-offsetof -O3 -DNDEBUG  -DNDEBUG -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk -fPIC -std=c++11 -MD -MT modules/dnn/CMakeFiles/opencv_dnn.dir/src/caffe/caffe_io.cpp.o -MF modules/dnn/CMakeFiles/opencv_dnn.dir/src/caffe/caffe_io.cpp.o.d -o modules/dnn/CMakeFiles/opencv_dnn.dir/src/caffe/caffe_io.cpp.o -c /Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/src/modules/dnn/src/caffe/caffe_io.cpp
/Users/spaceim/.conan/data/opencv/4.5.0/_/_/build/60edf68a86e5389620e0a0d6fa3a27323782a773/src/modules/dnn/src/caffe/caffe_io.cpp:1114:58: error: too many arguments to function call, expected single argument 'total_bytes_limit', have 2 arguments
    coded_input.SetTotalBytesLimit(kProtoReadBytesLimit, 536870912);
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                       ^~~~~~~~~
/Users/spaceim/.conan/data/protobuf/3.21.4/_/_/package/ebb41598f88669366da6070619c3f6d43d0d809f/include/google/protobuf/io/coded_stream.h:384:8: note: 'SetTotalBytesLimit' declared here
  void SetTotalBytesLimit(int total_bytes_limit);
       ^
1 error generated.

Anyway, I think it's a bad idea to drop maintenance of versions in PR doing conan v2 migrations unless there is a good reason, because these recipes will be totally broken soon, not just unmaintained.

jcar87 · 2023-01-30T17:08:32Z

Thanks! Let me investigate

jcar87 · 2023-01-30T17:34:34Z

This has been fixed upstream - this has also been backported to 3.x releases as well.
opencv/opencv@9cfa843

Given that this looks like a trivial patch (and also considering that they appear to have been using a very old deprecated function signature) - I would advise to fix this in the OpenCV recipe and bump the version rather than continue to maintain 3.17.1. I'm happy to do this unless there's already a PR open where this can be done.
I will bump the version elsewhere where the older version is referenced - it doesn't seem to be many places anyway.

I agree versions shouldn't be dropped that are still in use. The 3.17.1 recipe is still there and available and working in the configurations it was tested when it was last published - so I dont think users are experiencing any issues. Given how easy it seems to fix OpenCV, the sooner we fix that, the less need to re-add an old version.

SpaceIm · 2023-01-30T18:27:07Z

I can incorporate this patch in #15193

The 3.17.1 recipe is still there and available and working in the configurations it was tested when it was last published - so I dont think users are experiencing any issues

It's still there but not migrated to conan v2. Given backward & forward compatibility issues in profile against conan v1 & v2 recipes, and that conan v2 won't support legacy helpers, I don't think it's a good thing, and it will be worse once conan v2 released.

conan-center-bot · 2023-01-31T18:36:32Z

Conan v1 pipeline ✔️

All green in build 6 (31706e8251ab69d92b7c62f8cf938e89c80ea817):

protobuf/3.21.9@:
All packages built successfully! (All logs)
protobuf/3.21.4@:
All packages built successfully! (All logs)
protobuf/3.19.4@:
All packages built successfully! (All logs)
protobuf/3.20.0@:
All packages built successfully! (All logs)
protobuf/3.19.6@:
All packages built successfully! (All logs)
protobuf/3.18.1@:
All packages built successfully! (All logs)
protobuf/3.17.1@:
All packages built successfully! (All logs)

Conan v2 pipeline (informative, not required for merge) ✔️

Note: Conan v2 builds are informative and they are not required for the PR to be merged.

All green in build 6 (31706e8251ab69d92b7c62f8cf938e89c80ea817):

protobuf/3.21.4@:
All packages built successfully! (All logs)
protobuf/3.21.9@:
All packages built successfully! (All logs)
protobuf/3.20.0@:
All packages built successfully! (All logs)
protobuf/3.19.4@:
All packages built successfully! (All logs)
protobuf/3.19.6@:
All packages built successfully! (All logs)
protobuf/3.18.1@:
All packages built successfully! (All logs)
protobuf/3.17.1@:
All packages built successfully! (All logs)

SpaceIm · 2023-02-06T13:26:16Z

@jcar87 still any reason to block this PR given that protobuf 3.18 has removed deprecated functions, and 3.17.1 will not be usable in conan v2 client without this PR? I don't ask to maintain this version forever, just to have at least one RREV of this version with full conan v2 support.

issue clarified

jcar87 · 2023-02-06T13:58:35Z

@jcar87 still any reason to block this PR given that protobuf 3.18 has removed deprecated functions, and 3.17.1 will not be usable in conan v2 client without this PR? I don't ask to maintain this version forever, just to have at least one RREV of this version with full conan v2 support.

We currently don't offer hard guarantees that the recipes or packages work with Conan 2.0, so older versions of OpenCV not building with Conan 2.0 today due to a revision of Protobuf 3.17.1 not existing for Conan 2.0, can also easily be solved by applying the upstream patch from OpenCV. I appreciate the intention, but neither OpenCV nor Protobuf were Conan 2.0 compatible until very recently, so I don't consider this a regression. I believe that if the goal is to enable all versions of the OpenCV recipe to build with Conan 2.0, the version of the Protobuf dependency should be bumped, along with applying the upstream patch. I see no point in going backwards when the CI efforts of the alternative are roughly equivalent.

I have, however, dismissed my review as the issue has been clarified.

…t_v1_package conanfile.py * robust conanfile for test_v1_package * restore 3.17.1 * use correct 3.21.9 url * fix patch fields * fix removal of libprotobuf-lite * add libm to system libs * cleanup

robust conanfile for test_v1_package

ce7cd8e

This was referenced Jan 28, 2023

[service] Potentially conflicting recipes #3571

Closed

Add protoc python plugin support #15501

Closed

Add protobuf 3.5.1.1 version #15202

Closed

github-actions bot mentioned this pull request Jan 28, 2023

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

This comment has been minimized.

Sign in to view

prince-chrismc mentioned this pull request Jan 30, 2023

Rank effort to review a given PR prince-chrismc/conan-center-index-pending-review#109

Merged

1 task

SpaceIm added 2 commits January 30, 2023 09:20

restore 3.17.1

fb9554a

use correct 3.21.9 url

2998623

SpaceIm changed the title ~~protobuf: improve robustness of test_v1_package conanfile.py~~ protobuf: restore 3.17.1 + improve robustness of test_v1_package conanfile.py Jan 30, 2023

fix patch fields

b0b9a7f

jwillikers previously approved these changes Jan 30, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

SpaceIm added 3 commits January 30, 2023 16:07

fix removal of libprotobuf-lite

ac11b1d

add libm to system libs

f550456

cleanup

31706e8

SpaceIm dismissed jwillikers’s stale review via 31706e8 January 30, 2023 15:11

jcar87 previously requested changes Jan 30, 2023

View reviewed changes

conan-center-bot added the Failed label Jan 30, 2023

This comment has been minimized.

Sign in to view

SpaceIm closed this Jan 31, 2023

SpaceIm reopened this Jan 31, 2023

conan-center-bot removed the Failed label Jan 31, 2023

ghost mentioned this pull request Feb 5, 2023

protobuf: add version 3.21.12, use is_msvc_static_runtime #15738

Closed

3 tasks

jwillikers approved these changes Feb 6, 2023

View reviewed changes

Hopobcn approved these changes Feb 6, 2023

View reviewed changes

prince-chrismc approved these changes Feb 8, 2023

View reviewed changes

conan-center-bot merged commit fd3f249 into conan-io:master Feb 8, 2023

SpaceIm deleted the fix/protobuf-test-v1-package branch February 8, 2023 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

protobuf: restore 3.17.1 + improve robustness of test_v1_package conanfile.py #15518

protobuf: restore 3.17.1 + improve robustness of test_v1_package conanfile.py #15518

SpaceIm commented Jan 28, 2023 •

edited

Loading

ghost commented Jan 28, 2023 •

edited by ghost

Loading

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 30, 2023

jcar87 commented Jan 30, 2023

SpaceIm commented Jan 30, 2023 •

edited

Loading

jcar87 commented Jan 30, 2023

SpaceIm commented Jan 30, 2023

jcar87 commented Jan 30, 2023 •

edited

Loading

jcar87 left a comment

SpaceIm commented Jan 30, 2023 •

edited

Loading

jcar87 commented Jan 30, 2023

jcar87 commented Jan 30, 2023 •

edited

Loading

SpaceIm commented Jan 30, 2023

This comment has been minimized.

conan-center-bot commented Jan 31, 2023

SpaceIm commented Feb 6, 2023 •

edited

Loading

jcar87 commented Feb 6, 2023

protobuf: restore 3.17.1 + improve robustness of test_v1_package conanfile.py #15518

protobuf: restore 3.17.1 + improve robustness of test_v1_package conanfile.py #15518

Conversation

SpaceIm commented Jan 28, 2023 • edited Loading

ghost commented Jan 28, 2023 • edited by ghost Loading

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 30, 2023

jcar87 commented Jan 30, 2023

SpaceIm commented Jan 30, 2023 • edited Loading

jcar87 commented Jan 30, 2023

SpaceIm commented Jan 30, 2023

jcar87 commented Jan 30, 2023 • edited Loading

jcar87 left a comment

Choose a reason for hiding this comment

SpaceIm commented Jan 30, 2023 • edited Loading

jcar87 commented Jan 30, 2023

jcar87 commented Jan 30, 2023 • edited Loading

SpaceIm commented Jan 30, 2023

This comment has been minimized.

conan-center-bot commented Jan 31, 2023

Conan v1 pipeline ✔️

Conan v2 pipeline (informative, not required for merge) ✔️

SpaceIm commented Feb 6, 2023 • edited Loading

jcar87 commented Feb 6, 2023

SpaceIm commented Jan 28, 2023 •

edited

Loading

ghost commented Jan 28, 2023 •

edited by ghost

Loading

SpaceIm commented Jan 30, 2023 •

edited

Loading

jcar87 commented Jan 30, 2023 •

edited

Loading

SpaceIm commented Jan 30, 2023 •

edited

Loading

jcar87 commented Jan 30, 2023 •

edited

Loading

SpaceIm commented Feb 6, 2023 •

edited

Loading