Skip to content

Conversation

@garethsb
Copy link
Contributor

@garethsb garethsb commented May 21, 2022

To start with, apply patch for non-constant SIGSTKSZ. See #259.

@garethsb
Copy link
Contributor Author

I think the Ubuntu 14.04 failure was an intermittent one.

This patch seems to solve the Catch compile issue on Ubuntu 22.04, but... I'm seeing the install test step fail... looks like an issue with bst::optional being std::optional when building my-nmos-node, but presumably being boost::optional when building the rest of nmos-cpp. That's odd, because C++17 is the default language standard with GCC 11.2, so I expected std::optional throughout.

@garethsb
Copy link
Contributor Author

garethsb commented May 21, 2022

Ah, the problem will be related to #206...

We currently have:

if(NOT DEFINED CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

This CMake file is included when building the library (and example apps in the same go) but not when building my-nmos-node. The nmos-cpp::compile-settings have this to specify the minimum requirements:

target_compile_features(compile-settings INTERFACE cxx_std_11)

Generally, maybe we should default to the compiler default (remove setting CMAKE_CXX_STANDARD completely), so that with GCC 11.2 it would build with C++17 unless overridden by the user. The reason we didn't is that the default for GCC 4 and GCC 5 is C++98, which ain't gonna work.

@garethsb garethsb changed the title Apply patch for non-constant SIGSTKSZ Ubuntu 22.04 May 21, 2022
@garethsb
Copy link
Contributor Author

The reason we didn't is that the default for GCC 4 and GCC 5 is C++98, which ain't gonna work.

Huh, so the Ubuntu 14.04 build has succeeded too. OK then...

@garethsb
Copy link
Contributor Author

garethsb commented May 21, 2022

Assuming the latest build-test is all green ✔️ I think this could be squash merged and resolves #251 and resolves #259.

@lo-simon lo-simon merged commit 1118aa4 into sony:master May 23, 2022
@garethsb garethsb deleted the patch-1 branch June 24, 2022 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants