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.

Sign up for GitHub

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

Jump to bottom

Enable google/cloud/asset #8145

Closed
coryan opened this issue Jan 28, 2022 · 5 comments
Closed

Enable google/cloud/asset #8145

coryan opened this issue Jan 28, 2022 · 5 comments
Labels
api: cloudasset Issues related to the Cloud Asset Inventory API. external This issue is blocked on a bug with the actual product. type: cleanup An internal cleanup or hygiene concern.

Comments

@coryan
Copy link
Contributor

coryan commented Jan 28, 2022

We cannot compile google/cloud/asset with protobuf <= 3.19.3 (current as-of 2022-01-28) because the protos define TRUE and FALSE as enums, while they are macros on macOS:

In file included from bazel-out/darwin-fastbuild/bin/external/com_google_googleapis/google/cloud/asset/v1/asset_service.pb.h:40:
bazel-out/darwin-fastbuild/bin/external/com_google_googleapis/google/cloud/asset/v1/assets.pb.h:4015:36: error: expected member name or ';' after declaration specifiers
  static constexpr EvaluationValue TRUE =
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Applications/Xcode_12.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/mach/boolean.h:81:17: note: expanded from macro 'TRUE'
#define TRUE    1
                ^
@coryan coryan added type: cleanup An internal cleanup or hygiene concern. api: cloudasset Issues related to the Cloud Asset Inventory API. labels Jan 28, 2022
@coryan coryan added the external This issue is blocked on a bug with the actual product. label Jan 31, 2022
@coryan coryan removed the external This issue is blocked on a bug with the actual product. label Jul 7, 2022
@coryan
Copy link
Contributor Author

coryan commented Jul 7, 2022

This is now unblocked.

@dbolduc dbolduc mentioned this issue Jul 13, 2022
Merged
@dbolduc dbolduc changed the title Enable google/cloud/asset on macOS Enable google/cloud/asset Jul 18, 2022
@dbolduc
Copy link
Member

dbolduc commented Jul 18, 2022

Windows bazel builds are still blocked. E.g. https://source.cloud.google.com/results/invocations/90512c0b-7383-4221-a311-b49200c0e635

They fail on the TRUE, FALSE definitions. port_def.inc inside google3 handles these (internal link), but the open source doesn't (link). I did not look into why these are out of sync, despite the internal changes being made in 2021-04

@coryan coryan added the external This issue is blocked on a bug with the actual product. label Nov 16, 2022
@h-vetinari h-vetinari mentioned this issue Feb 6, 2023
Merged
4 tasks
@copybara-service copybara-service bot mentioned this issue Feb 7, 2023
Closed
copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this issue Feb 7, 2023
@protobuf-github-bot @copybara-github
Workaround TRUE and FALSE macro definitions on WIN32
582e1a4 
One of the [GCP proto files](https://github.com/googleapis/googleapis/blob/master/google/cloud/asset/v1/assets.proto) defines `TRUE` and `FALSE` as enum values ([permalink](https://github.com/googleapis/googleapis/blob/a4f2de456480c0a4ed9feeeaa1f8ee620bbef23a/google/cloud/asset/v1/assets.proto#L782-L786)).  These cause trouble on WIN32, where `TRUE` and `FALSE` are also macro definitions.

For more details, see googleapis/google-cloud-cpp#8145.

PiperOrigin-RevId: 507602457
copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this issue Feb 7, 2023
@protobuf-github-bot @copybara-github
Workaround TRUE and FALSE macro definitions on WIN32
e349ef9 
One of the [GCP proto files](https://github.com/googleapis/googleapis/blob/master/google/cloud/asset/v1/assets.proto) defines `TRUE` and `FALSE` as enum values ([permalink](https://github.com/googleapis/googleapis/blob/a4f2de456480c0a4ed9feeeaa1f8ee620bbef23a/google/cloud/asset/v1/assets.proto#L782-L786)).  These cause trouble on WIN32, where `TRUE` and `FALSE` are also macro definitions.

For more details, see googleapis/google-cloud-cpp#8145.

PiperOrigin-RevId: 507831036
alielbashir pushed a commit to alielbashir/protobuf that referenced this issue Feb 14, 2023
@protobuf-github-bot @alielbashir
Workaround TRUE and FALSE macro definitions on WIN32
a804b96 
One of the [GCP proto files](https://github.com/googleapis/googleapis/blob/master/google/cloud/asset/v1/assets.proto) defines `TRUE` and `FALSE` as enum values ([permalink](https://github.com/googleapis/googleapis/blob/a4f2de456480c0a4ed9feeeaa1f8ee620bbef23a/google/cloud/asset/v1/assets.proto#L782-L786)).  These cause trouble on WIN32, where `TRUE` and `FALSE` are also macro definitions.

For more details, see googleapis/google-cloud-cpp#8145.

PiperOrigin-RevId: 507831036
@coryan
Copy link
Contributor Author

coryan commented Apr 19, 2023

Need to wait until Protobuf v23 and the corresponding changes in gRPC.

This was referenced May 24, 2023
Merged
Closed
@coryan
Copy link
Contributor Author

coryan commented May 25, 2023

Only cleaning up some CI builds remains. We need to wait until a release of google-cloud-cpp, and a release of vcpkg that includes Protobuf 23 and our library.

@coryan
Copy link
Contributor Author

coryan commented Nov 29, 2023

This is now fixed.

@coryan coryan closed this as completed Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api: cloudasset Issues related to the Cloud Asset Inventory API. external This issue is blocked on a bug with the actual product. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@coryan @dbolduc