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 bazel to build aws-sdk-cpp #2334

Merged
merged 18 commits into from
Feb 27, 2024
Merged

Use bazel to build aws-sdk-cpp #2334

merged 18 commits into from
Feb 27, 2024

Conversation

atobiszei
Copy link
Collaborator

@atobiszei atobiszei commented Feb 21, 2024

Additionally bump aws-sdk-cpp version to 1.11.279

Encountered issues with bazel. Non ASCII filenames don't work inside bazel. Workaround is to remove test file from aws-sdk-cpp.
bazelbuild/bazel#374

Building with rules_foreign cmake doesn't work with parallel builds. This is something we could optimize later, for now hardcode some low value:
bazel-contrib/rules_foreign_cc#329

On Redhat to properly build targets we now have to add:
--//:distro=redhat
By default distro is set to ubuntu. This is workaround for not bazel not being able to differentiate between the two. There is difference in aws-sdk-cpp static libraries location after build. Ideally we should find how to use select on aws-sdk-cpp repo BUILD file which would rely on main repo flag, but I didn't find way to support it there.

@atobiszei atobiszei added the WIP Do not merge until resolved label Feb 21, 2024
@atobiszei atobiszei removed the WIP Do not merge until resolved label Feb 26, 2024
@atobiszei atobiszei added this to the 2024.0 milestone Feb 26, 2024
Copy link
Collaborator

@rasapala rasapala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Make sure it works in mediapipe fork.

tf.patch Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is leftover copy of file already present in external/.

src/BUILD Outdated
constraint_value(constraint_setting = "linux_distribution_family", name = "debian") # like Ubuntu

create_config_settings()
#create_config_settings()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?

Comment on lines +864 to +867
"@aws-sdk-cpp//:aws-sdk-cpp_ubuntu",
],
"//:redhat_build": [
"@aws-sdk-cpp//:aws-sdk-cpp_redhat",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about using dashes in whole target name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it actually distinguish target from some separate gimicks for ubuntu/redhat.

src/BUILD Outdated
@@ -878,6 +877,9 @@ cc_library( # make ovms_lib dependent, use share doptions
local_defines = COMMON_LOCAL_DEFINES,
copts = COPTS_ADJUSTED,
linkopts = LINKOPTS_ADJUSTED,
# Check if needed TODO
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to do something here?

recursive_init_submodules = True,
# https://github.com/bazelbuild/bazel/issues/374
# issues with ASCI handling of file_test.c *xample file.txt in bazel
patch_cmds = ["find . -name '*xample.txt' -delete"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why *xample ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because bazel cannot handle non ascii names:
aws-sdk-cpp/crt/aws-crt-cpp/crt/aws-c-common/tests/resources/Å Éxample.txt
I would have the full name here but I cannot ;)

@atobiszei
Copy link
Collaborator Author

atobiszei commented Feb 27, 2024

LGTM. Make sure it works in mediapipe fork.

openvinotoolkit/mediapipe#65
Builds fine

@atobiszei atobiszei merged commit 7bf5d7f into main Feb 27, 2024
@atobiszei atobiszei deleted the atobisze_bump_cloud_aws branch February 27, 2024 15:26
@atobiszei atobiszei restored the atobisze_bump_cloud_aws branch February 27, 2024 15:26
psakamoori pushed a commit to psakamoori/model_server that referenced this pull request Apr 2, 2024
Additionally bump aws-sdk-cpp version to 1.11.279

Encountered issues with bazel. Non ASCII filenames don't work inside bazel. Workaround is to remove test file from aws-sdk-cpp.
bazelbuild/bazel#374

Building with rules_foreign cmake doesn't work with parallel builds. This is something we could optimize later, for now hardcode some low value:
bazel-contrib/rules_foreign_cc#329

On Redhat to properly build targets we now have to add:
--//:distro=redhat
By default distro is set to ubuntu. This is workaround for not bazel not being able to differentiate between the two. There is difference in aws-sdk-cpp static libraries location after build. Ideally we should find how to use select on aws-sdk-cpp repo BUILD file which would rely on main repo flag, but I didn't find way to support it there.

JIRA:CVS-130367
@atobiszei atobiszei deleted the atobisze_bump_cloud_aws branch April 24, 2024 15:20
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