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

Fixes Fortran compilation on Windows #3839

Closed
wants to merge 3 commits into from

Conversation

brtnfld
Copy link
Contributor

@brtnfld brtnfld commented Nov 9, 2023

Fixes unresolved symbols with Fortran libraries on Windows with Intel OneAPI.

@brtnfld brtnfld added Merge - To 1.14 Priority - 1. High 🔼 These are important issues that should be resolved in the next release Component - Fortran Fortran wrappers Component - Build CMake, Autotools Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub labels Nov 9, 2023
@brtnfld
Copy link
Contributor Author

brtnfld commented Nov 9, 2023

Need to resolve subfiling DLLs when parallel is enabled.

Copy link
Contributor

@byrnHDF byrnHDF left a comment

Choose a reason for hiding this comment

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

The add_library changes are the only ones that need to be fixed.

if(MSVC)
set(DLLDEF ${HDF5_F90_SRC_BINARY_DIR}/hdf5_fortrandll.def)
endif()
add_library (${HDF5_F90_LIBSH_TARGET} SHARED ${DLLDEF} ${f90_F_SOURCES_SHARED})
Copy link
Contributor

Choose a reason for hiding this comment

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

This relies on "DLLDEF" not being defined for non-msvc. I would prefer that we use an if/else here and in the other file. The other question and maybe why the other form did not work - should we check for it being on windows "if (WIN32) " instead of "if (MSVC)" - because those are not the same thing. (Another choice is "if (WINDOWS)")

# set_property(TARGET ${HDF5_F90_LIBSH_TARGET} APPEND PROPERTY LINK_FLAGS "$<$<STREQUAL:x${CMAKE_Fortran_SIMULATE_ID},xMSVC>:${WIN_LINK_FLAGS}>")
# set_property(TARGET ${HDF5_F90_LIBSH_TARGET} APPEND PROPERTY LINK_FLAGS "$<$<STREQUAL:x${CMAKE_Fortran_SIMULATE_ID},xMSVC>:-DLL>")
# set_property(TARGET ${HDF5_F90_LIBSH_TARGET} APPEND PROPERTY LINK_FLAGS "$<$<STREQUAL:x${CMAKE_Fortran_SIMULATE_ID},xMSVC>:-DEF:${HDF5_F90_SRC_BINARY_DIR}/hdf5_fortrandll.def>")
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.

Maybe MSVC should be WIN32? OR maybe the commented out property commands might work now?

@brtnfld
Copy link
Contributor Author

brtnfld commented Nov 9, 2023

Replaced with PR #3836

@brtnfld brtnfld closed this Nov 9, 2023
@brtnfld brtnfld deleted the fwindows branch November 9, 2023 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Component - Fortran Fortran wrappers Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants