transmission icon indicating copy to clipboard operation
transmission copied to clipboard

Fix building transmission with C++23

Open nevack opened this issue 1 year ago • 11 comments

nevack avatar May 07 '24 12:05 nevack

Is this exhaustive? I haven't tried, but translatable strings wrapped in _() probably needs fmt::runtime() too.

As I said, I haven't touched UI code in gtk/ and qt/

Only libtransmission and cli are updated

nevack avatar May 08 '24 07:05 nevack

There are a bunch of translatable strings in libtransmission too. For example:

https://github.com/transmission/transmission/blob/6c1cee5f79d8bbdc78f96640b52e49441d70247c/libtransmission/announcer-http.cc#L438-L443

tearfur avatar May 08 '24 08:05 tearfur

For example:

My bad, I don't have HAVE_GETTEXT, so #define _(a) (a).

We can merge this one as it is, and I will fix other issues in later PRs with the help of my Linux machine. Or I can convert this one to draft and incrementally fix building any other possible configurations. I do not have any preference on this.

nevack avatar May 08 '24 08:05 nevack

I'd say do it in one go, but you decide whatever works for you :)

tearfur avatar May 08 '24 11:05 tearfur

How are we guaranteeing that this won't break tomorrow, given that we're still on C++17?

mikedld avatar May 10 '24 15:05 mikedld

How are we guaranteeing that this won't break tomorrow, given that we're still on C++17?

I can add workflow for that, but it must be optional for now.

I convert this PR to draft and will add more fixes while thinking how to check this on CI.

nevack avatar May 14 '24 19:05 nevack

@tearfur You were right, I missed a lot of issues with fmt strings on the first iteration. Should be fixed now.

nevack avatar May 20 '24 18:05 nevack

Will work in this PR continue? This is a blocker for bumping to C++20.

tearfur avatar Oct 22 '24 02:10 tearfur

Will work in this PR continue? This is a blocker for bumping to C++20.

Yes, I address raised issues and will update this PR soon

nevack avatar Oct 22 '24 13:10 nevack

Will work in this PR continue? This is a blocker for bumping to C++20.

Yes, I address raised issues and will update this PR soon

Rebased, fixed conflicts and applied Mike's suggestions.

Did not test if this is still buildable with c++20/c++23 after months of work being merged.

We have a CI workflow to test this, so I will fix after it will report anything.

nevack avatar Oct 29 '24 22:10 nevack

Ready for review

nevack avatar Oct 30 '24 00:10 nevack

Manually rebased onto latest main to check, whether or not this is still buildable with C++23 action

nevack avatar Dec 29 '24 14:12 nevack

@mikedld can you re-review? You've still got this PR marked as Changes Requested :)

ckerr avatar Dec 30 '24 15:12 ckerr

Unfortunately merge conflicts arose again. Hope this is merged soon. 🥲

tearfur avatar Mar 05 '25 14:03 tearfur

Unfortunately merge conflicts arose again.

Resolved, again. Waiting for CI to find new compilation issues, if there are any.

nevack avatar Mar 05 '25 16:03 nevack

@tearfur Could you help me?

I've got build failure here after #6914:

Details
FAILED: libtransmission/CMakeFiles/transmission.dir/utils.cc.o 
/usr/bin/clang++ -DFMT_EXCEPTIONS=0 -DFMT_HEADER_ONLY=1 -DHAVE_COPY_FILE_RANGE -DHAVE_FALLOCATE64 -DHAVE_FLOCK -DHAVE_GETMNTENT -DHAVE_GETTEXT -DHAVE_LIBINTL_H -DHAVE_MKDTEMP -DHAVE_NGETTEXT -DHAVE_POSIX_FADVISE -DHAVE_POSIX_FALLOCATE -DHAVE_PREAD -DHAVE_PWRITE -DHAVE_SENDFILE64 -DHAVE_SO_REUSEPORT=1 -DHAVE_STATVFS -DHAVE_SYS_STATVFS_H -DPACKAGE_DATA_DIR=\"/home/runner/work/transmission/transmission/pfx/share\" -DPOSIX -DRAPIDJSON_HAS_STDSTRING=1 -DSMALL_DISABLE_EXCEPTIONS=1 -DWIDE_INTEGER_DISABLE_FLOAT_INTEROP -DWIDE_INTEGER_DISABLE_IOSTREAM -DWIDE_INTEGER_HAS_LIMB_TYPE_UINT64 -DWITH_INOTIFY -DWITH_OPENSSL -DWITH_UTP -D__TRANSMISSION__ -I/home/runner/work/transmission/transmission/src/libtransmission/.. -I/home/runner/work/transmission/transmission/obj/libtransmission/.. -I/home/runner/work/transmission/transmission/src/third-party/libutp/include -I/home/runner/work/transmission/transmission/src/third-party/libb64/include -I/home/runner/work/transmission/transmission/src/third-party/wildmat -isystem /home/runner/work/transmission/transmission/src/third-party/fast_float/include -isystem /home/runner/work/transmission/transmission/obj/third-party/dht.bld/pfx/include -isystem /home/runner/work/transmission/transmission/src/third-party/rapidjson/include -isystem /home/runner/work/transmission/transmission/src/third-party/utfcpp/source -isystem /home/runner/work/transmission/transmission/src/third-party/wide-integer -isystem /home/runner/work/transmission/transmission/src/third-party/fmt/include -isystem /home/runner/work/transmission/transmission/src/third-party/small/include -O2 -g -DNDEBUG -std=gnu++23 -W -Wall -Wextra -Wcast-align -Wextra-semi -Wextra-semi-stmt -Wextra-tokens -Wfloat-equal -Wgnu -Winit-self -Wint-in-bool-context -Wmissing-format-attribute -Wnull-dereference -Wpointer-arith -Wredundant-decls -Wredundant-move -Wreorder-ctor -Wreturn-std-move -Wself-assign -Wself-move -Wsemicolon-before-method-body -Wsentinel -Wshadow -Wsign-compare -Wsometimes-uninitialized -Wstring-conversion -Wsuggest-destructor-override -Wsuggest-override -Wuninitialized -Wunreachable-code -Wunused -Wunused-const-variable -Wunused-parameter -Wunused-result -Wwrite-strings -Wformat-security -MD -MT libtransmission/CMakeFiles/transmission.dir/utils.cc.o -MF libtransmission/CMakeFiles/transmission.dir/utils.cc.o.d -o libtransmission/CMakeFiles/transmission.dir/utils.cc.o -c /home/runner/work/transmission/transmission/src/libtransmission/utils.cc
In file included from /home/runner/work/transmission/transmission/src/libtransmission/utils.cc:10:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/chrono:49:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/shared_ptr.h:53:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/shared_ptr_base.h:59:
/usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:91:16: error: invalid application of 'sizeof' to an incomplete type '(anonymous namespace)::tr_net_init_impl::tr_net_init_mgr'
   91 |         static_assert(sizeof(_Tp)>0,
      |                       ^~~~~~~~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:398:4: note: in instantiation of member function 'std::default_delete<(anonymous namespace)::tr_net_init_impl::tr_net_init_mgr>::operator()' requested here
  398 |           get_deleter()(std::move(__ptr));
      |           ^
/home/runner/work/transmission/transmission/src/libtransmission/utils.cc:772:52: note: in instantiation of member function 'std::unique_ptr<(anonymous namespace)::tr_net_init_impl::tr_net_init_mgr>::~unique_ptr' requested here
  772 |     static inline std::unique_ptr<tr_net_init_mgr> instance;
      |                                                    ^
/home/runner/work/transmission/transmission/src/libtransmission/utils.cc:740:7: note: definition of '(anonymous namespace)::tr_net_init_impl::tr_net_init_mgr' is not complete until the closing '}'
  740 | class tr_net_init_mgr
      |       ^
1 error generated.

I can fix this by un-inlining it and defining outside, but I'm not sure this is the right way to do.

diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc
index 814f587b0..d1a631a67 100644
--- a/libtransmission/utils.cc
+++ b/libtransmission/utils.cc
@@ -769,8 +769,11 @@ public:
     }
 
 private:
-    static inline std::unique_ptr<tr_net_init_mgr> instance;
+    static std::unique_ptr<tr_net_init_mgr> instance;
 };
+
+std::unique_ptr<tr_net_init_mgr> tr_net_init_mgr::instance;
+
 } // namespace tr_net_init_impl
 } // namespace

UPD: Or maybe like this?

diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc
index 814f587b0..78a3d85a0 100644
--- a/libtransmission/utils.cc
+++ b/libtransmission/utils.cc
@@ -769,8 +769,11 @@ public:
     }
 
 private:
-    static inline std::unique_ptr<tr_net_init_mgr> instance;
+    static std::unique_ptr<tr_net_init_mgr> instance;
 };
+
+inline std::unique_ptr<tr_net_init_mgr> tr_net_init_mgr::instance = nullptr;
+
 } // namespace tr_net_init_impl
 } // namespace
 

nevack avatar Mar 05 '25 20:03 nevack

Test failure on Teamcity for OpenBSD/amd64 seems unrelated.

../../../tests/libtransmission/timer-test.cc:60
Expected: (actual) < (upper_bound), actual: 8-byte object <A5-00 00-00 00-00 00-00> vs 8-byte object <96-00 00-00 00-00 00-00>
actual:165 upper_bound:150
../../../tests/libtransmission/timer-test.cc:60
Expected: (actual) < (upper_bound), actual: 8-byte object <A5-00 00-00 00-00 00-00> vs 8-byte object <96-00 00-00 00-00 00-00>
actual:165 upper_bound:150

@ckerr Ready to merge, up to you now.

nevack avatar Mar 06 '25 13:03 nevack

@ckerr ping!

nevack avatar Mar 08 '25 23:03 nevack