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

Add clrcompression static library #33922

Merged
merged 2 commits into from
Mar 22, 2020
Merged
Changes from 1 commit
Commits
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
13 changes: 13 additions & 0 deletions src/libraries/Native/Windows/clrcompression/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,25 @@ add_library(clrcompression
${CMAKE_REPO_ROOT}/artifacts/obj/NativeVersion.rc
)

add_library(clrcompression-static
STATIC
${NATIVECOMPRESSION_SOURCES}
# This will add versioning to the library
${CMAKE_REPO_ROOT}/artifacts/obj/NativeVersion.rc
)

SET_TARGET_PROPERTIES(clrcompression-static PROPERTIES PREFIX "")
SET_TARGET_PROPERTIES(clrcompression-static PROPERTIES OUTPUT_NAME libclrcompression)
Copy link
Member

Choose a reason for hiding this comment

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

@NextTurn I have a couple of questions:

  • Why do we add the lib prefix on Windows? This naming convention is Unix only.
  • If we really want to do that for some reason, why don't we use the PREFIX property to do that instead of setting the PREFIX to an empty string and modifying the OUTPUT_NAME?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we add the lib prefix on Windows? This naming convention is Unix only.

There is already a clrcompression.lib on Windows serving as an import library. The lib prefix is taken from a similar library libnethost.lib as per discussion in dotnet/core-setup#8659.

If we really want to do that for some reason, why don't we use the PREFIX property to do that instead of setting the PREFIX to an empty string and modifying the OUTPUT_NAME?

Setting the PREFIX to lib seems to work as well... I'll take another look later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about we set PREFIX to lib and then OUTPUT_NAME to clrcompression?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I have not realized before that we need to set the output name anyways, as it would be clrcompression-static by default. So I guess it is not worth the hassle to spend time changing it to set the PREFIX to lib. Please feel free to keep it as is.


# Allow specification of arguments that should be passed to the linker
SET_TARGET_PROPERTIES(clrcompression PROPERTIES LINK_FLAGS ${__LinkArgs})
SET_TARGET_PROPERTIES(clrcompression-static PROPERTIES LINK_FLAGS ${__LinkArgs})

# Allow specification of libraries that should be linked against
# CMake doesn't like space delimiters as input to target_link_libraries
separate_arguments(linker_libs_sanitized WINDOWS_COMMAND ${__LinkLibraries})
target_link_libraries(clrcompression ${linker_libs_sanitized})
target_link_libraries(clrcompression-static ${linker_libs_sanitized})

GENERATE_EXPORT_HEADER( clrcompression
BASE_NAME clrcompression
Expand All @@ -95,5 +107,6 @@ GENERATE_EXPORT_HEADER( clrcompression
)

install (TARGETS clrcompression DESTINATION .)
install (TARGETS clrcompression-static DESTINATION .)
install (FILES ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_BUILD_TYPE}/clrcompression.pdb DESTINATION .)