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

CMake: Fix linking with compiler other than MSVC. #104

Merged
merged 1 commit into from
Mar 9, 2021
Merged

CMake: Fix linking with compiler other than MSVC. #104

merged 1 commit into from
Mar 9, 2021

Conversation

Biswa96
Copy link
Contributor

@Biswa96 Biswa96 commented Mar 3, 2021

Previously, MSVC specific pragma comment are used to link with ws2_32 library.
But those does not work with MinGW gcc and clang. Hence, use CMake's own
target_link_libraries() to link with ws2_32 for Win32 platform conditionally.

Previously, MSVC specific pragma comment are used to link with ws2_32 library.
But those does not work with MinGW gcc and clang. Hence, use CMake's own
target_link_libraries() to link with ws2_32 for Win32 platform conditionally.
@@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 2.8.12)
# Defer enabling C and CXX languages.
project(AWSLC NONE)

if(WIN32)
if(MSVC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand why is this line changed, could you please clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because cl.exe is Visual Studio compiler and this PR is about when not compiling with Visual Studio.

@andrewhop
Copy link
Contributor

Thanks for fixing this! We currently only test Windows with MSVC and that build is passing with this change. Windows with Clang is currently untested but we're happy to take this fix now and investigate adding more Windows builds in the future.

How did you test this change?

If you don't mind me asking, what is your use case for AWS-LC?

@Biswa96
Copy link
Contributor Author

Biswa96 commented Mar 8, 2021

Is 'just poking around' considered as use case? 😛

@andrewhop andrewhop merged commit 66e3510 into aws:main Mar 9, 2021
@Biswa96 Biswa96 deleted the cmake-ws2-32-link branch March 9, 2021 20:11
aqjune-aws pushed a commit to aqjune-aws/aws-lc-public that referenced this pull request Mar 4, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
s2n-bignum original commit: awslabs/s2n-bignum@48fb153
dkostic pushed a commit to dkostic/aws-lc that referenced this pull request Jul 22, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
s2n-bignum original commit: awslabs/s2n-bignum@48fb153
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 18, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
s2n-bignum original commit: awslabs/s2n-bignum@48fb153
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 18, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
s2n-bignum original commit: awslabs/s2n-bignum@48fb153

s2n-bignum original commit: awslabs/s2n-bignum@f133bad
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 19, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
s2n-bignum original commit: awslabs/s2n-bignum@48fb153
dkostic pushed a commit to dkostic/aws-lc that referenced this pull request Dec 5, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
dkostic pushed a commit to dkostic/aws-lc that referenced this pull request Dec 10, 2024
* Allow MIT-0 license as well as Apache-2.0 and ISC

* Add appropriate year range to MIT-0 license
s2n-bignum original commit: awslabs/s2n-bignum@48fb153
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