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

Fix libcmt/libcmtd.lib link warning #1338

Merged
merged 3 commits into from
Sep 29, 2020
Merged

Fix libcmt/libcmtd.lib link warning #1338

merged 3 commits into from
Sep 29, 2020

Conversation

antkmsft
Copy link
Member

@antkmsft antkmsft commented Sep 22, 2020

Closes #1020

@ahsonkhan ahsonkhan added the EngSys This issue is impacting the engineering system. label Sep 22, 2020
@ahsonkhan ahsonkhan added this to the [2020] October milestone Sep 22, 2020
@antkmsft antkmsft changed the title Fix libcmt/libcmtd.lib link warning [not ready for review yet] Fix libcmt/libcmtd.lib link warning Sep 22, 2020
@antkmsft antkmsft changed the title [not ready for review yet] Fix libcmt/libcmtd.lib link warning Fix libcmt/libcmtd.lib link warning Sep 28, 2020
@antkmsft antkmsft merged commit 21a389d into Azure:master Sep 29, 2020
@@ -169,7 +169,7 @@ jobs:
Windows_Preconditions_UnitTests_Samples:
OSVmImage: 'windows-2019'
vcpkg.deps: 'curl[winssl] paho-mqtt cmocka'
VCPKG_DEFAULT_TRIPLET: 'x64-windows-static'
VCPKG_DEFAULT_TRIPLET: 'x64-windows-static-md'
Copy link
Contributor

Choose a reason for hiding this comment

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

What does -md mean in this context and how does that fix the underlying issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked too closely, but this might be interesting to follow up to understand why:
microsoft/vcpkg#1131 (comment)

It is discouraged to use static-md triplet...

I wonder if the issue is with the static part or the -md part.

Copy link
Contributor

Choose a reason for hiding this comment

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

@barcharcraz - can you please take a look at this as well and share your thoughts.

Copy link
Member Author

Choose a reason for hiding this comment

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

That thread is from 2017. Probably there were issues back then. And also I can see that these triplets were added to the officiale vcpkg repo on February, so maybe there were other issues at a time. But now it is there. microsoft/vcpkg@71a9def. Right now, it seems permanent and I don't see why we can't use it.
The reason why we were getting the warning was that we built our SDK with dynamic runtime, but cmocka was built with a reference to static runtime, and we were inking them together, and that is what the warning was telling us. Now we build cmocka with the same runtime as our SDK, so now they are consistent, and there is no warning anymore.

@@ -215,7 +215,7 @@ jobs:
Windows_Release_MapFiles:
OSVmImage: 'windows-2019'
vcpkg.deps: ''
VCPKG_DEFAULT_TRIPLET: 'x64-windows-static'
VCPKG_DEFAULT_TRIPLET: 'x64-windows-static-md'
Copy link
Contributor

@ahsonkhan ahsonkhan Sep 29, 2020

Choose a reason for hiding this comment

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

Does it make sense to use -md even for the release leg (presumably -md refers to some debug runtime)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not. -md stands for "multithreaded dynamic".

Copy link
Contributor

Choose a reason for hiding this comment

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

@antkmsft antkmsft deleted the libcmtd branch October 3, 2020 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't suppress linker warning about libcmtd.lib
4 participants