scripted-diff: Avoid incompatibility with CMake AUTOUIC feature #25338

1. 
   hebasto commented at 4:14 pm on June 11, 2022: member

   Working on migration from Autotools to CMake build system, I found that our current code base needs to be adjusted.

   CMake allows to

   handle the Qt uic code generator automatically

   When using this feature, statements like #include "ui_<ui_base>.h" are processed in a special way.

   The node/ui_interface.h unintentionally breaks this feature. Of course, it is possible to provide a list of source files to be excluded from AUTOUIC. But, unfortunately, this approach does not work for the qt/sendcoinsdialog.cpp source file, where there are both https://github.com/bitcoin/bitcoin/blob/b71d37da2c8c8d2a9cef020731767a6929db54b4/src/qt/sendcoinsdialog.cpp#L10 and https://github.com/bitcoin/bitcoin/blob/b71d37da2c8c8d2a9cef020731767a6929db54b4/src/qt/sendcoinsdialog.cpp#L24

2. hebasto force-pushed on Jun 11, 2022
3. DrahtBot added the label Refactoring on Jun 11, 2022
4. 
   dongcarl commented at 8:04 pm on June 11, 2022: member

   Oh! What happens if we do set_property(SOURCE node/ui_interface.h PROPERTY SKIP_AUTOUIC ON)?
5. 
   hebasto commented at 8:43 pm on June 11, 2022: member

   @dongcarl

   Oh! What happens if we do set_property(SOURCE node/ui_interface.h PROPERTY SKIP_AUTOUIC ON)?

   It does not help:

    0[ 12%] Automatic MOC and UIC for target bitcoinqt
    1
    2AutoUic error
    3-------------
    4"SRC:/src/qt/sendcoinsdialog.cpp"
    5includes the uic file "node/ui_interface.h",
    6but the user interface file "interface.ui"
    7could not be found in the following directories
    8  "SRC:/src/qt/node"
    9  "SRC:/src/qt"
   10  "SRC:/src/qt/forms"
   11  "SRC:/src/qt/forms/node"
   12
   13gmake[3]: *** [src/qt/CMakeFiles/bitcoinqt_autogen.dir/build.make:71: src/qt/CMakeFiles/bitcoinqt_autogen] Error 1
   14gmake[2]: *** [CMakeFiles/Makefile2:754: src/qt/CMakeFiles/bitcoinqt_autogen.dir/all] Error 2
   15gmake[1]: *** [CMakeFiles/Makefile2:734: src/qt/CMakeFiles/bitcoin-qt.dir/rule] Error 2
   16gmake: *** [Makefile:358: bitcoin-qt] Error 2
   

6. 
   DrahtBot commented at 11:54 pm on June 11, 2022: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)
   • #23443 (p2p: Erlay support signaling by naumenkogs)

   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.

7. 
   hebasto commented at 6:27 pm on June 13, 2022: member

   Friendly ping @ryanofsky
8. 
   furszy commented at 7:09 pm on June 13, 2022: member

   Concept ACK
9. in src/Makefile.am:201 in 4f22e5e03d outdated

   197@@ -198,7 +198,7 @@ BITCOIN_CORE_H = \
   198   node/minisketchwrapper.h \
   199   node/psbt.h \
   200   node/transaction.h \
   201-  node/ui_interface.h \
   202+  node/uiinterface.h \
   

   ---

   ---

   

   Sjors commented at 8:12 am on June 14, 2022:

   interface_ui.h is a bit more readable

   ---

   

   hebasto commented at 8:40 am on June 14, 2022:

   Thanks! Updated.
10. scripted-diff: Avoid incompatibility with CMake AUTOUIC feature

    -BEGIN VERIFY SCRIPT-
    sed -i "s|node/ui_interface|node/interface_ui|g" $(git grep -l "node/ui_interface" ./src)
    git mv src/node/ui_interface.cpp src/node/interface_ui.cpp
    git mv src/node/ui_interface.h src/node/interface_ui.h
    sed -i "s|BITCOIN_NODE_UI_INTERFACE_H|BITCOIN_NODE_INTERFACE_UI_H|g" src/node/interface_ui.h
    -END VERIFY SCRIPT-

    018d70b587
11. hebasto force-pushed on Jun 14, 2022
12. 
    laanwj commented at 9:21 am on June 14, 2022: member

    I don’t know, ~0 on this, I dislike the idea of having to rename source files to make the build system happy.

    Oh! What happens if we do set_property(SOURCE node/ui_interface.h PROPERTY SKIP_AUTOUIC ON)?

    I would prefer a solution like that.

13. 
    MarcoFalke commented at 9:32 am on June 14, 2022: member

    cr ACK 018d70b58726b361b6951e0e6de04f13eb97a89d

    New name is fine and I reviewed the scripted diff.

    No opinion on the build system stuff.

14. 
    hebasto commented at 10:01 am on June 14, 2022: member

    @laanwj

    I don’t know, ~0 on this, I dislike the idea of having to rename source files to make the build system happy.

    Oh! What happens if we do set_property(SOURCE node/ui_interface.h PROPERTY SKIP_AUTOUIC ON)?

    I would prefer a solution like that.

    You are right. I also prefer some kind of build system switchers. But they do not work for qt/sendcoinsdialog.cpp source file, which should be processed, as it contains #include <qt/forms/ui_sendcoinsdialog.h> , and should be skipped, as it contains #include <node/ui_interface.h>. Looks like it is impossible without renaming.

15. 
    Sjors commented at 12:38 pm on June 14, 2022: member

    We already have build system code that special cases ui_<...>.h files (though afaik only in qt/forms), so the rename is probably a good idea anyway.
16. ryanofsky approved
17. 
    ryanofsky commented at 4:28 pm on June 14, 2022: member

    Code review ACK 018d70b58726b361b6951e0e6de04f13eb97a89d
18. furszy approved
19. 
    furszy commented at 7:24 pm on June 14, 2022: member

    Code review ACK 018d70b5

    Small scripted-diff.

20. MarcoFalke merged this on Jun 15, 2022
21. MarcoFalke closed this on Jun 15, 2022

    ---

22. hebasto deleted the branch on Jun 15, 2022
23. sidhujag referenced this in commit d7f9f35223 on Jun 15, 2022
24. DrahtBot locked this on Jun 15, 2023

Contributors
hebasto dongcarl DrahtBot furszy Sjors laanwj MarcoFalke ryanofsky

Labels
Refactoring