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

aws-c-s3: add missing interface definition if shared + modernize more for conan v2 #17117

Merged
merged 7 commits into from
Aug 24, 2023

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Apr 19, 2023

see https://github.com/awslabs/aws-c-s3/blob/v0.2.8/include/aws/s3/exports.h


@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SpaceIm SpaceIm mentioned this pull request Apr 19, 2023
13 tasks
@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Apr 20, 2023

waiting for #17104, #17105, #17107, #17108 & #17119

@conan-center-bot

This comment has been minimized.

@CLAassistant
Copy link

CLAassistant commented May 18, 2023

CLA assistant check
All committers have signed the CLA.

@ghost
Copy link

ghost commented Jul 7, 2023

I detected other pull requests that are modifying aws-c-s3/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@mpimenov
Copy link
Contributor

mpimenov commented Aug 4, 2023

@jcar87 Please take a look

@SpaceIm SpaceIm closed this Aug 10, 2023
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 10, 2023

In aws-c-s3/0.1.29, if apple-clang with conan v2:

[100%] Linking C shared library libaws-c-s3.dylib
Undefined symbols for architecture x86_64:
  "_aws_md5_compute", referenced from:
      _aws_s3_message_util_add_content_md5_header in s3_request_messages.c.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I've not tested locally, but shared build of 0.1.49 works fine for me.

@SpaceIm SpaceIm reopened this Aug 10, 2023
@gjasny
Copy link
Contributor

gjasny commented Aug 14, 2023

The aws-c-s3 package directly uses functions provided by aws-c-cal and therefore should explicitly create a dependency on it. With the following change both: 0.1.29 and 0.1.49 build on macOS with shared library support:

diff --git a/recipes/aws-c-s3/all/conanfile.py b/recipes/aws-c-s3/all/conanfile.py
index b63b926fa..f63d8e59f 100644
--- a/recipes/aws-c-s3/all/conanfile.py
+++ b/recipes/aws-c-s3/all/conanfile.py
@@ -43,10 +43,12 @@ class AwsCS3(ConanFile):
         self.requires("aws-c-common/0.8.2", transitive_headers=True, transitive_libs=True)
         if Version(self.version) < "0.1.49":
             self.requires("aws-c-auth/0.6.11", transitive_headers=True)
+            self.requires("aws-c-cal/0.5.13")
             self.requires("aws-c-http/0.6.13")
             self.requires("aws-c-io/0.10.20", transitive_headers=True)
         else:
             self.requires("aws-c-auth/0.6.17", transitive_headers=True)
+            self.requires("aws-c-cal/0.5.13")
             self.requires("aws-c-http/0.6.22")
             self.requires("aws-c-io/0.13.4", transitive_headers=True)
         if Version(self.version) >= "0.1.36":

@jcar87
Copy link
Contributor

jcar87 commented Aug 17, 2023

Relaunched CI jobs for this PR, will check later to complete the review.

@conan-center-bot

This comment has been minimized.

jcar87
jcar87 previously approved these changes Aug 23, 2023
@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 23, 2023

There is no dependency to asw-c-cal before 0.1.20 (there is still 0.1.19 in conandata): awslabs/aws-c-s3#48

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 7 (5221bee9bec22114e3a5a9dcef148b328c8f4c50):

  • aws-c-s3/0.1.29:
    All packages built successfully! (All logs)

  • aws-c-s3/0.1.27:
    All packages built successfully! (All logs)

  • aws-c-s3/0.1.37:
    All packages built successfully! (All logs)

  • aws-c-s3/0.1.49:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds may be required once they are on the v2 ready list

All green in build 7 (5221bee9bec22114e3a5a9dcef148b328c8f4c50):

  • aws-c-s3/0.1.49:
    All packages built successfully! (All logs)

  • aws-c-s3/0.1.37:
    All packages built successfully! (All logs)

  • aws-c-s3/0.1.29:
    All packages built successfully! (All logs)

  • aws-c-s3/0.1.27:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 6445bc6 into conan-io:master Aug 24, 2023
@SpaceIm SpaceIm deleted the aws-c-s3-dllimport branch August 24, 2023 23:07
ericLemanissier pushed a commit to ericLemanissier/conan-center-index that referenced this pull request Sep 15, 2023
… + modernize more for conan v2

* add AWS_S3_USE_IMPORT_EXPORT interface definition if shared

* modernize more

* more elegant way to define target for legacy generators

* aws-c-common, aws-c-auth & aws-c-io are public dependencies

* aws-c-s3: add direct dependency on aws-c-cal

* aws-c-s3: fix requirement on aws-c-cal

* aws-c-s3: drop older version

---------

Co-authored-by: Luis Caro Campos <3535649+jcar87@users.noreply.github.com>
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.

7 participants