Update cmake support for AdsLib#295
Conversation
This commit modernized the cmake implementations and fixes issues when
building with the MSVC (Microsoft Visual Compiler). Building with clang
and GCC compilers was also tested and supported.
This cmake implementation allows building both variants of
the AdsLib (standalone and with twincat rounter) on a single cmake
project and is also possible to install both on the same directory (for
easy import) in both debug and release builds.
The changes included on this commit are as follows:
- Increase cmake min version to 3.23
- Set the current AdsLib version in project() (previously 0.0.20)
- Add basic module FindTcAdsDll.cmake for locating the TcAdsDll (windows
only).
- Allow building both shared and static library version. Simply set
BUILD_SHARED_LIBS to ON or OFF according to your needs.
- Add generation of a cmake config file (AdsConfig.cmake) on install and
enebale using find_package on the consuming project. One only needs to
specify the Ads_ROOT variable.
- Use GNU-style installation directories.
- Provide library headers under include/ads
- Improve import target naming.
- AdsLib : Standalone variant of the AdsLib.
- TcAdsLib : TwinCAT rounter variant of the AdsLib. Only if TcAdsDll
is found and available. Otherwise the target is no created and a
warning message is display.
- Fatal error if BUILD_SHARED_LIBS=ON when using the microsoft visual
studio compiler. The reson is that the AdsLib does not provide the
needed import and export decorators.
- Allow building tests using cmake.
- Allow building AdsTool using cmake in both standalone and twincat
rounter variants.
- Add build options:
- WITH_TEST : When OFF the project does not create/build targets for the
different unitests and integration tests.
- WITH_EXAMPLES : When OFF the project does not create/build targets
for the project examples.
- STANDALONE_ONLY : When ON create/build only the AdsLib stand alone
version (does seach for TwinCat TcAdsDll)
Steps for testing/building.
- Install cmake min v3.23
- Configure the cmake project with the needed options:
cmake -G "Ninja"
-B %BUILD_DIR%\release
-DCMAKE_INSTALL_PREFIX=%INSTALL_DIR%
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_PREFIX_PATH=C:\TwinCAT\AdsApi\TcAdsDll
-DCMAKE_POSITION_INDEPENDENT_CODE=ON
-DBUILD_SHARED_LIBS=OFF
-DWITH_TESTS=OFF
-DWITH_EXAMPLES=OFF
-DSTANDALONE_ONLY=OFF
- Build & install:
cmake --build %BUILD_DIR%\release
cmake --build %BUILD_DIR%\release --target install
izonfreak
force-pushed
the
update-cmake-support
branch
from
2262ce5 to
ff91c39
Compare
pbruenn
left a comment
pbruenn
left a comment
There was a problem hiding this comment.
Hi @izonfreak ,
thanks for this PR. I can't really review the CMake changes, as they are more of a community effort. I chose Meson over CMake because, at the time, it seemed much cleaner to me (and I still think so today).
That said, the reality is that CMake is used in many more places, and I understand the importance of supporting it. Therefore, I'm happy to merge the CMake--related changes as they are, provided that my small CMake test continues to pass:
Lines 113 to 114 in 6788651
However, please split the other minor changes into separate commits, and then we can merge this PR, thanks.
p.s.: I just updated the CONTRIBUTING file to make clear we prefer logical split commits.
| rowNames{ { | ||
| "1", "2", "5", "10", "20", "50", "100", "200", "500", | ||
| "\u221E" // infinity symbol | ||
| u8"\u221E" // infinity symbol |
There was a problem hiding this comment.
Please keep this separate from the CMake changes and put it into its own commit explaining why this is needed.
| } | ||
| if (SOCK_STREAM == type) { | ||
| if (::connect(m_Socket, rp->ai_addr, rp->ai_addrlen)) { | ||
| if (::connect(m_Socket, rp->ai_addr, static_cast<int>(rp->ai_addrlen))) { |
There was a problem hiding this comment.
Again, please separate this from the CMake changes and put it into its own commit and explain why we need the change.
| .vscode/ | ||
|
|
||
| # Clion | ||
| .idea/ |
There was a problem hiding this comment.
Well, I know this file is already quite heavy and I already forgot forgotten about it, if until you didn't remember until you reminded me.
I don't want to bloat it even more with IDE specific stuff.
Git supports $XDG_CONFIG_HOME/git/ignore which is much better suited for developer specific IDE files. You won't even need to copy & paste your .gitignore settings into every project anymore.
https://git-scm.com/docs/gitignore
3 participants
This commit modernized the cmake implementations and fixes issues when building with the MSVC (Microsoft Visual Compiler). Building with clang and GCC compilers was also tested and supported.
This cmake implementation allows building both variants of the AdsLib (standalone and with twincat rounter) on a single cmake project and is also possible to install both on the same directory (for easy import) in both debug and release builds.
The changes included on this commit are as follows:
Steps for testing/building.
cmake -G "Ninja"
-B %BUILD_DIR%\release
-DCMAKE_INSTALL_PREFIX=%INSTALL_DIR%
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_PREFIX_PATH=C:\TwinCAT\AdsApi\TcAdsDll
-DCMAKE_POSITION_INDEPENDENT_CODE=ON
-DBUILD_SHARED_LIBS=OFF
-DWITH_TESTS=OFF
-DWITH_EXAMPLES=OFF
-DSTANDALONE_ONLY=OFF
cmake --build %BUILD_DIR%\release
cmake --build %BUILD_DIR%\release --target install