awslabs / aws-crt-python

Python bindings for the AWS Common Runtime
Apache License 2.0
87 stars 43 forks source link

Allow using system aws c libraries #588

Closed BwL1289 closed 3 weeks ago

BwL1289 commented 1 month ago

Describe the feature

Right now you can only configure the build to use system libcrypto via an environment variable AWS_CRT_BUILD_USE_SYSTEM_LIBCRYPTO.

This should be expanded to support all libaws-c-* libraries, including libs2n.

For now I am patching this library but would like to see this officially supported.

Use Case

Using vendored libs when you're compiling & using those libraries yourself in other places may lead to strange and hard to debug bugs due to binary incompatibilities as not only API and ABI have to be the same in such cases and objects must be fully identical in RAM which includes hidden fields (which are affected by defines).

It's also a setuptools best practice.

Proposed Solution

Control building system vs vendored libraries via an environment variable.

Other Information

No response

Acknowledgements

graebm commented 4 weeks ago

It's not documented, but this is already possible. If setup.py can't find the source code for the other C libraries under crt/*, it will assume those libraries were already compiled some other way. The logic is here: https://github.com/awslabs/aws-crt-python/blob/5a1f499911786a759e8ba4f466f70e8d6be92a0d/setup.py#L274-L278

So, if you're getting the source code from github, don't download submodules. Then the crt/ folder will be empty.

If you're getting the source code from pypi, delete everything under crt/*.

Does that do the trick for you? Or would you still like an environment variable?

BwL1289 commented 4 weeks ago

Hi @graebm thanks for the response. Yes, that is essentially what we are doing now. I think it would make it easier if there were an environment variable to toggle this instead of building from source and using the trick you mentioned.

Is that possible for the team to prioritize?

Please let me know how I might be able to help.

graebm commented 3 weeks ago

PR is up. Look good?

BwL1289 commented 3 weeks ago

LGTM. Thank you @graebm.

graebm commented 3 weeks ago

Do you need a new release?

BwL1289 commented 3 weeks ago

If possible that would be great.

graebm commented 3 weeks ago

v0.21.5 is available on PyPI and GitHub

BwL1289 commented 3 weeks ago

Thank you. Appreciate it @graebm.