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

Missing includes #3002

Open
laramiel opened this issue Jun 15, 2024 · 5 comments · May be fixed by #3006
Open

Missing includes #3002

laramiel opened this issue Jun 15, 2024 · 5 comments · May be fixed by #3006
Labels
bug This issue is a bug. build-problem problems with building this sdk p3 This is a minor priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet.

Comments

@laramiel
Copy link

Describe the bug

I am working on bazel build rules for some aws sdk libraries; bazel is more strict about includes than cmake.
These files are missing includes, which causes errors.

  • aws-cpp-sdk-core/include/aws/core/NoResult.h
  • aws-cpp-sdk-core/include/aws/core/utils/stream/StreamBufProtectedWriter.h
  • aws-cpp-sdk-core/include/aws/core/monitoring/MonitoringFactory.h
  • aws-cpp-sdk-core/include/aws/core/Globals.h
  • aws-cpp-sdk-core/include/aws/core/utils/event/EventStreamEncoder.h

Expected Behavior

No errors.

Current Behavior

Errors on missing includes.

Reproduction Steps

Add BUILD.bazel rules and build.

Possible Solution

  • aws-cpp-sdk-core/include/aws/core/NoResult.h
#include <aws/core/AmazonWebServiceResult.h>
  • aws-cpp-sdk-core/include/aws/core/utils/stream/StreamBufProtectedWriter.h
#include <cstdint>
#include <aws/core/utils/memory/stl/AWSStreamFwd.h>
#include <aws/core/utils/logging/LogMacros.h>
  • aws-cpp-sdk-core/include/aws/core/monitoring/MonitoringFactory.h
include <aws/core/utils/memory/AWSMemory.h>
  • aws-cpp-sdk-core/include/aws/core/Globals.h
#include <memory>
  • aws-cpp-sdk-core/include/aws/core/utils/event/EventStreamEncoder.h
#include <aws/core/utils/memory/stl/AWSString.h>
#include <aws/core/utils/event/EventMessage.h>

Additional Information/Context

No response

AWS CPP SDK version used

67f1aae

Compiler and Version used

clang

Operating System and version

linux
@laramiel laramiel added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 15, 2024
@jmklix jmklix linked a pull request Jun 17, 2024 that will close this issue
Open
11 tasks
@jmklix
Copy link
Member

jmklix commented Jun 17, 2024

Hi @laramiel,

I made a PR with your suggested changes here. If this passes all of the tests then we can merge it. We currently don't have any integration tests for bazel and we don't have any plans to add them anytime soon. That being said if you run into any other missing includes please let us know.

@jmklix jmklix added p3 This is a minor priority issue build-problem problems with building this sdk pending-release This issue will be fixed by an approved PR that hasn't been released yet. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 17, 2024
@jmklix jmklix linked a pull request Jun 17, 2024 that will close this issue
fix includes for bazel #3006
Open
11 tasks
@jmklix
Copy link
Member

jmklix commented Jun 17, 2024

How are you trying to build this sdk within bazel? Can you share a minimal bazel build files that reproduce the missing includes?

@jmklix jmklix added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jun 17, 2024
@laramiel
Copy link
Author

laramiel commented Jun 21, 2024 •
edited
Loading

Sure, I'll try and get that available at some point. For now, this is a variant of a pull request that we received in tensorstore, but updated to use repositories at github head: google/tensorstore#149

My current attempt includes these repositories:

https://github.com/awslabs/aws-c-sdkutils
https://github.com/awslabs/aws-checksums
https://github.com/awslabs/aws-c-cal
https://github.com/awslabs/aws-c-compression
https://github.com/aws/s2n-tls
https://github.com/awslabs/aws-c-common
https://github.com/awslabs/aws-c-event-stream
https://github.com/awslabs/aws-c-http
https://github.com/awslabs/aws-c-s3
https://github.com/awslabs/aws-c-auth
https://github.com/awslabs/aws-c-mqtt
https://github.com/awslabs/aws-c-io
https://github.com/awslabs/aws-crt-cpp
https://github.com/aws/aws-sdk-cpp

Only a subset of the aws-sdk-cpp repositories are included in the build, and ,ost of the BAZEL.build files are pretty simple, something like:

cc_library(
    name = "aws_c_compression",
    srcs = glob(
        include = [
            "source/*.c",
        ],
        exclude = ["source/huffman_testing.c"],
    ),
    hdrs = glob([
        "include/aws/compression/*.h",
    ]),
    includes = ["include/"],
    deps = [ "@aws_c_common//:aws_c_common" ],
)

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jun 21, 2024
@sjperkins sjperkins mentioned this issue Jun 24, 2024
Draft
@jmklix
Copy link
Member

jmklix commented Sep 30, 2024

Did you have a minimal bazel build files yet?

@jmklix jmklix added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Sep 30, 2024
@laramiel
Copy link
Author

I have them for almost all dependencies of aws-sdk-cpp. See the BUILD.bazel files under the aws directories here: https://github.com/google/tensorstore/tree/master/third_party

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This issue is a bug. build-problem problems with building this sdk p3 This is a minor priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix includes for bazel aws/aws-sdk-cpp
2 participants
@jmklix @laramiel