Skip to content
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

Rebuild for protobuf423 #137

Conversation

regro-cf-autotick-bot
Copy link
Contributor

@regro-cf-autotick-bot regro-cf-autotick-bot commented May 27, 2023

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 @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
Copy link
Contributor

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 h-vetinari marked this pull request as draft May 27, 2023 10:28
@h-vetinari
Copy link
Member

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

@h-vetinari h-vetinari force-pushed the rebuild-protobuf423-0-2_h61140d branch from 66ae1f3 to 24f5f00 Compare June 4, 2023 11:31
@h-vetinari
Copy link
Member

@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
Copy link
Contributor

coryan commented Jun 5, 2023

@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:

protocolbuffers/protobuf#12903

@coryan
Copy link
Contributor

coryan commented Jun 5, 2023

Filed googleapis/google-cloud-cpp#11779 to fix the cross-compilation problems in google-cloud-cpp.

@h-vetinari
Copy link
Member

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 googleapis/google-cloud-cpp#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
Copy link
Contributor

coryan commented Jun 6, 2023

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
Copy link
Member

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

@h-vetinari h-vetinari marked this pull request as ready for review June 7, 2023 00:12
@h-vetinari h-vetinari merged commit 21a8aa4 into conda-forge:main Jun 7, 2023
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the rebuild-protobuf423-0-2_h61140d branch June 7, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants