bbc / bmx

Library and utilities to read and write broadcasting media files. Primarily supports the MXF file format
BSD 3-Clause "New" or "Revised" License
62 stars 17 forks source link

Add a compatibility fix for C++20 #69

Closed irieger closed 4 months ago

irieger commented 4 months ago

Add a compatibility fix for C++20 (and maybe others) to fix a non-supported conversion from std::string_view to std::string.

When preparing a recipe for the Conan package manager to properly add bmx as a dependency in other projects. One of the points there is to honor the user request for the C++ version requested by the user, thus I patch out the fixed setting of C++11. (See corresponding PR: https://github.com/conan-io/conan-center-index/pull/23955)

With C++20 set, I got the following error requiring an implicit conversion:

/home/ingmar/.conan2/p/b/bmx8699e87e1c8ea/b/src/src/common/Version.cpp: In function ‘mxfProductVersion bmx::get_bmx_mxf_product_version()’:
/home/ingmar/.conan2/p/b/bmx8699e87e1c8ea/b/src/src/common/Version.cpp:222:47: error: conversion from ‘const bmx_git::StringOrView’ {aka ‘const std::basic_string_view<char>’} to non-scalar type ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’} requested
  222 |         string describe = bmx_git::DescribeTag();
philipnbbc commented 4 months ago

Thanks for looking into this. I set CMAKE_CXX_STANDARD to 20 in the libMXF and libMXF++ libraries as well and found one more issue with a missing include - see below. If you could add that one as well please then it should all be good.

--- a/deps/libMXFpp/libMXF++/MXFVersion.cpp
+++ b/deps/libMXFpp/libMXF++/MXFVersion.cpp
@@ -33,6 +33,8 @@
 #include "config.h"
 #endif

+#include <string>
+
 #include "git.h"
 #include "fallback_git_version.h"
irieger commented 4 months ago

Thanks for looking into this. I set CMAKE_CXX_STANDARD to 20 in the libMXF and libMXF++ libraries as well and found one more issue with a missing include - see below. If you could add that one as well please then it should all be good.

--- a/deps/libMXFpp/libMXF++/MXFVersion.cpp
+++ b/deps/libMXFpp/libMXF++/MXFVersion.cpp
@@ -33,6 +33,8 @@
 #include "config.h"
 #endif

+#include <string>
+
 #include "git.h"
 #include "fallback_git_version.h"

Sure, just added it. And with your hint also changed our patches for Conan to comment out set(CMAKE_CXX_STANDARD 11) also for the deps and tried it with that patch.