Fix building transmission with C++23 #6832

nevack · 2024-05-07T12:45:15Z

No description provided.

tearfur

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

nevack · 2024-05-08T07:52:26Z

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

tearfur · 2024-05-08T08:24:20Z

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

transmission/libtransmission/announcer-http.cc

Lines 438 to 443 in 6c1cee5

    
           tr_logAddWarn( 
        
               fmt::format( 
        
                   _("Couldn't parse announce response: {error} ({error_code})"), 
        
                   fmt::arg("error", error.message()), 
        
                   fmt::arg("error_code", error.code())), 
        
               log_name);

nevack · 2024-05-08T08:53:58Z

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.

tearfur · 2024-05-08T11:20:33Z

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

mikedld · 2024-05-10T15:15:25Z

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

nevack · 2024-05-14T19:02:28Z

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 · 2024-05-20T18:07:17Z

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

tearfur

Nice that you have made the changes to qt and gtk too!

CMakeLists.txt

libtransmission/peer-mse.cc

qt/Application.h

Fixes build error with C++20/C++23 error: return type 'auto' of selected 'operator==' function for rewritten '!=' comparison is not 'bool' Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

Fixes building with C++20/C++23 error: no matching function for call to 'size' function 'size' with deduced return type cannot be used before it is defined Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fmt::format_string ctor is consteval with C++20 See fmtlib/fmt#2438 Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

mikedld

Overall looks quite intrusive. Since we moved away from FMT_STRING() (or whatever it was called) some time ago, which was intrusive as well, I wonder if there's an option in libfmt to differentiate automatically based on the type of argument being either char const* or char const[N]...

@ckerr, anything specific you had in mind when adding needs update label other than syncing the PR with current main to resolve conflicts?

mikedld · 2024-06-03T21:47:54Z

.github/workflows/actions.yml

@@ -990,3 +990,80 @@ jobs:
      working-directory: ./android
      run: |
        gradle build
+
+  ubuntu-24-04-from-tarball-cxx-23:


Nit: I think we have enough from-tarball configurations as is, since this one doesn't exactly need to prove that it builds specifically from a tarball we can speed feedback up by building from git clone instead to not block on tarball generation job.

nevack requested a review from tearfur May 7, 2024 12:45

tearfur reviewed May 8, 2024

View reviewed changes

tearfur approved these changes May 10, 2024

View reviewed changes

nevack marked this pull request as draft May 14, 2024 19:03

nevack changed the title ~~Fix building libtransmission/ and utils/ with C++20 and C++23~~ Fix building transmission with C++23 May 19, 2024

nevack force-pushed the nevack/fix-std20-std23-fmt branch 2 times, most recently from 7931ac3 to bb2feaa Compare May 19, 2024 23:19

nevack requested a review from tearfur May 20, 2024 18:06

tearfur suggested changes May 21, 2024

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

libtransmission/peer-mse.cc Outdated Show resolved Hide resolved

qt/Application.h Outdated Show resolved Hide resolved

nevack force-pushed the nevack/fix-std20-std23-fmt branch 4 times, most recently from 1028e5b to 1d9ac88 Compare May 22, 2024 20:52

nevack added 8 commits May 23, 2024 00:38

fix: operator== should return bool in tr_strbuf

51950f5

Fixes build error with C++20/C++23 error: return type 'auto' of selected 'operator==' function for rewritten '!=' comparison is not 'bool' Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: explicitly specify Blocklist::size() return type as size_t

5947448

Fixes building with C++20/C++23 error: no matching function for call to 'size' function 'size' with deduced return type cannot be used before it is defined Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: wrap runtime format strings with fmt::runtime

cd1188c

fmt::format_string ctor is consteval with C++20 See fmtlib/fmt#2438 Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: wrap remaining runtime format strings in library, daemon and cli

ac4a420

Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: wrap runtime format strings with fmt::runtime for GTK client

40457f1

Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: explicitly define Application dtor to fix unique ptr issues in qt

fa57649

Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: allow to override C and CXX standard via cmdline or env

17159a6

Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

fix: add job to check if transmission compiles with C++23

3676a8c

Signed-off-by: Dzmitry Neviadomski <nevack.d@gmail.com>

nevack force-pushed the nevack/fix-std20-std23-fmt branch from 1d9ac88 to 3676a8c Compare May 22, 2024 21:39

nevack marked this pull request as ready for review May 22, 2024 21:40

tearfur approved these changes May 24, 2024

View reviewed changes

ckerr added needs update The PR has needs to be updated by the submitter type:refactor A code change that neither fixes a bug nor adds a feature notes:none Should not be listed in release notes labels May 26, 2024

mikedld reviewed Jun 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix building transmission with C++23 #6832

Fix building transmission with C++23 #6832

nevack commented May 7, 2024 •

edited

tearfur left a comment

nevack commented May 8, 2024

tearfur commented May 8, 2024

nevack commented May 8, 2024

tearfur commented May 8, 2024

mikedld commented May 10, 2024

nevack commented May 14, 2024

nevack commented May 20, 2024

tearfur left a comment

mikedld left a comment •

edited

mikedld Jun 3, 2024

Fix building transmission with C++23 #6832

Are you sure you want to change the base?

Fix building transmission with C++23 #6832

Conversation

nevack commented May 7, 2024 • edited

tearfur left a comment

Choose a reason for hiding this comment

nevack commented May 8, 2024

tearfur commented May 8, 2024

nevack commented May 8, 2024

tearfur commented May 8, 2024

mikedld commented May 10, 2024

nevack commented May 14, 2024

nevack commented May 20, 2024

tearfur left a comment

Choose a reason for hiding this comment

mikedld left a comment • edited

Choose a reason for hiding this comment

mikedld Jun 3, 2024

Choose a reason for hiding this comment

nevack commented May 7, 2024 •

edited

mikedld left a comment •

edited