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

Update setup.py #594

Closed
wants to merge 1 commit into from
Closed

Update setup.py #594

wants to merge 1 commit into from

Conversation

gorloffslava
Copy link

This PR adds a new buildtime environment variable AWS_CRT_BUILD_USE_SHARED_LIBS.

When it's set to 1, it overrides the default behavior of trying to link static library variants on Unix OSs (except darwin ).

This default behavior is enforced by the following lines:

aws-crt-python/setup.py

Lines 340 to 348 in 2dae492

# linker will prefer shared libraries over static if it can find both.
# force linker to choose static variant by using using
# "-l:libaws-c-common.a" syntax instead of just "-laws-c-common".
#
# This helps AWS developers creating Lambda applications from Brazil.
# In Brazil, both shared and static libs are available.
# But Lambda requires all shared libs to be explicitly packaged up.
# So it's simpler to link them in statically and have less runtime dependencies.
libraries = [':lib{}.a'.format(x) for x in libraries]

Currently, there is no way to disable it and prefer shared libraries instead, what this PR solves.

As a continuation of the #593, it allows to reduce the libraries footprint in disk-restricted environments like Lambda, promote shared libraries reuse in RAM (e.g., if multiple Python packages depend on them), and generally make it more aligned with best native Python extensions build practices.

This PR is backward-compatible as feature it provides is opt-in, it doesn't change the default behavior of preferring static libraries.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@BwL1289
Copy link

BwL1289 commented Sep 10, 2024

@graebm as stated this is a continuation of #593. Thought it was easier to open a PR then submit a new issue 😄.

@graebm
Copy link
Contributor

graebm commented Sep 12, 2024

Thanks for the PR. I'm looking into this ... just busy

@BwL1289
Copy link

BwL1289 commented Sep 12, 2024

No problem. Apologies I missed this in the previous issue/PR.

@graebm
Copy link
Contributor

graebm commented Sep 12, 2024

Here's my take on it: #596

It's the opposite of your approach. I'll fix AWS's internal build system to use the new-extra-weird env-var. Instead of forcing slightly-less-weird users like yourself to set 2 env-vars.

@graebm graebm closed this Sep 12, 2024
@graebm
Copy link
Contributor

graebm commented Sep 16, 2024

Thoughts on #596? Does this work for you?

@BwL1289
Copy link

BwL1289 commented Sep 16, 2024

@graebm just seeing - will test and get back to you. Thank you!

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