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 debug build failure on Windows ARM64+MSVC #5285

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix debug build failure on Windows ARM64+MSVC #5285

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,10 @@ if (BUILD_STATIC_LIBS)
PRIVATE "${HDF5_SRC_INCLUDE_DIRS};${HDF5_SRC_BINARY_DIR};${HDF5_COMP_INCLUDE_DIRECTORIES};$<$<BOOL:${HDF5_ENABLE_PARALLEL}>:${MPI_C_INCLUDE_DIRS}>"
INTERFACE "$<INSTALL_INTERFACE:$<INSTALL_PREFIX>/include>;$<BUILD_INTERFACE:${HDF5_SRC_INCLUDE_DIRS};${HDF5_SRC_BINARY_DIR}>"
)
if(MSVC AND CMAKE_SYSTEM_PROCESSOR STREQUAL "ARM64" AND ${HDF_CFG_NAME} MATCHES "Debug")
# Required to work around linker error LNK1322
target_link_options(${HDF5_LIB_TARGET} PRIVATE "/Gy")
endif()
target_compile_options(${HDF5_LIB_TARGET} PRIVATE "${HDF5_CMAKE_C_FLAGS}")
target_compile_definitions(${HDF5_LIB_TARGET}
PUBLIC
Expand Down Expand Up @@ -1131,6 +1135,10 @@ if (BUILD_SHARED_LIBS)
PUBLIC "$<$<BOOL:${HDF5_ENABLE_HDFS}>:${HDFS_INCLUDE_DIR}>"
INTERFACE "$<INSTALL_INTERFACE:$<INSTALL_PREFIX>/include>;$<BUILD_INTERFACE:${HDF5_SRC_INCLUDE_DIRS};${HDF5_SRC_BINARY_DIR}>"
)
if(MSVC AND CMAKE_SYSTEM_PROCESSOR STREQUAL "ARM64" AND ${HDF_CFG_NAME} MATCHES "Debug")
# Required to work around linker error LNK1322
target_link_options(${HDF5_LIB_TARGET} PRIVATE "/Gy")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be ${HDF5_LIBSH_TARGET}, but this kind of flag also seems like something that should be set much higher up in the process rather than here, especially considering the very specific platform and build combination logic.

Copy link
Author

Choose a reason for hiding this comment

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

Is there a place you would suggest to be more appropriate? I'm not massively familiar with the codebase, and would like to make sure I've done things correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we don't really have other calls to target_link_options in the library other than a few niche places in the C++ wrappers, it may take a small bit of work. I suggest having another variable similar to HDF5_CMAKE_C_FLAGS, say HDF5_CMAKE_C_LINK_FLAGS, then setting whatever you need in config/cmake/HDFCompilerFlags.cmake maybe. You can see some examples there like:

list (APPEND HDF5_CMAKE_C_FLAGS "-erroff=%none -DBSD_COMP")

where flags are added to the HDF5_CMAKE_C_FLAGS list. Once you have your flag added to the new variable there, you can have these calls be:

target_link_options (${HDF5_LIB_TARGET} PRIVATE "${HDF5_CMAKE_C_LINK_FLAGS}")
target_link_options (${HDF5_LIBSH_TARGET} PRIVATE "${HDF5_CMAKE_C_LINK_FLAGS}")

for whatever the variable is called. That way will be a bit more maintainable in case other flags need to be added in the future and keeps the platform-specific logic out of the main CMake files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is the macro in HDFMacros.cmake
macro (TARGET_C_PROPERTIES wintarget libtype)
target_compile_options(${wintarget} PRIVATE
"$<$<C_COMPILER_ID:MSVC>:${WIN_COMPILE_FLAGS}>"
"$<$<CXX_COMPILER_ID:MSVC>:${WIN_COMPILE_FLAGS}>"
)
if(MSVC)
set_property(TARGET ${wintarget} APPEND PROPERTY LINK_FLAGS "${WIN_LINK_FLAGS}")
endif()
#Disable UNITY_BUILD for now
set_property(TARGET ${wintarget} APPEND PROPERTY UNITY_BUILD OFF)
endmacro ()

endif()
target_compile_options(${HDF5_LIBSH_TARGET} PRIVATE "${HDF5_CMAKE_C_FLAGS}")
target_compile_definitions(${HDF5_LIBSH_TARGET}
PUBLIC
Expand Down
Loading