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

366: fix fatal_warnings linker arg typo #392

Closed

Conversation

timblaktu
Copy link

@timblaktu timblaktu commented Sep 3, 2022

Issue #, if available:
#366

Description of changes:
Fixed typo, see issue.

I've confirmed that i can successfully pip install git+https://github.com/timblaktu/aws-crt-python.git@bugfix366-fatal_warnings-linker-typo.

Also I was able to get the latest awscli tag 2.7.29 working by modifying awscli's setup.py to use a test branch on my fork (identical to PR src branch except changed version in init.py):

sed -i 's/awscrt.*$/''awscrt @ git+ssh:\/\/[email protected]\/timblaktu\/aws-crt-python@366-localtest''/g' setup.cfg 

Once this PR is merged, we can remove this hack..

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

@TwistedTwigleg
Copy link
Contributor

Thank you for making this PR!

I just tested this on my Mac, and it does not appear to work as is. I get the following error when I use the change in this PR:

      ld: file not found:  -fatal-warnings
      clang: error: linker command failed with exit code 1 (use -v to see invocation)
      error: command '/usr/bin/clang' failed with exit code 1
      [end of output]

(I also tried extra_link_args += ['-Wl', '-fatal-warnings'] and got the same error message)

However, changing the line to extra_link_args += ['-Wl', '-Wfatal-warnings'] seems to work as expected on Mac. If possible, can you check to see if extra_link_args += ['-Wl', '-Wfatal-warnings'] works on your end?

Thanks!

@yasminetalby yasminetalby added response-requested Waiting on additional info and feedback. Will move to 'closing-soon' in 7 days. pr/needs-review This PR needs a review from a Member. labels Jun 26, 2023
@jmklix jmklix closed this Apr 22, 2024
@graebm
Copy link
Contributor

graebm commented Apr 23, 2024

Fixed in: #528

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/needs-review This PR needs a review from a Member. response-requested Waiting on additional info and feedback. Will move to 'closing-soon' in 7 days.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants