Please provide the following information
libtorrent version (or branch):
RC_1_2
platform/architecture:
linux/x64
compiler and compiler version:
gcc version 10.1.0
cmake version 3.18.1
please describe what symptom you see, what you would expect to see instead and
how to reproduce it.
This bug is related to PR #4968.
Before that PR, cmake correctly detected the default C++ mode of the compiler (C++14) and generate the correct Cflags: -fexceptions -std=c++14 ... in libtorrent-rasterbar.pc (file generated by cmake) .
After PR #4968, it can be seen that libtorrent-rasterbar.pc contains the unexpected mode: Cflags: -fexceptions -std=c++11 ....
The build script used:
cd "libtorrent"
mkdir -p "_build" && cd "_build"
cmake \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX="/usr" \
-DCMAKE_INSTALL_LIBDIR="lib" \
-Dpython-bindings=ON \
-Dboost-python-module-name="python" \
-Ddeprecated-functions=OFF \
"../"
make
@FranciscoPombal do you want to address this?
It's possible this is part of the solution too: https://github.com/arvidn/libtorrent/pull/5010
perhaps cmake really likes to build targets with the oldest possible version, regardless of what the dependee is building with. It does seem wrong to propagate C++11 up though..
@arvidn
C++11 should only be propagated up as a _minimum_ requirement, so the compiler default of C++14 should have been respected. I wonder if there is something wrong with the generated CMake config files/pkg-config.
pkg-config only has the concept of propagating compiler flags, nothing fancy
@Chocobo1
Minimal example producing the expected results, using CMake config files instead of pkg-config:
cmake_minimum_required(VERSION 3.16 FATAL_ERROR) # Policies <= CMP0097 default to NEW
project(testProject LANGUAGES CXX)
find_package(LibtorrentRasterbar 1.2.9 CONFIG REQUIRED COMPONENTS torrent-rasterbar)
add_executable(test)
target_sources(test PRIVATE main.cpp)
target_link_libraries(test PRIVATE LibtorrentRasterbar::torrent-rasterbar)
On a compiler that defaults to C++11 or later (in my case, I used GCC 8 which defaults to C++14), CMake does not add any -std= flag to the compile command of the test executable. If the compiler defaults to an earlier standard (or if you force an older standard with -DCMAKE_CXX_STANDARD at configure-time), main.cpp will be compiled with -std=gnu++11.
The problem is with pkg-config. It has no notion of minimum requirement. So if libtorrent's pkg-config file is generated with -std=gnu++11 in the Cflags field, the imported libtorrent target when using pkg-config will force that flag, overriding the compiler default. I believe the solution is to tweak the way libtorrent generates its pkg-config: the -std flag passed to the Cflags should either be the compiler default or the value the user overrode with -DCMAKE_CXX_STANDARD when building libtorrent, as opposed to the minimum specified in target_compile_features().
but the issue is the generated libtorrent-rasterbar.pc file, right? there's a bunch of cmake code in the libtorrent repo to generate that file
but the issue is the generated
libtorrent-rasterbar.pcfile, right? there's a bunch of cmake code in the libtorrent repo to generate that file
Exactly, I'm looking into it now.
apparently the way it used to work was better (right @Chocobo1 ?) maybe stripping all -std= flags from package configs would do it
apparently the way it used to work was better (right @Chocobo1 ?) maybe stripping all
-std=flags from package configs would do it
The downside of stripping all -std= flags is that building a project with libtorrent might fail if the compiler default is too old.
The downside of stripping all -std= flags is that building a project with libtorrent might fail if the compiler default is too old.
I don't see how propagating -std=c++11 to dependees help that situation. that might as well break something else they link against
@arvidn
I don't see how propagating
-std=c++11to dependees help that situation. that might as well break something else they link against
The goal would be to propagate _C++11 or later, if that's the compiler default_. That's the previous behaviour. I don't see an easy way to do that with pkg-config though, especially since CMAKE_CXX_STANDARD_DEFAULT stopped being part of the public API and is buggy anyway. So yeah, I guess the best way would be to cut our losses and just strip the -std= flags in the pkg-config. The user can always refer to the documentation to find the libtorrent's minimum supported C++ version.
@arvidn @Chocobo1 https://github.com/arvidn/libtorrent/pull/5013
apparently the way it used to work was better (right @Chocobo1 ?)
yeah, at least I haven't faced any issues.
Is it the case that the change in https://github.com/arvidn/libtorrent/pull/4968 caused cmake to always build libtorrent c++11 mode, regardless of what the top level project, or the compiler default, is?
because if so, maybe that PR should be reverted until a better solution can be found
@arvidn
Is it the case that the change in #4968 caused cmake to always build libtorrent c++11 mode, regardless of what the top level project, or the compiler default, is?
No. With that PR, the CMake build of libtorrent iteself has the expected behaviour: libtorrent is built with a C++ standard value according to the following precedence rule:
-DCMAKE_CXX_STANDARD passed by the user, as long as it's >= C++11, otherwise C++11 > Compiler default, as long as it's >= C++11, otherwise C++11 > C++11
The problem is that the value written to the generated pkg-config is taken from the INTERFACE_COMPILE_FEATURES as-is, which is _always_ C++11. This becomes -std=gnu++11 in the Cflags: field of the generated pkg-config, which effectively forces it when using pkg-config to find libtorrent for another project, rather than enforce it as a minimum and respect defaults/top-level project settings if they are greater than that value. The CMake config files, on the other hand, have the expected behavior - so a top level project will be compiled with _at least_ C++11 when linking against libtorrent, not _forced to_ use C++11.
I see. but I get the impression, still, that with https://github.com/arvidn/libtorrent/pull/5013 , the -std= option propagated to pkg-config still has different behavior from before.
Part of me thinks that the more idiomatic use of cmake is probably better, because then, in a way, it become's CMake's problem to get it right (even if there are cases now that it doesn't get right).
@arvidn
I see. but I get the impression, still, that with #5013 , the -std= option propagated to pkg-config still has different behavior from before.
with #5013 as it is now, there is no propagation of -std= at all for pkg-config. It is simply not added to the Cflags: field of the pkg-config, no matter what*. What me and zeule were discussing is if it is better to:
-std= flag propagation at all)-std= flagRegarding the latter approach, here's a reference I found about how we could go about doing it, at least with GCC (it would be nice if it could be done in a more compiler-independent way, though, otherwise we're back where we started): https://stackoverflow.com/questions/44734397/which-c-standard-is-the-default-when-compiling-with-g
From https://github.com/arvidn/libtorrent/issues/5012#issuecomment-675706159 I took it you were leaning to the former approach, though.
*: this is in fact different from the previous behavior when using GCC. The thing is, the old method that worked with GCC was critically buggy with other compilers.
From #5012 (comment) I took it you were leaning to the former approach, though.
I am, yes. In a way I think it's just a limitation of the pkg-config system, you kind of have to assume that the compiler used for all packages are somewhat compatible. And I think there's very little gain in trying to be cleverer than that, for pkg-config.
@arvidn
I am, yes. In a way I think it's just a limitation of the pkg-config system, you kind of have to assume that the compiler used for all packages are somewhat compatible. And I think there's very little gain in trying to be cleverer than that, for pkg-config.
:+1: , https://github.com/arvidn/libtorrent/pull/5013 is ready then.
I can confirm #5013 work as expected.
Although I lean towards https://github.com/arvidn/libtorrent/pull/5013#issuecomment-675771066, but that is out of the scope of this issue.
Thanks everyone for the help.
Most helpful comment
but the issue is the generated
libtorrent-rasterbar.pcfile, right? there's a bunch of cmake code in the libtorrent repo to generate that file