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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
780fc85
Rebase
antkmsft Sep 22, 2020
d38d4cc
toggle rebuild
antkmsft Sep 28, 2020
3d5ef23
toggle rebuild
antkmsft Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions cmake-modules/AddTestCMocka.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,6 @@ function(ADD_CMOCKA_TEST _TARGET_NAME)
)
endif()

# Workaround for linker warning LNK4098: defaultlib 'LIBCMTD' conflicts with use of other libs
if (MSVC)
set_target_properties(${_TARGET_NAME}
PROPERTIES
LINK_FLAGS
"/NODEFAULTLIB:libcmtd.lib"
LINK_FLAGS_RELEASE
"/NODEFAULTLIB:libcmt.lib"
)
endif()

target_include_directories(${_TARGET_NAME} PRIVATE ${CMOCKA_INCLUDE_DIR})

if (DEFINED _add_cmocka_test_PRIVATE_ACCESS)
Expand Down
6 changes: 3 additions & 3 deletions eng/pipelines/templates/jobs/archetype-sdk-client.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ jobs:
WindowsX86_Release_MapFiles_UnitTests:
OSVmImage: 'windows-2019'
vcpkg.deps: 'cmocka'
VCPKG_DEFAULT_TRIPLET: 'x86-windows-static'
VCPKG_DEFAULT_TRIPLET: 'x86-windows-static-md'
build.args: '-DPRECONDITIONS=OFF -DUNIT_TESTING=ON -DLOGGING=OFF'
CMAKE_GENERATOR: 'Visual Studio 16 2019'
CMAKE_GENERATOR_PLATFORM: Win32
Expand Down Expand Up @@ -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.

build.args: '-DTRANSPORT_CURL=ON -DTRANSPORT_PAHO=ON -DAZ_PLATFORM_IMPL=WIN32 -DUNIT_TESTING=ON -DLOGGING=OFF'
CMAKE_GENERATOR: 'Visual Studio 16 2019'
CMAKE_GENERATOR_PLATFORM: x64
Expand Down Expand Up @@ -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 •
edited
Loading

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.

Consider updating references x64-windows-static in docs to include -md. We have a couple of places if you search for triplet:
https://github.com/Azure/azure-sdk-for-c/blob/ef22522414e5b56acba056eae84dd3facda63865/sdk/samples/iot/README.md#set-environment-variables

https://github.com/Azure/azure-sdk-for-c/blob/master/sdk/samples/iot/docs/how_to_iot_hub_samples_windows.md#setup-instructions

build.args: '-DPRECONDITIONS=OFF -DLOGGING=OFF'
CMAKE_GENERATOR: 'Visual Studio 16 2019'
CMAKE_GENERATOR_PLATFORM: x64
Expand Down
31 changes: 0 additions & 31 deletions sdk/samples/iot/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,6 @@ target_link_libraries(az_iot_sample_common
OpenSSL::Crypto
)

# Workaround for linker warning LNK4098: defaultlib 'LIBCMTD' conflicts with use of other libs
if (MSVC)
set_target_properties(az_iot_sample_common
PROPERTIES
LINK_FLAGS
"/NODEFAULTLIB:libcmtd.lib"
LINK_FLAGS_RELEASE
"/NODEFAULTLIB:libcmt.lib"
)
endif()

add_library (az::iot::sample::common ALIAS az_iot_sample_common)

# Azure IoT Samples Executables
Expand Down Expand Up @@ -154,24 +143,4 @@ target_link_libraries(paho_iot_provisioning_sas_sample
az::iot::sample::common
)

# Workaround for linker warning LNK4098: defaultlib 'LIBCMTD' conflicts with use of other libs
if (MSVC)
set_target_properties(
paho_iot_hub_c2d_sample
paho_iot_hub_methods_sample
paho_iot_hub_pnp_component_sample
paho_iot_hub_pnp_sample
paho_iot_hub_telemetry_sample
paho_iot_hub_sas_telemetry_sample
paho_iot_hub_twin_sample
paho_iot_provisioning_sample
paho_iot_provisioning_sas_sample
PROPERTIES
LINK_FLAGS
"/NODEFAULTLIB:libcmtd.lib"
LINK_FLAGS_RELEASE
"/NODEFAULTLIB:libcmt.lib"
)
endif()

endif() # TRANSPORT_PAHO
1 change: 0 additions & 1 deletion sdk/tests/iot/common/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ add_cmocka_test(az_iot_common_test SOURCES
LINK_TARGETS
az_iot_common
az_core
${PAL}
)
1 change: 0 additions & 1 deletion sdk/tests/iot/hub/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,4 @@ add_cmocka_test(az_iot_hub_test SOURCES
az_iot_common
az_iot_hub
az_core
${PAL}
)
1 change: 0 additions & 1 deletion sdk/tests/iot/provisioning/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@ add_cmocka_test(az_iot_provisioning_test SOURCES
az_iot_common
az_iot_provisioning
az_core
${PAL}
)