Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

Add CMake opt files (#34) #36

Merged
merged 4 commits into from
Jun 4, 2019
Merged

Conversation

bourtemb
Copy link
Member

Add cmake_common_target.opt and cmake_tango.opt to the distribution
in order to be able to generate CMakeLists.txt files for Device
Servers from Pogo.

Add cmake_common_target.opt and cmake_tango.opt to the distribution
in order to be able to generate CMakeLists.txt files for Device
Servers from Pogo.
message(STATUS "The compiler ${CMAKE_CXX_COMPILER} has no C++11 support.")
endif()

set(CMAKE_CXX_FLAGS "${CXXFLAGS_USER} ${CMAKE_CXX_FLAGS} ${C++11_FLAGS}")
Copy link

Choose a reason for hiding this comment

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

Consider removing C++11_FLAGS. This is harmful as all compilers are using C++14 by default from some time.

Following workaround does not work as CMAKE_CXX_FLAGS goes before C++11_FLAGS and only last -std= flag is effective.

cmake .. -DCMAKE_CXX_FLAGS='-std=c++17'

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 see. Thanks for the input.
So you propose to use directly the default provided by the compiler and if the user wants to add -std option, this can be added via the CXXFLAGS_USER (in the device server Makefile) or via CMAKE_CXX_FLAGS CMake variable?

Copy link

Choose a reason for hiding this comment

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

Yes, I suggest to remove any -std= options. It seems to be unnecessary limitation (for gcc 6 or better), as there were efforts in the past to compile tango with c++17 (tango-controls/cppTango#405).

Tango does not require c++11 so the flag is not helping also in gcc 5 case (which defaults to c++98).

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 removed -std flag from cmake_tango.opt and from tango.opt (for consistency) in the following commits:

Copy link

Choose a reason for hiding this comment

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

Thanks! I've just checked the new tango.opt. A device server (benchmark) compiles just fine without -std on gcc 7. It also compiles with -std=c++17.

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've checked with gcc 4.4 (on an old Debian 6) and it requires the flag -std=c++0x or you would get this error message:

In file included from /usr/include/c++/4.4/type_traits:35,
                 from /tmp/Reynald/install/debian6/include/tango/devapi_attr.tpp:38,
                 from /tmp/Reynald/install/debian6/include/tango/tango.h:102,
                 from ./Bidon.h:36,
                 from Bidon.cpp:37:
/usr/include/c++/4.4/c++0x_warning.h:31:2: error: #error This file requires compiler and library support for the upcoming ISO C++ standard, C++0x. This support is currently experimental, and must be enabled with the -std=c++0x or -std=gnu++0x compiler options.

I don't have the problem when compiling with CMake on Debian 6 (of course you need to install a recent CMake version) because it uses pkgconfig.
tango.pc file has this line when installed on Debian6:
Cflags: -std=c++0x -I${includedir}/tango

On Linux Mint 19 (with gcc 7.3) it is:
Cflags: -I${includedir}/tango

I think we could put something in place similar to what was done in this PR for cmake_tango.opt where we use the flags given by tango.pc file.
The current tango.opt file is already using pkg-config to get the tango LD_FLAGS, but not for the CXXFlags.
Moreover, it seems like it is not needed to use pkg-config to get the LDFLAGS needed for OmniORB and zmq since pkg-config --libs tango seems to already provide that information.

I personally think that it would also be useful to set default output dir to bin instead of $(HOME)/DeviceServers for device servers compilations in tango.opt because I find it very confusing for newcomers (and experimented users too actually).

Copy link

Choose a reason for hiding this comment

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

Where did you get those .pc files?

The flag in source distribution is set according to the compiler version used to compile Tango itself. Device server may be compiled later with some other version.

Maybe it will be enough to add -std=c++0x in tango.opt if GCC version is <5 (6?) or >4.3?

I personally think that it would also be useful to set default output dir to bin instead of $(HOME)/DeviceServers for device servers compilations in tango.opt because I find it very confusing for newcomers (and experimented users too actually).

I think it's a good idea. Maybe some install target which will put the server in ~/.local/bin or /usr/local/bin will be also useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Where did you get those .pc files?

I got the one from TangoSourceDistribution

The flag in source distribution is set according to the compiler version used to compile Tango itself. Device server may be compiled later with some other version.

But then the user would need to update PKG_CONFIG_PATH to take into account the tango.pc file from this other version, and the good flags will be used...

Maybe it will be enough to add -std=c++0x in tango.opt if GCC version is <5 (6?) or >4.3?

I personally think that it would also be useful to set default output dir to bin instead of $(HOME)/DeviceServers for device servers compilations in tango.opt because I find it very confusing for newcomers (and experimented users too actually).

I think it's a good idea. Maybe some install target which will put the server in ~/.local/bin or /usr/local/bin will be also useful.

Actually, there is already an install target (at least in the makefile version):
https://github.com/bourtemb/TangoSourceDistribution/blob/fix-34/assets/pogo/preferences/common_target.opt#L171

It installs the exe in $TANGO_HOME/bin

@bourtemb bourtemb merged commit 07ac614 into tango-controls:prepare-9.3.3 Jun 4, 2019
bourtemb added a commit that referenced this pull request Jun 5, 2019
* Update cpptango to release 9.3.3

* Update assets/README...
... to show changes between 9.2.5a and 9.3.3

* Update assets/TANGO_CHANGES for 9.3.3
Minimal text explanation in this version.

* Update TangoAccessControl to Release 2.14

* Update to latest available release versions
Update AtkPanel to      Release 5.8
Update JSSHTerminal to  Release 1.13
Update tango_admin to   Release 1.14

* Fix missing separator error in common_target.opt

Fix the following error (a tab was missing):
common_target.opt:166: *** missing separator.

* Update build.xml
Use copy ant task instead of deprecated copydir task

* Cleanup POGO (#20)

Remove obsolete POGO templates
Remove pogo-6

* Update astor to release 7.2.5

* rename atkpanel to ATKPanel in java applications (fixes #25) (#27)

* Pass correct arguments to TangoRestServer (fixes #24) (#26)

* Remove zmq.hpp (#23) (#28)

* Add logback configuration file (#21) (#29)

* Merge liblog4tango into libtango (change to convenience lib) (#30) (#31)

Change log4tango into a libtool convenience library.
liblog4tango.so is no longer generated, but log4tango objects are still compiled and are now linked into libtango.so

* Update README

* Remove my.cnf config file (tango-controls/TangoDatabase#16) (#35)

* Remove log4tango from tango.pc & makefiles; add dummy liblog4tango.so (#33)

* Add CMake opt files (#34) (#36)

Add cmake_common_target.opt and cmake_tango.opt to the distribution
in order to be able to generate CMakeLists.txt files for Device
Servers from Pogo.

* Compile with -std=c++0x when using g++ < 6.1

* Update to latest available release versions (#37)

Update to latest available release versions
Update Jive to          Release 7.22
Update ATK to           Release 9.3.6
Update Pogo to          Release 9.6.23
Update rest-server to   Release 1.14
Update TangoDatabase to Release 5.11
Update JTango to 9.5.14
@bourtemb bourtemb mentioned this pull request Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants