Libtorrent: Wrong default C++ mode flag in libtorrent-rasterbar.pc

Created on 18 Aug 2020  路  21Comments  路  Source: arvidn/libtorrent

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

Most helpful comment

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

All 21 comments

@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.pc file, 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++11 to 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.

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:

  • do it that way (no -std= flag propagation at all)
  • go to some length to determine the compiler default in a different way than previously, and pass that as the -std= flag

Regarding 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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

master255 picture master255  路  10Comments

jbeich picture jbeich  路  5Comments

Lash78 picture Lash78  路  9Comments

pavel-pimenov picture pavel-pimenov  路  5Comments

aldenml picture aldenml  路  7Comments