test: Add missed header #729

1. 
   hebasto commented at 2:00 pm on May 16, 2023: member

   Should fix MSVC link errors like that:

   0addressbooktests.obj : error LNK2019: unresolved external symbol "void __cdecl ConfirmMessage(class QString *,class std::chrono::duration<__int64,struct std::ratio<1,1000> >)" (?ConfirmMessage@@YAXPEAVQString@@V?$duration@_JU?$ratio@$00$0DOI@@std@@@chrono@std@@@Z) referenced in function "void __cdecl `anonymous namespace'::EditAddressAndSubmit(class EditAddressDialog *,class QString const &,class QString const &,class QString)" (?EditAddressAndSubmit@?A0x2e52698e@@YAXPEAVEditAddressDialog@@AEBVQString@@1V3@@Z) [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin-qt\test_bitcoin-qt.vcxproj]
   1wallettests.obj : error LNK2001: unresolved external symbol "void __cdecl ConfirmMessage(class QString *,class std::chrono::duration<__int64,struct std::ratio<1,1000> >)" (?ConfirmMessage@@YAXPEAVQString@@V?$duration@_JU?$ratio@$00$0DOI@@std@@@chrono@std@@@Z) [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin-qt\test_bitcoin-qt.vcxproj]
   2C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\x64\Release\test_bitcoin-qt.exe : fatal error LNK1120: 1 unresolved externals [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin-qt\test_bitcoin-qt.vcxproj]
   

2. 
   DrahtBot commented at 2:00 pm on May 16, 2023: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	fanquake

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #650 (Add Import to Wallet GUI by KolbyML)

   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.

3. 
   hebasto commented at 2:04 pm on May 16, 2023: member

   cc @fanquake @sipsorcery
4. qt, test: Add missed header

   Should fix MSVC link errors.

   36e2d51b8f
5. hebasto force-pushed on May 16, 2023
6. DrahtBot added the label CI failed on May 16, 2023
7. fanquake approved
8. 
   fanquake commented at 4:30 pm on May 16, 2023: member

   ACK 36e2d51b8f6bb0125911c831ba2b6fd022fca708
9. DrahtBot removed the label CI failed on May 16, 2023
10. hebasto renamed this:
    qt, test: Add missed header
    test: Add missed header
    on May 17, 2023
11. hebasto merged this on May 17, 2023
12. hebasto closed this on May 17, 2023

    ---

13. hebasto deleted the branch on May 17, 2023
14. 
    maflcko commented at 8:59 am on May 17, 2023: contributor

    Any understanding why or when this happened?
15. 
    fanquake commented at 9:01 am on May 17, 2023: member

    I think it was https://github.com/bitcoin/bitcoin/pull/26715.
16. 
    maflcko commented at 9:06 am on May 17, 2023: contributor

    But CI passed on the pull and merge commit (https://github.com/bitcoin/bitcoin/runs/13481551706), no?
17. 
    hebasto commented at 9:10 am on May 17, 2023: member

    The MSVC link issue was kind of intermittent. I guess, it was dependent on the actual order of processing of the source files.
18. 
    maflcko commented at 9:12 am on May 17, 2023: contributor

    Ok, interesting. Maybe with iwyu this kind of bug will be fixed eventually :smiling_face_with_tear:
19. 
    fanquake commented at 9:13 am on May 17, 2023: member

    Yea. I guess the compiler is broken? Developers should not have to worry about putting files in the “right order” for it.
20. 
    hebasto commented at 12:01 pm on May 17, 2023: member

    Observing the same issue – https://cirrus-ci.com/task/6646912535756800

    :(

21. 
    maflcko commented at 12:04 pm on May 17, 2023: contributor

    Anything holding back https://github.com/bitcoin/bitcoin/pull/27571 ?
22. 
    fanquake commented at 12:06 pm on May 17, 2023: member

    No.
23. 
    maflcko commented at 1:27 pm on May 17, 2023: contributor

    Looks like iwyu doesn’t work with qt?

     0(qt/test/util.h has correct #includes/fwd-decls)
     1
     2qt/test/util.cpp should add these lines:
     3#include <QtCore/qobjectdefs.h>  // for FunctionPointer<>::IsPointerToMember...
     4#include <qapplication.h>        // for QApplication
     5#include <qlist.h>               // for QList
     6#include <qmessagebox.h>         // for QMessageBox
     7#include <qobject.h>             // for qobject_cast
     8#include <qpushbutton.h>         // for QPushButton
     9#include <qstring.h>             // for QString
    10#include <qtimer.h>              // for QTimer
    11#include <qwidget.h>             // for QWidget
    12#include <utility>               // for move
    13
    14qt/test/util.cpp should remove these lines:
    15- #include <QApplication>  // lines 9-9
    16- #include <QMessageBox>  // lines 10-10
    17- #include <QPushButton>  // lines 11-11
    18- #include <QString>  // lines 12-12
    19- #include <QTimer>  // lines 13-13
    20- #include <QWidget>  // lines 14-14
    

24. 
    fanquake commented at 1:29 pm on May 17, 2023: member

    Looks like iwyu doesn’t work with qt?

    We might have to provide a mapping file, for it to work sanely? i.e https://github.com/include-what-you-use/include-what-you-use/blob/master/qt5_11.imp.

25. sidhujag referenced this in commit 647fd0ec10 on May 17, 2023
26. 
    hebasto commented at 12:35 pm on May 21, 2023: member

    Looks like iwyu doesn’t work with qt?

    We might have to provide a mapping file, for it to work sanely? i.e include-what-you-use/include-what-you-use@master/qt5_11.imp.

    Added in https://github.com/bitcoin/bitcoin/pull/27710.

27. bitcoin-core locked this on May 20, 2024

Contributors
hebasto DrahtBot fanquake maflcko