conda-forge / google-cloud-cpp-feedstock

A conda-smithy repository for google-cloud-cpp.
BSD 3-Clause "New" or "Revised" License
2 stars 10 forks source link

Rebuild for protobuf423 #137

Closed regro-cf-autotick-bot closed 1 year ago

regro-cf-autotick-bot commented 1 year ago

This PR has been triggered in an effort to update protobuf423.

Notes and instructions for merging this PR:

  1. Please merge the PR only after the tests have passed.
  2. Feel free to push to the bot's branch to update this PR if needed.

Please note that if you close this PR we presume that the feedstock has been rebuilt, so if you are going to perform the rebuild yourself don't close this PR until the your rebuild has been merged.

Closes #136

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase code>@<space/conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/5097913761, please use this URL for debugging.

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

h-vetinari commented 1 year ago

This one pins tighter than #136, but will need to wait for the same reasons as that PR

h-vetinari commented 1 year ago

@coryan @dbolduc It seems that google-cloud-cpp is not respecting -DProtobuf_PROTOC_EXECUTABLE=$BUILD_PREFIX/bin/protoc somewhere (note how the following tries to call $PREFIX/bin/protoc-23.2.0 from the host rather than the build environment, and then unsurprisingly fails):

[11/6324] Running C++ protocol buffer compiler on $SRC_DIR/build_cmake/external/googleapis/src/googleapis_download/google/api/backend.proto
FAILED: external/googleapis/google/api/backend.pb.cc external/googleapis/google/api/backend.pb.h $SRC_DIR/build_cmake/external/googleapis/google/api/backend.pb.cc $SRC_DIR/build_cmake/external/googleapis/google/api/backend.pb.h 
cd $SRC_DIR/build_cmake/external/googleapis && $PREFIX/bin/protoc-23.2.0 --cpp_out $SRC_DIR/build_cmake/external/googleapis --proto_path $SRC_DIR/build_cmake/external/googleapis/src/googleapis_download --proto_path $PREFIX/include $SRC_DIR/build_cmake/external/googleapis/src/googleapis_download/google/api/backend.proto
/bin/sh: $PREFIX/bin/protoc-23.2.0: Bad CPU type in executable

On linux we get something else (probably unrelated to the above as it also appears in native compilation for x64):

[2290/6324] Building CXX object google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o
FAILED: google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o 
$BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DPROTOBUF_USE_DLLS -Dgoogle_cloud_cpp_channel_protos_EXPORTS -isystem $SRC_DIR/build_cmake/google/cloud/channel -isystem $SRC_DIR/build_cmake/external/googleapis -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/google-cloud-cpp-split-2.11.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -O3 -DNDEBUG -fPIC -MD -MT google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o -MF google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o.d -o google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o -c $SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/customers.pb.cc
In file included from $BUILD_PREFIX/x86_64-conda-linux-gnu/include/c++/12.2.0/cmath:45,
                 from $PREFIX/include/absl/time/time.h:78,
                 from $PREFIX/include/absl/log/log_entry.h:35,
                 from $PREFIX/include/absl/log/internal/log_message.h:41,
                 from $PREFIX/include/absl/log/internal/strip.h:24,
                 from $PREFIX/include/absl/log/internal/check_op.h:37,
                 from $PREFIX/include/absl/log/internal/check_impl.h:19,
                 from $PREFIX/include/absl/log/absl_check.h:38,
                 from $PREFIX/include/google/protobuf/io/coded_stream.h:132,
                 from $SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/customers.pb.h:24,
                 from $SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/customers.pb.cc:4:
$SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/common.pb.h:564:33: error: expected unqualified-id before numeric constant
  564 |   static constexpr CustomerType DOMAIN = CloudIdentityInfo_CustomerType_DOMAIN;
      |                                 ^~~~~~
coryan commented 1 year ago

@coryan @dbolduc It seems that google-cloud-cpp is not respecting -DProtobuf_PROTOC_EXECUTABLE=$BUILD_PREFIX/bin/protoc somewhere (note how the following tries to call $PREFIX/bin/protoc-23.2.0 from the host rather than the build environment, and then unsurprisingly fails):

[11/6324] Running C++ protocol buffer compiler on $SRC_DIR/build_cmake/external/googleapis/src/googleapis_download/google/api/backend.proto
FAILED: external/googleapis/google/api/backend.pb.cc external/googleapis/google/api/backend.pb.h $SRC_DIR/build_cmake/external/googleapis/google/api/backend.pb.cc $SRC_DIR/build_cmake/external/googleapis/google/api/backend.pb.h 
cd $SRC_DIR/build_cmake/external/googleapis && $PREFIX/bin/protoc-23.2.0 --cpp_out $SRC_DIR/build_cmake/external/googleapis --proto_path $SRC_DIR/build_cmake/external/googleapis/src/googleapis_download --proto_path $PREFIX/include $SRC_DIR/build_cmake/external/googleapis/src/googleapis_download/google/api/backend.proto
/bin/sh: $PREFIX/bin/protoc-23.2.0: Bad CPU type in executable

That seems like a bug upstream. We never cross-compile and have hard-coded protoc to the wrong thing. We could patch this by replacing $<TARGET_FILE:protobuf::protoc> with ${Protobuf_PROTOC_EXECUTABLE}. And I suspect something similar needs to happen with $<TARGET_FILE:gRPC::grpc_cpp_plugin> though I am not sure what the right variable would be in that case.

The patch upstream may need to be more complicated because google-cloud-cpp still supports older versions of Protobuf and supports compiling with the Protobuf module.

On linux we get something else (probably unrelated to the above as it also appears in native compilation for x64):

[2290/6324] Building CXX object google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o
FAILED: google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o 
$BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DPROTOBUF_USE_DLLS -Dgoogle_cloud_cpp_channel_protos_EXPORTS -isystem $SRC_DIR/build_cmake/google/cloud/channel -isystem $SRC_DIR/build_cmake/external/googleapis -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/google-cloud-cpp-split-2.11.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -O3 -DNDEBUG -fPIC -MD -MT google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o -MF google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o.d -o google/cloud/channel/CMakeFiles/google_cloud_cpp_channel_protos.dir/google/cloud/channel/v1/customers.pb.cc.o -c $SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/customers.pb.cc
In file included from $BUILD_PREFIX/x86_64-conda-linux-gnu/include/c++/12.2.0/cmath:45,
                 from $PREFIX/include/absl/time/time.h:78,
                 from $PREFIX/include/absl/log/log_entry.h:35,
                 from $PREFIX/include/absl/log/internal/log_message.h:41,
                 from $PREFIX/include/absl/log/internal/strip.h:24,
                 from $PREFIX/include/absl/log/internal/check_op.h:37,
                 from $PREFIX/include/absl/log/internal/check_impl.h:19,
                 from $PREFIX/include/absl/log/absl_check.h:38,
                 from $PREFIX/include/google/protobuf/io/coded_stream.h:132,
                 from $SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/customers.pb.h:24,
                 from $SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/customers.pb.cc:4:
$SRC_DIR/build_cmake/google/cloud/channel/google/cloud/channel/v1/common.pb.h:564:33: error: expected unqualified-id before numeric constant
  564 |   static constexpr CustomerType DOMAIN = CloudIdentityInfo_CustomerType_DOMAIN;
      |                                 ^~~~~~

Evidently we still need to skip that library. I thought this would have prevented the problem:

https://github.com/googleapis/google-cloud-cpp/blob/0c7f077b96f802c0e10f500a3e601b2663bcf353/google/cloud/channel/CMakeLists.txt#L18-L22

But evidently it does not work. There is a fix upstream in Protobuf, but it did not make it to the 23.1 release:

https://github.com/protocolbuffers/protobuf/pull/12903

coryan commented 1 year ago

Filed https://github.com/googleapis/google-cloud-cpp/issues/11779 to fix the cross-compilation problems in google-cloud-cpp.

h-vetinari commented 1 year ago

The patch upstream may need to be more complicated because google-cloud-cpp still supports older versions of Protobuf and supports compiling with the Protobuf module.

While I think the variable Protobuf_PROTOC_EXECUTABLE has a long-standing history, it would be completely fine from my POV to carry a patch here that does the replacement unconditionally, because for our infra that will work.

That said, you've been exemplary in upstreaming changes that are necessary for the feedstock, and I'm very grateful you've already tackled this in https://github.com/googleapis/google-cloud-cpp/pull/11782 - thanks a lot!

But evidently it does not work. There is a fix upstream in Protobuf, but it did not make it to the 23.1 release:

protocolbuffers/protobuf#12903

Didn't make it into 23.2 either. Perhaps you need to ask for it to be backported? In the meantime, that means we'd just have to skip google/cloud/channel here at the moment (whereas the skipped-on-21.x asset and storagetransfer should now be fine)?

coryan commented 1 year ago

That said, you've been exemplary in upstreaming changes that are necessary for the feedstock, and I'm very grateful you've already tackled this in googleapis/google-cloud-cpp#11782 - thanks a lot!

Thanks for saying that. Part of my job is to make these libraries easy to package. We cannot create or maintain packages for all the different packaging systems out there, but we can at least make the work easy (or easier) for the package maintainers.

But evidently it does not work. There is a fix upstream in Protobuf, but it did not make it to the 23.1 release: protocolbuffers/protobuf#12903

Didn't make it into 23.2 either.

Yes. Sorry. I lost track of what "Protobuf latest" was.

Perhaps you need to ask for it to be backported?

I see you did that, thanks.

In the meantime, that means we'd just have to skip google/cloud/channel here at the moment

Yes. Looks that way.

(whereas the skipped-on-21.x asset and storagetransfer should now be fine)?

Yes, I believe those are fine now. We have builds on macOS and Windows to test that.

h-vetinari commented 1 year ago

So with https://github.com/protocolbuffers/protobuf/pull/12987 backported to libprotobuf, and https://github.com/googleapis/google-cloud-cpp/pull/11782 backported in this PR, things are looking pretty good here (and I got to remove an ugly sed-hack, yay! 🥳)