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

Allow using system aws c libraries #588

Closed
2 tasks done
BwL1289 opened this issue Aug 28, 2024 · 8 comments
Closed
2 tasks done

Allow using system aws c libraries #588

BwL1289 opened this issue Aug 28, 2024 · 8 comments
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@BwL1289
Copy link

BwL1289 commented Aug 28, 2024

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

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@BwL1289 BwL1289 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2024
@graebm
Copy link
Contributor

graebm commented Sep 3, 2024

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:

aws-crt-python/setup.py

Lines 274 to 278 in 5a1f499

if os.path.exists(os.path.join(PROJECT_DIR, 'crt', 'aws-c-common', 'CMakeLists.txt')):
self._build_dependencies(dep_build_dir, dep_install_path)
else:
print("Skip building dependencies, source not found.")

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
Copy link
Author

BwL1289 commented Sep 3, 2024

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
Copy link
Contributor

graebm commented Sep 6, 2024

PR is up. Look good?

@BwL1289
Copy link
Author

BwL1289 commented Sep 6, 2024

LGTM. Thank you @graebm.

graebm added a commit that referenced this issue Sep 6, 2024
*Issue #:* #588

User wants explicit way to choose whether aws-crt-python builds its own dependencies (their source code is under `crt/`), or find them prebuilt on the system.

There's currently a secret way to do this (delete `crt/` folder, or don't sync submodules), but it's not documented.

*Description of changes:*
Set `AWS_CRT_BUILD_USE_SYSTEM_LIBS=1` to explicitly disable building dependencies
@graebm graebm closed this as completed Sep 6, 2024
@graebm
Copy link
Contributor

graebm commented Sep 6, 2024

Do you need a new release?

@BwL1289
Copy link
Author

BwL1289 commented Sep 6, 2024

If possible that would be great.

@graebm
Copy link
Contributor

graebm commented Sep 7, 2024

v0.21.5 is available on PyPI and GitHub

@BwL1289
Copy link
Author

BwL1289 commented Sep 7, 2024

Thank you. Appreciate it @graebm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants