Skip to content

Conversation

@peterNordin
Copy link
Member

This is part one of switch over to CMake, has been in progress for more than three years.
After adding DCP additional work is needed to be able to build release, but i will make a part2

This PR enables me to build all of Hopsan on Linux GCC, Windows MinGW and Windows MSVC 2019.
Including with DCP. But I do not know if it actually works in all these cases. Hopsan at least starts.

Deb build has been migrated to CMake. But flatpak/snap and Windows builds remain.

@peterNordin peterNordin requested a review from robbr48 December 10, 2024 22:58
Copy link
Contributor

@robbr48 robbr48 left a comment

Choose a reason for hiding this comment

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

Seems to work except for the DCPLib. It no is no longer compiled and linked since it is header-only. See separate comments in hopsandcp/CMakeLists.txt.

The conflict is just because I fixed the same typo, so it is very easy to fix.

endif()

set(CMAKE_AUTOMOC ON)
include(${CMAKE_CURRENT_LIST_DIR}/../dependencies/dcplib.cmake)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not include this, since DCPLib is header-only and does no longer require linking.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK it was I while ago I wrote this, so lets see If I can remember and complete this work.

I think that the dcplib.cmake setup header-only targets for dcplib (from its own cmake config packages) it also does a bunch of complex magic to figure out if the dependencies of dcp lib are available and what name the libraries have.

Even if dcplib is header only. its dependencies may still need to be linked if they are not header only.
Such as xercses, libzip / zlib, icu (if available)

#include(${CMAKE_CURRENT_LIST_DIR}/../dependencies/xerces.cmake)

# include(${CMAKE_CURRENT_LIST_DIR}/../dependencies/xerces.cmake)
# include(${CMAKE_CURRENT_LIST_DIR}/../dependencies/libzip.cmake)
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to include libzip.cmake, however, because we must link to libzip directly below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the libzip setup into dcplib.cmake since it was only a dependency to dcplib, ..... I think.

$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../dependencies/libzip/include>
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../dependencies/asio-code/include>
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../dependencies/xerces/include>
# $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../dependencies/dcplib-code/include/bluetooth>
Copy link
Contributor

Choose a reason for hiding this comment

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

These include directories are needed, because we need to link to the headers in DCPLib, libzip, Asio and Xerces. But perhaps they should can be in dcplib.cmake if we remove all stuff with linking to the library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes all of these paths should be added by the targets made available from dcplib.cmake. Most of them are header-only library targets.

target_link_libraries(hopsandcp hopsancore libzip -lws2_32)
else()
target_link_libraries(hopsandcp hopsancore libzip)
target_link_libraries(hopsandcp hopsancore DCPLib::Master DCPLib::Slave DCPLib::Zip DCPLib::Ethernet)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove: DCPLib::Master, DCPLib::Slave and DCPLib::Zip
Add: libzip

Copy link
Member Author

Choose a reason for hiding this comment

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

Those are the targets defined by the dcplib project, so I wanted to "link" (depend) on them instead of explicitly pointing to the header directories. There may be more properties in these targets (compiler definitions and such things) that may be required. I think that DCPLib::Zip depends on libzip

@peterNordin peterNordin force-pushed the switch_to_cmake_part1 branch from 3d090aa to 4f9e5fc Compare August 12, 2025 20:18
@peterNordin peterNordin force-pushed the switch_to_cmake_part1 branch from e9fa0de to a54ee01 Compare August 12, 2025 21:09
@robbr48
Copy link
Contributor

robbr48 commented Aug 18, 2025

There is something wrong with fmi4c, I get the following error:

[cmake] CMake Error in componentLibraries/defaultLibrary/CMakeLists.txt:
[cmake]   Imported target "fmi4c::fmi4c" includes non-existent path
[cmake] 
[cmake]     "/minizip"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants