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

Use quotes instead of angles for an include in compute/global_operations #12854

Closed
lx3-g opened this issue Oct 9, 2023 · 4 comments · Fixed by #12864
Closed

Use quotes instead of angles for an include in compute/global_operations #12854

lx3-g opened this issue Oct 9, 2023 · 4 comments · Fixed by #12864
Assignees
Labels
api: compute Issues related to the Compute Engine API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@lx3-g
Copy link

lx3-g commented Oct 9, 2023

What component of google-cloud-cpp is this related to?
google/cloud/compute/global_operations

Describe the bug
This autogenerated line

#include <google/cloud/compute/global_operations/v1/global_operations.pb.h>

should be quoted.

To Reproduce Steps to reproduce the behavior:

cc_binary(
name = "gcp_bug",
srcs = ["gcp_bug.cc"],
deps = [
"@com_github_googleapis_google_cloud_cpp//:compute_instances",
],
)

bazel build : gcp_bug
results in:

external/com_github_googleapis_google_cloud_cpp_2/google/cloud/compute/global_operations/v1/global_operations_connection_idempotency_policy.h:24:10: error: 'google/cloud/compute/global_operations/v1/global_operations.pb.h' file not found with <angled> include; use "quotes" instead
#include <google/cloud/compute/global_operations/v1/global_operations.pb.h>

Expected behavior
It should build.

Operating system:
PRETTY_NAME="Debian GNU/Linux rodete"

What compiler and version are you using?

Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-linux-gnu/13/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 13.2.0-4' --with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-13 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/libexec --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-13-oyarai/gcc-13-13.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-13-oyarai/gcc-13-13.2.0/debian/tmp-gcn/usr --enable-offload-defaulted --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu --with-build-config=bootstrap-lto-lean --enable-link-serialization=28
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.0 (Debian 13.2.0-4)

What version of google-cloud-cpp are you using?

https://github.com/googleapis/google-cloud-cpp/archive/v2.16.0.tar.gz

Additional context

I see that this might be intentional.
#3598
If so, can you suggest changes on my side to fix this.

@lx3-g lx3-g added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Oct 9, 2023
@coryan coryan added the api: compute Issues related to the Compute Engine API. label Oct 9, 2023
@coryan
Copy link
Contributor

coryan commented Oct 9, 2023

I see that this might be intentional.
#3598
If so, can you suggest changes on my side to fix this.

Yes. But maybe we can change our intentions. Some background: with MSVC, the code generated by Protobuf has more warnings that one cares to deal with. We wanted to see warnings in our code, and suppress warnings for code in Protobuf, or gRPC, or any "external" projects. We use these:

# Disable warnings on "external" headers. These are non-actionable, or at
# least not urgent, as they are in generated code or code we do not control.
# This is the motivation to include things outside the project with angle
# brackets. There is no other succinct way to tell MSVC what is "external".
'--per_file_copt=^//google/cloud@-experimental:external'
'--per_file_copt=^//google/cloud@-external:W0'
'--per_file_copt=^//google/cloud@-external:anglebrackets'

You may be able to enable angle brackets in your code with something like this:

https://github.com/googleapis/google-cloud-cpp/blob/main/bazel/googleapis.BUILD

Which you would enable in your workspace file (or workspace function):

build_file = Label("//bazel:googleapis.BUILD"),

Maybe we should add the includes directive to our Bazel files (at least for the proto libraries) so folks don't need to add the directive themselves. Let me give that a try.

/FYI: @scotthart

@coryan
Copy link
Contributor

coryan commented Oct 10, 2023

I am not clever enough to use includes in our Bazel files. All my attempts end-up with Bazel telling me off.

Changing all the code to include with quotes is "easy". But requires changes to the code generator, and then changes to the CMake build scripts, and then changes to the CI scripts.

@lx3-g
Copy link
Author

lx3-g commented Oct 10, 2023

Thnx @coryan for getting back to me! The idea you proposed does not seem to work, as we have confirmed offline.
It does seem that the switch to using quotes would be beneficial for the adoption of this SDK, even though it does require a few steps on your side.

@coryan
Copy link
Contributor

coryan commented Oct 11, 2023

Thanks for the detailed bug report. The fixes are available from the main branch on GitHub. If you prefer to use a release, the next release should happen around the first week of 2023-11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: compute Issues related to the Compute Engine API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants