qt: Log Qt related info #17826

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:20191229-host-info changing 3 files +31 −0
  1. hebasto commented at 4:38 PM on December 29, 2019: member

    This PR adds some info to debug.log I found useful for testing (e.g., on Wayland) and debugging issues like #17153:

    $ ./src/qt/bitcoin-qt -printtoconsole | head -n 6
2020-01-04T14:57:40Z [main] Bitcoin Core version v0.19.99.0-0df287f4e (release build)
2020-01-04T14:57:40Z [main] InitParameterInteraction: parameter interaction: -externalip set -> setting -discover=0
2020-01-04T14:57:40Z [main] Qt 5.9.5 (dynamic), plugin=xcb (dynamic)
2020-01-04T14:57:40Z [main] System: Linux Mint 19.3, x86_64-little_endian-lp64
2020-01-04T14:57:40Z [main] Screen: HDMI-1 1600x1200, pixel ratio=1.0
2020-01-04T14:57:40Z [main] Assuming ancestors of block 00000000000000b7ab6ce61eb6d571003fbe5fe892da4c9b740c49a07542462d have valid signatures.
  2. fanquake added the label GUI on Dec 29, 2019
  3. emilengler commented at 7:12 PM on December 29, 2019: contributor

    -host is to confusing in my opinion. I think something like -debuginfo or -sysinfo would be better

  4. DrahtBot commented at 7:18 PM on December 29, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15768 (gui: Add close window shortcut by IPGlider)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  5. laanwj commented at 8:46 PM on January 2, 2020: member

    Concept ACK, but yes, -host is confusing. I would expect it to do something with P2P.

    -host is to confusing in my opinion. I think something like -debuginfo or -sysinfo would be better

    yes, or -guiinfo or -qtinfo, to make it clear this is GUI specific

  6. hebasto force-pushed on Jan 3, 2020
  7. hebasto renamed this:
    qt: Add -host command-line option
    qt: Add -qtinfo command-line option
    on Jan 3, 2020
  8. hebasto commented at 11:53 AM on January 3, 2020: member

    @emilengler @laanwj Thank you for your reviews. Your comments have been addressed. -qtinfo is chosen as the shortest string.

    The OP and the title have been updated. Added info about Qt version and link type.

  9. sipa commented at 12:33 PM on January 3, 2020: member

    Wouldn't it be better to add this information to -version? That's where I would expect it.

    EDIT: hmm, maybe not as it's not just Qt version information but also X stuff.

  10. hebasto commented at 12:38 PM on January 3, 2020: member

    @sipa

    Wouldn't it be better to add this information to -version? That's where I would expect it.

    -version also contains a huge piece of license info. -qtinfo is intended to supply some environment info, that seems a bit unappropriated for -version output, no?

  11. emilengler commented at 7:02 PM on January 3, 2020: contributor

    @sipa

    Wouldn't it be better to add this information to -version? That's where I would expect it.

    I think help2man would have issues with it and it would be weird on the GNU userland if it would contain information that is not the version and license stuff.

  12. hebasto commented at 8:41 PM on January 3, 2020: member

    I think help2man would have issues with it...

    help2man relies on gen-manpages.sh wrapper which in turn relies on bitcoind -version.

  13. sipa commented at 8:38 AM on January 4, 2020: member

    @hebasto You're right, the environment information does not belong in -version.

  14. hebasto commented at 9:51 AM on January 4, 2020: member

    @sipa

    @hebasto You're right, the environment information does not belong in -version.

    Does it mean "Concept ACK"? :smile:

  15. in src/qt/bitcoin.cpp:485 in 5ea7c911fc outdated 
     480 | @@ -480,6 +481,11 @@ int GuiMain(int argc, char* argv[])
 481 |          return EXIT_SUCCESS;
 482 |      }
 483 |  
 484 | +    if (gArgs.IsArgSet("-qtinfo")) {
 485 | +        tfm::format(std::cout, "%s", GUIUtil::QtInfo().toStdString());
    emilengler commented at 9:57 AM on January 4, 2020:

    I'm not 100% familiar with GUIUtil::QtInfo() but maybe it is better to print a new line after %s

  16. emilengler commented at 9:58 AM on January 4, 2020: contributor

    Concept ACK

  17. laanwj commented at 11:35 AM on January 4, 2020: member

    Wouldn't it be better to add this information to -version? That's where I would expect it.

    I personally prefer -version to be a short message that just states the version. And some license info. I think it's currently fine. I wouldn't expect all kind of window system information there.

    Thinking of it, this is for troubleshooting right? Let's just always log this in a message to the debug log instead of adding another step?

  18. sipa commented at 12:24 PM on January 4, 2020: member

    Concept ACK

  19. hebasto force-pushed on Jan 4, 2020
  20. hebasto commented at 2:59 PM on January 4, 2020: member

    @laanwj

    Thinking of it, this is for troubleshooting right?

    Yes, it is.

    Let's just always log this in a message to the debug log instead of adding another step?

    Done in the latest push.

    The OP has been updated.

  21. hebasto renamed this:
    qt: Add -qtinfo command-line option
    qt: Log Qt related info
    on Jan 4, 2020
  22. in src/qt/guiutil.cpp:890 in 0df287f4e4 outdated 
     883 | @@ -879,4 +884,22 @@ int TextWidth(const QFontMetrics& fm, const QString& text)
 884 |  #endif
 885 |  }
 886 |  
 887 | +void LogQtInfo()
 888 | +{
 889 | +#ifdef QT_STATIC
    emilengler commented at 3:32 PM on January 4, 2020:

    nit, do

    #if defined(QT_STATIC) || defined(QT_STATICPLUGIN)

    and remove the two conditions below

    promag commented at 4:34 PM on January 4, 2020:

    qt_link and plugin_link are orthogonal.

  23. in src/qt/guiutil.cpp:895 in 0df287f4e4 outdated 
     889 | +#ifdef QT_STATIC
 890 | +    const std::string qt_link{"static"};
 891 | +#else
 892 | +    const std::string qt_link{"dynamic"};
 893 | +#endif
 894 | +#ifdef QT_STATICPLUGIN
    emilengler commented at 3:32 PM on January 4, 2020:

    ^^

  24. promag commented at 4:36 PM on January 4, 2020: member

    Code review ACK 0df287f4e4d4b8121e3879aedee925bf4b0ed1c8. I also prefer laanwj suggestion.

  25. laanwj commented at 9:02 PM on January 4, 2020: member

    FWIW it doesn't seem like it detects multiple screens that form one display area. output:

    2020-01-04T20:58:32Z Qt 5.9.5 (dynamic), plugin=xcb (dynamic)
2020-01-04T20:58:32Z System: Ubuntu 18.04.3 LTS, x86_64-little_endian-lp64
2020-01-04T20:58:32Z Screen: DVI-I-1 1920x1200, pixel ratio=1.0

    xrandr output:

    Screen 0: minimum 320 x 200, current 3520 x 1200, maximum 16384 x 16384
DVI-I-1 connected primary 1920x1200+1600+0 (normal left inverted right x axis y axis) 519mm x 324mm
DVI-I-2 connected 1600x900+0+0 (normal left inverted right x axis y axis) 442mm x 249mm
  26. hebasto force-pushed on Jan 5, 2020
  27. hebasto commented at 8:15 AM on January 5, 2020: member

    @laanwj

    FWIW it doesn't seem like it detects multiple screens that form one display area.

    Now it detects all the screens associated with the windowing system the application is connected to. Please test.

  28. in src/qt/guiutil.cpp:903 in 092d146da6 outdated 
     898 | +    const std::string plugin_link{"dynamic"};
 899 | +#endif
 900 | +    LogPrintf("Qt %s (%s), plugin=%s (%s)\n", qVersion(), qt_link, QGuiApplication::platformName().toStdString(), plugin_link);
 901 | +    LogPrintf("System: %s, %s\n", QSysInfo::prettyProductName().toStdString(), QSysInfo::buildAbi().toStdString());
 902 | +    const QList<QScreen*> screens = QGuiApplication::screens();
 903 | +    for (int i = 0; i < screens.size(); ++i) {
    promag commented at 9:19 PM on January 5, 2020:

    for (const QScreen* s : QGuiApplication::screens())?

    hebasto commented at 9:33 PM on January 5, 2020:

    @promag Are these concerns relevant https://stackoverflow.com/questions/35811053/using-c11-range-based-for-loop-correctly-in-qt ?

    promag commented at 9:36 PM on January 5, 2020:

    It's fine here, these are pointers.

    hebasto commented at 9:41 PM on January 5, 2020:

    Implemented in the latest push.

  29. promag commented at 9:20 PM on January 5, 2020: member

    ACK 092d146da615de4722b6d171bdd2487969c374d5, now outputs all screens details.

  30. qt: Add LogQtInfo() function a004673c54
  31. hebasto force-pushed on Jan 5, 2020
  32. laanwj commented at 1:22 PM on January 8, 2020: member

    Thanks, works now!

    2020-01-08T13:20:54Z Qt 5.9.5 (dynamic), plugin=xcb (dynamic)
2020-01-08T13:20:54Z System: Ubuntu 18.04.3 LTS, x86_64-little_endian-lp64
2020-01-08T13:20:54Z Screen: DVI-I-1 1920x1200, pixel ratio=1.0
2020-01-08T13:20:54Z Screen: DVI-I-2 1600x900, pixel ratio=1.0

    ACK a004673c54d4c015775e0baced21f3fa961bf754

  33. laanwj referenced this in commit 07efb3fe2b on Jan 8, 2020
  34. laanwj merged this on Jan 8, 2020
  35. laanwj closed this on Jan 8, 2020
  36. hebasto deleted the branch on Jan 8, 2020
  37. MarkLTZ referenced this in commit fa0215b51f on Jan 12, 2020
  38. deadalnix referenced this in commit d575127e96 on Nov 14, 2020
  39. jonasschnelli referenced this in commit 7595183543 on Jan 28, 2021
  40. sidhujag referenced this in commit 7310454603 on Jan 28, 2021
  41. DrahtBot locked this on Feb 15, 2022
Contributors
hebastoemilenglerDrahtBotlaanwjsipapromag
Labels
GUI
Linked (view graph)
#17153 Regression: GUI screen resolution seems wrong
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-21 18:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me