Add CMake configuration for external C-Sources

[beutlich]

Resolves #4093.

This can be used as base for a discussion on CMake configuration.

TODO:

• [x] Darwin specifics
• [x] Compiler specific library subdirectory
• [x] Test in CI (GitHub Actions)
• [x] Unix CFLAGS?
• [x] CTest
• [x] Location of top-level CMakeLists.txt: \<LibName>/Resources/CMakeLists.txt
• [x] Minimum CMake version: 3.15

Note, that this always builds with the static MSVC runtime library (required by Dymola on Win) which best is handled by CMake ≥ 3.15.

[beutlich]

@GallLeo @MatthiasBSchaefer @tobolar For the sake of consistency it might be useful if the triggered MSL test runs by LTX ReSim could also build the binaries after the MSL sources are updated and before Dymola is started first time. This PR might reduce the burden for you now.

cd C:\Projects\.SomeBuildDir
# assume that this repo+branch is checked out at: C:/Projects/MSL
# 32-bit
cmake -S C:/Projects/MSL/Modelica/Resources -B buildWin32 -G "Visual Studio 17 2022" -A Win32 -DMODELICA_UTILITIES_INCLUDE_DIR:PATH="C:/Projects/MSL/.CI/Test"
cmake --build buildWin32 --target install --config Release
# 64-bit
cmake -S C:/Projects/MSL/Modelica/Resources -B buildWin64 -G "Visual Studio 17 2022" -A x64 -DMODELICA_UTILITIES_INCLUDE_DIR:PATH="C:/Projects/MSL/.CI/Test"
cmake --build buildWin64 --target install --config Release
# log the modified files
cd C:\Projects\MSL
git status --porcelain

[maltelenz]

@beutlich I used this as a basis when setting up the System Modeler builds of MSL libs for 4.1.0. It was straight forward, and I could use most of it without changes.

Regarding the installation target directory, we have the following functions to get that (sub)path.

They are more narrow than what you have in some ways (we don't care about as many Visual Studio versions), but also have features the current draft is lacking, like macOS support.

Feel free to incorporate these as you wish in your pull request.

# Determines the Modelica platform name of the current platform
function(get_modelica_platform_name var)
  set(IS_64BIT_BUILD false)
  if(CMAKE_SIZEOF_VOID_P EQUAL 8)
      set(IS_64BIT_BUILD true)
  endif()
  if(APPLE)
    list(LENGTH CMAKE_OSX_ARCHITECTURES NUMBER_OF_OSX_ARCHITECTURES)
    if (NUMBER_OF_OSX_ARCHITECTURES EQUAL 0)
      if (CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64")
        set(PLATFORM_PATH_SUFFIX "darwin64")
      elseif (CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "arm64")
        set(PLATFORM_PATH_SUFFIX "aarch64-darwin")
      else()
        message(FATAL_ERROR "Unknown macOS architecture CMAKE_HOST_SYSTEM_PROCESSOR=${CMAKE_HOST_SYSTEM_PROCESSOR}.")
      endif()
    elseif(NUMBER_OF_OSX_ARCHITECTURES EQUAL 1)
      if (CMAKE_OSX_ARCHITECTURES STREQUAL "x86_64")
        set(PLATFORM_PATH_SUFFIX "darwin64")
      elseif (CMAKE_OSX_ARCHITECTURES STREQUAL "arm64")
        set(PLATFORM_PATH_SUFFIX "aarch64-darwin")
      else()
        message(FATAL_ERROR "Unknown macOS architecture CMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES}.")
      endif()
    else()
      message(FATAL_ERROR "Universal builds not supported CMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES}.")
    endif()
  elseif(UNIX)
    if(IS_64BIT_BUILD)
      set(PLATFORM_PATH_SUFFIX "linux64")
    else()
      set(PLATFORM_PATH_SUFFIX "linux32")
    endif()
  elseif(WIN32)
    # Misleading name, this is for all windows builds
    if(IS_64BIT_BUILD)
      set(PLATFORM_PATH_SUFFIX "win64")
    else()
      set(PLATFORM_PATH_SUFFIX "win32")
    endif()
  endif()

  message(STATUS "Building for Modelica platform ${PLATFORM_PATH_SUFFIX}")

  set(${var} ${PLATFORM_PATH_SUFFIX} PARENT_SCOPE)
endfunction()

# Determines the Modelica platform name of the current platform, including VS version
function(get_modelica_platform_name_with_compiler_version var)

  get_modelica_platform_name(PLATFORM_PATH_SUFFIX)

  if(WIN32)
    if(MSVC_VERSION EQUAL 1900)
      set(PLATFORM_PATH_SUFFIX "${PLATFORM_PATH_SUFFIX}/vs2015")
    elseif(MSVC_VERSION GREATER_EQUAL 1910 AND MSVC_VERSION LESS 1920)
      set(PLATFORM_PATH_SUFFIX "${PLATFORM_PATH_SUFFIX}/vs2017")
    elseif(MSVC_VERSION GREATER_EQUAL 1920 AND MSVC_VERSION LESS 1930)
      set(PLATFORM_PATH_SUFFIX "${PLATFORM_PATH_SUFFIX}/vs2019")
    elseif(MSVC_VERSION GREATER_EQUAL 1930)
      set(PLATFORM_PATH_SUFFIX "${PLATFORM_PATH_SUFFIX}/vs2022")
    endif()
  endif()

  set(${var} ${PLATFORM_PATH_SUFFIX} PARENT_SCOPE)
endfunction()

Can be used as follows:

get_modelica_platform_name_with_compiler_version(MODELICA_PLATFORM_NAME)

set(CMAKE_INSTALL_LIBDIR "${CMAKE_INSTALL_PREFIX}/Library/${MODELICA_PLATFORM_NAME}" CACHE PATH "Library installation path (don't change)" FORCE)

[beutlich]

Thanks @maltelenz. It's incorporated, refactored and further tested. The 2 Modelica_*.cmake files are kind of generic.

Is the setting of the UNIX CFLAGS alright for you?

[maltelenz]

@beutlich Looking great!

I added two commits to further align it with what we do for System Modeler, in my own continuation of your branch here. Please have a look and feel free to cherry-pick or give feedback on what you think about the changes.

The changes are:

• Explicitly compile with -fPIC (related to your question about CFLAGS, this is the only difference we had to your current list).
• New option to build against an already compiled zlib elsewhere. We already use zlib in our simulation runtime, so we do not want the zlib from MSL (it would not be used anyway, since our own zlib will be earlier in the search paths for linking simulations).
• New option to also install the ModelicaExternalC library.

I left the default behaviors for the new options as they were previously.

I have done basic testing with System Modeler on a Linux machine so far. I will do more testing on all our supported platforms in the coming days and weeks as we do more testing of the MSL 4.1.0 branch.

[beutlich]

Thanks for the two new updates. Will pick them and adapt later.

• compile with -fPIC

There's a more CMake-like setting: https://stackoverflow.com/a/38297422/8520615

[maltelenz]

• compile with -fPIC

There's a more CMake-like setting: https://stackoverflow.com/a/38297422/8520615

I will update my branch.

Edit: Actually, the default behavior from cmake already seems good, so looks like we don't need this flag.

[beutlich]

Actually, the default behavior from cmake already seems good, so looks like we don't need this flag.

Yes, I read about it previously, but could not find it anymore.

[maltelenz]

Actually, the default behavior from cmake already seems good, so looks like we don't need this flag.

Yes, I read about it previously, but could not find it anymore.

https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html

This property is True by default for SHARED and MODULE library targets and False otherwise.

[beutlich]

@maltelenz I pushed your commit and slightly adapted the variable names. Thankfs for the contribution.

[beutlich]

CI runs the testsuite now.

I'd prefer to have the unit tests close to the sources (and not as part of the .CI folder though).

[beutlich]

@maltelenz Cygwin detects linux64 as lib dir which is not correct. I guess it should be win64 (like MinGW), but with a different naming. Well, seems to be a edge case issue only since CygWin is not that widely used for Modelica simulation environments.

$ cmake -S . -B build_cygwin  -DMODELICA_UTILITIES_INCLUDE_DIR="/cygdrive/c/Projekte/MSL/.CI/Test" -DCMAKE_BUILD_TYPE=Debug -DZLIB_INCLUDE_DIR=/usr/include -DMODELICA_BUILD_ZLIB=OFF
-- Building for Modelica platform linux64

[maltelenz]

@maltelenz Cygwin detects linux64 as lib dir which is not correct. I guess it should be win64 (like MinGW), but with a different naming. Well, seems to be a edge case issue only since CygWin is not that widely used for Modelica simulation environments.

$ cmake -S . -B build_cygwin  -DMODELICA_UTILITIES_INCLUDE_DIR="/cygdrive/c/Projekte/MSL/.CI/Test" -DCMAKE_BUILD_TYPE=Debug -DZLIB_INCLUDE_DIR=/usr/include -DMODELICA_BUILD_ZLIB=OFF
-- Building for Modelica platform linux64

I'm not surprised, we haven't used our library detection code in cygwin. If we knew what directory to generate, we could do something with the CYGWIN variable. But since the spec doesn't say anything about cygwin, and I'm not aware of any tool that has created a de-facto choice, I think our choices are to add an explicit case making it fail with an error, or ignore it.

[beutlich]

Requesting changes to the CMAKE_GENERATOR restriction on Windows as in previous suggestion to enable use of Ninja.

This is the one outstanding thing that I would like changed before we merge it.

After merging, it now prints for MinGW:

Configuring to link the MSVC runtime library statically

This is probably not what we want.

[beutlich]

See also https://github.com/tbeu/ModelicaTableAdditions/commit/5c37fbcaa541d58e67c5be43ecd4a991a076f3ec for another PoC.

[beutlich]

I used (and already) adapted the CMake configuration a lot and I believe there is a need for restructuring regarding testability since usually all CMake relevant folders are subfolders of the root CMakeLists.txt file.

1. The C/C++ unit tests should not be located in a .CI folder since they are also ment for local testing.
2. The C/C++ unit tests should be closer to the source.
3. The C/C++ unit tests could be part of the release asset.

With these points in mind, the root CMakeLists.txt should be moved up from Modelica/Resources/BuildProjects/CMake/ to Modelica/Resources/ and the unit tests should be moved from ./CI/Test/ to Modelica/Resources/Test/.

Edit: Here is how the restructuring could look like.

[beutlich]

The main question for me is where to put the top-level CMakeLists.txt file. This needs to be decided since otherwise moving this file to a subdirectory later can be seen as a breaking change.

1. In the PR it is in ./\<LibName>/Resources/BuildProjects/CMake/ currently.
2. A better proposal is to have it ./\<LibName>/Resources/ which eases unit testing of the C sources.
3. A third proposal could be to have it directly in ./\<LibName>.
4. A fourth idea could be the repository root ./.

In my opinion proposal 2 is good and specific enough.

[maltelenz]

I can see a future where we want to specify the location of the top CMakeLists.txt in the Modelica language.

For this, I think it needs to go at least inside ./<LibName> (otherwise, how would a tool find it for a specific LibName). That disqualifies 4.

I agree that it seems good style to have the top level CMakeLists.txt above the relevant folders, as you say. That disqualifies 1.

It seems bad style to clutter ./<LibName> directly (although I don't feel super strongly about this).

Based on this reasoning, I prefer option 2.

[beutlich]

I forgot to mention that M_DD also uses option 2: https://github.com/modelica-3rdparty/Modelica_DeviceDrivers/tree/master/Modelica_DeviceDrivers/Resources

OK, will update PR accordingly.

[henrikt-ma]

I forgot to mention that M_DD also uses option 2: https://github.com/modelica-3rdparty/Modelica_DeviceDrivers/tree/master/Modelica_DeviceDrivers/Resources

OK, will update PR accordingly.

I also like this option for various reasons.

[casella]

@beutlich, is this ready to be merged to the master branch? Should we have another round of reviews for that purpose?

[beutlich]

Yes, ready for review, all tasks solved, and discussions resolved. I was hoping to get some early second feedback but this was not the case.

[otronarp]

Looks good (I have only tested on macOS).

[casella]

@otronarp can you please re-review this? For some reason, there is only one approval in the system and we need to to merge. Thanks!

[otronarp]

@casella I approved it again, but it doesn't seem to help. Perhaps my review doesn't count since I'm not listed as member of map-lib?

[casella]

@otronarp could be. Would you be interested to join?

I'll force the merge, this is not a critical PR as it is confined to a specific Resources sub-directory.