Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set C++11 compile flag #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sriharshav
Copy link

No description provided.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Aug 26, 2018

Can you provide a bit of context? C++11 is not used in this package (yet, iirc), so that flag would seem to be unnecessary.

If you need it for some downstream package, I would suggest to set it there.

@sriharshav
Copy link
Author

I tried to compile it on aarch64 platform with Linux for Tegra

When I tried to compile Poco installation based on ppa build failed as Poco/Net/WebSocket.h was missing.

echo "deb http://ppa.launchpad.net/gezakovacs/poco/ubuntu xenial main" >> /etc/apt/sources.list.d/gezakovacs-poco.list

Then I compiled current stable version of Poco poco-1.9.0-release from source. This version requires C++11 std.

Thus when linking with Poco, without C++11 I envounter following error

In file included from /usr/include/c++/5/type_traits:35:0,
                 from /usr/local/include/Poco/Alignment.h:21,
                 from /usr/local/include/Poco/Foundation.h:111,
                 from /usr/local/include/Poco/XML/XML.h:23,
                 from /usr/local/include/Poco/SAX/InputSource.h:21,
                 from /media/nvidia/data/harsha/rws/abb_librws/src/rws_client.cpp:39:
/usr/include/c++/5/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
 #error This file requires compiler and library support \

And thus to link with Poco current stable release proposing to set standard to C++11. Also check POCO 2.0 goals

@gavanderhoorn
Copy link
Member

@sriharshav wrote:

I tried to compile it on aarch64 platform with Linux for Tegra

That is not a platform that we currently explicitly target with this library, so please be prepared to encounter some issues.

When I tried to compile Poco installation based on ppa build failed as Poco/Net/WebSocket.h was missing.

echo "deb http://ppa.launchpad.net/gezakovacs/poco/ubuntu xenial main" >> /etc/apt/sources.list.d/gezakovacs-poco.list

Which is not surprising, as that PPA only builds for amd64 and i386 (see here).

Then I compiled current stable version of Poco poco-1.9.0-release from source. This version requires C++11 std.

[..]

And thus to link with Poco current stable release proposing to set standard to C++11. Also check POCO 2.0 goals

We currently only test against Poco 1.7.x, so issues with newer Poco versions are expected.

If Poco requires C++11, it should set that and export that setting. I'd rather see whether the Poco CMake infrastructure exports something like that than hard-coding the flag in our build script.

Could you check whether that is the case?

@sriharshav
Copy link
Author

We currently only test against Poco 1.7.x, so issues with newer Poco versions are expected.

That wasn't evident from

POCO C++ Libraries (>= 1.4.3 due to WebSocket support)

And about Poco exporting C++11

If Poco requires C++11, it should set that and export that setting. I'd rather see whether the Poco
CMake infrastructure exports something like that than hard-coding the flag in our build script.

Could you check whether that is the case?

PocoConfig.cmake not exporting CMAKE_CXX_STANDARD though 1.8.x is last major non-c++11 release

Agree that hard-coding a flag to meet downstream requirements is not a good practice.

@gavanderhoorn
Copy link
Member

I'm not against fixing up our side of the CMakeLists.txt if it's needed but I'd like to make sure we also export this flag to downstream consumers.

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

Successfully merging this pull request may close these issues.

2 participants