Allow turning shared library building off #758

t-b · 2020-08-28T13:08:23Z

Close #740.

In 4791f0d (Rework static/shared variants into a configure-time build-switch (tango-controls#688), 2020-05-28) the option TANGO_BUILD_SHARED was added to select a shared or static library. Internally BUILD_SHARED_LIBS was already set to the same value. Switching to BUILD_SHARED_LIBS and dropping TANGO_BUILD_SHARED allows us to replace a specialized option with the default one [1]. [1]: https://cmake.org/cmake/help/v3.18/command/add_library.html#command:add_library

It does not make sense to check if(A) ... elseif(A) ... endif() so let's change the elseif to an else.

This is confusing and legacy according to [1]: Per legacy, the else() and endif() commands admit an optional <condition> argument. If used, it must be a verbatim repeat of the argument of the opening if command. [1]: https://cmake.org/cmake/help/v3.18/command/if.html#command:if

Remove trailing whitespace and empty lines at the front and back of the files.

mliszcz

Thanks for this change! I added one comment regarding the tests, could you please have a look?

CMakeLists.txt

mliszcz · 2020-08-31T11:14:06Z

Maybe we should have a CI job for static build on Linux?

In af72afb (Generate static library and fix compilation definitions (tango-controls#17) (tango-controls#437), 2018-05-15) support was added for building a static library next to the dynamic one. As most users want to have only either library we use the newly introduced option BUILD_SHARED_LIBS to build either one. This also solves issues when cross-compiling where you might not have shared libraries for the target architecture.

t-b · 2020-09-01T14:11:31Z

@mliszcz I've added a travis job.

@mliszcz @bourtemb Any opinions about backporting? I would think that having that in 9.3-backports would be good. IMHO only the last two commits are needed.

mliszcz

LGTM.

As for the backport, I do not have any opinion here. Maybe just ask the users if they need this.

JeanLucPons · 2020-09-03T06:19:59Z

Hello,
Thanks for this. I tried but am I doing something wrong ?
Thanks for help.

cmake .. -DCMAKE_CXX_COMPILER:FILEPATH=/segfs/linux/xilinx_sdk/arm-toolchain/bin/arm-xilinx-linux-gnueabi-g++ -DIDL_BASE=/users/pons/tango/tango-idl/install -DOMNI_BASE=/segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx -DZMQ_BASE=/segfs/tango/transport/zmq4.0.5/arm_xilinx -DIDL_BASE=/users/pons/tango/tango-idl -DTANGO_JPEG_MMX=OFF -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTING=OFF -DCMAKE_INSTALL_PREFIX=/segfs/tango/release/arm_xilinx/PR758

- CMake: version 3.7.2
-- Target platform: Linux 32-bit
-- C++ Compiler: GNU with version 4.7.3
-- C Compiler: GNU with version 6.3.0
-- Build type: Release
-- Environment: TANGO_HOST=acudebian7:10000
Using IDL_BASEIDL_BASE=/users/pons/tango/tango-idl
Using OMNI_BASE=/segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx
Using ZMQ_BASE=/segfs/tango/transport/zmq4.0.5/arm_xilinx
-- Looking for C++ include zmq.hpp
-- Looking for C++ include zmq.hpp - found
-- Check if zmq::socket has a disconnect method: FALSE
Verifying /segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx/bin/omniidl
omniidl version 1.0
found GNU compiler ...
setup for release build ...
-- Check if the compiler supports C++14 relaxed constexpr: FALSE
Generate tango.h, tangoSK.cpp and tangoDynSK.cpp from idl
Using OMNIIDL_PATH=/segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx/bin/
Using IDL=/users/pons/tango/tango-idl/include
-- Configuring done
CMake Error in CMakeLists.txt:
  Target "tango-static" requires the language dialect "CXX14" , but CMake
  does not know the compile flags to use to enable it.

t-b · 2020-09-03T14:42:11Z

@JeanLucPons CI passes, and it has a new job with static configuration, see 1.

You are using different compilers for C and C++, this does look odd.
I think cmake tries to tell you that your compiler is too old. You can try passing -DCMAKE_CXX_STANDARD=11 which might still work, but we will be moving towards C++14. So using something newer than g++ 4.7 is definitly more future proof.

JeanLucPons · 2020-09-08T07:27:15Z

Unfortunately I cannot update the compiler. The toolchain has been built by a manufacturer for a specific hardware and cannot be updated. I tried to use -DCMAKE_CXX_STANDARD=11 and the cmake ended successfully. However the make wants to build MMX although it was disabled (-DTANGO_JPEG_MMX=OFF).

[debian9-devel.pons] > cmake .. -DCMAKE_CXX_COMPILER:FILEPATH=/segfs/linux/xilinx_sdk/arm-toolchain/bin/arm-xilinx-linux-gnueabi-g++ -DIDL_BASE=/users/pons/tango/tango-idl/install -DOMNI_BASE=/segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx -DZMQ_BASE=/segfs/tango/transport/zmq4.0.5/arm_xilinx -DIDL_BASE=/users/pons/tango/tango-idl -DTANGO_JPEG_MMX=OFF -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTING=OFF -DCMAKE_CXX_STANDARD=11 -DCMAKE_INSTALL_PREFIX=/segfs/tango/release/arm_xilinx/PR758-- CMake: version 3.7.2
-- Target platform: Linux 32-bit
-- C++ Compiler: GNU with version 4.7.3
-- C Compiler: GNU with version 6.3.0
-- Build type: Release
-- Environment: TANGO_HOST=acudebian7:10000
Using IDL_BASEIDL_BASE=/users/pons/tango/tango-idl
Using OMNI_BASE=/segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx
Using ZMQ_BASE=/segfs/tango/transport/zmq4.0.5/arm_xilinx
-- Looking for C++ include zmq.hpp
-- Looking for C++ include zmq.hpp - found
-- Check if zmq::socket has a disconnect method: FALSE
Verifying /segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx/bin/omniidl
omniidl version 1.0
found GNU compiler ...
setup for release build ...
-- Check if the compiler supports C++14 relaxed constexpr: FALSE
Generate tango.h, tangoSK.cpp and tangoDynSK.cpp from idl
Using OMNIIDL_PATH=/segfs/tango/ORB/omniORB4.2.1_p1/arm_xilinx/bin/
Using IDL=/users/pons/tango/tango-idl/include
-- Configuring done
-- Generating done
-- Build files have been written to: /users/pons/tango/cppTango.PR758/build
[debian9-devel.pons] > 
[debian9-devel.pons] > make
[  0%] Building CXX object cppapi/server/jpeg_mmx/CMakeFiles/jpeg_mmx_objects.dir/jpeg_color_mmx.cpp.o
arm-xilinx-linux-gnueabi-g++: error: unrecognized command line option '-fstack-protector-strong'
cppapi/server/jpeg_mmx/CMakeFiles/jpeg_mmx_objects.dir/build.make:62: recipe for target 'cppapi/server/jpeg_mmx/CMakeFiles/jpeg_mmx_objects.dir/jpeg_color_mmx.cpp.o' failed

mliszcz · 2020-09-08T08:40:52Z

Hi @JeanLucPons, let me clarify a few points here:

cppTango (tango-9-lts branch and also this PR) requires C++14. CMAKE_CXX_STANDARD is here to allow compiling with newer C++ standard versions. Things generally won't work if you pass C++11 there. With your compiler you should use one of 9.3.x releases.
The flag TANGO_JPEG_MMX if for disabling use of MMX extensions in our internal jpeg_mmx library. This basically disables -mmmx switch. The library is still compiled, as you noticed.
The compilation is failing due to your compiler not supporting -fstack-protector-strong flag. In GCC this flag is only available since 4.9.0 release. We do not support older GCC releases as they do not support most of C++14 features (see first bullet). As of today, the oldest supported and tested in CI version of GCC is 4.9.2. To work around this particular problem, you may try building in Debug mode which should not include any hardening flags (-DCMAKE_BUILD_TYPE=Debug).

JeanLucPons · 2020-09-08T10:51:01Z

OK thanks.
There is no interest for me to compile in debug mode as I search maximum performance for running server on small embedded devices (Spark BPM). For the moment the 9.3.4-rc6 is working well and I hope it will work for the next years... We have at the ESRF lots of BPM (and BLM) spark devices that require embedded Tango.

t-b · 2020-09-08T14:26:37Z

@JeanLucPons Will make a backport PR available.

t-b · 2020-09-10T10:26:06Z

@JeanLucPons See #763 for the backport PR.

bourtemb

Looks good to me.
Thanks!

t-b added 4 commits August 28, 2020 12:33

configure/cmake_linux.cmake: Fix CURL variable checking

e978d23

It does not make sense to check if(A) ... elseif(A) ... endif() so let's change the elseif to an else.

CMake: Whitespace cleanup

d4f823f

Remove trailing whitespace and empty lines at the front and back of the files.

t-b requested review from bourtemb and mliszcz as code owners August 28, 2020 13:08

t-b added the Ready For Merge label Aug 28, 2020

mliszcz reviewed Aug 28, 2020

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

mliszcz previously approved these changes Aug 31, 2020

View reviewed changes

t-b added 2 commits September 1, 2020 16:01

travis: Add job for compiling tango static library

998a008

t-b dismissed mliszcz’s stale review via 998a008 September 1, 2020 14:10

t-b force-pushed the allow-turning-shared-library-building-off branch from ec13356 to 998a008 Compare September 1, 2020 14:10

t-b mentioned this pull request Sep 1, 2020

Docker hub TOS changes #761

Closed

mliszcz approved these changes Sep 2, 2020

View reviewed changes

t-b mentioned this pull request Sep 2, 2020

Problems when cross compiling for ARM #740

Closed

t-b added the Candidate For Backport Requires a backport to the release branches label Sep 8, 2020

t-b removed the Candidate For Backport Requires a backport to the release branches label Sep 10, 2020

bourtemb approved these changes Sep 15, 2020

View reviewed changes

t-b merged commit ce78115 into tango-controls:tango-9-lts Sep 15, 2020

t-b deleted the allow-turning-shared-library-building-off branch September 15, 2020 10:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow turning shared library building off #758

Allow turning shared library building off #758

t-b commented Aug 28, 2020

mliszcz left a comment

mliszcz commented Aug 31, 2020

t-b commented Sep 1, 2020

mliszcz left a comment

JeanLucPons commented Sep 3, 2020

t-b commented Sep 3, 2020

JeanLucPons commented Sep 8, 2020

mliszcz commented Sep 8, 2020

JeanLucPons commented Sep 8, 2020 •

edited

Loading

t-b commented Sep 8, 2020

t-b commented Sep 10, 2020

bourtemb left a comment

Allow turning shared library building off #758

Allow turning shared library building off #758

Conversation

t-b commented Aug 28, 2020

mliszcz left a comment

Choose a reason for hiding this comment

mliszcz commented Aug 31, 2020

t-b commented Sep 1, 2020

mliszcz left a comment

Choose a reason for hiding this comment

JeanLucPons commented Sep 3, 2020

t-b commented Sep 3, 2020

JeanLucPons commented Sep 8, 2020

mliszcz commented Sep 8, 2020

JeanLucPons commented Sep 8, 2020 • edited Loading

t-b commented Sep 8, 2020

t-b commented Sep 10, 2020

bourtemb left a comment

Choose a reason for hiding this comment

JeanLucPons commented Sep 8, 2020 •

edited

Loading