WIP: Qt: add QML based mobile GUI #16883

icota · 2019-09-16T15:11:10Z

This adds a Qt Quick Controls 2 based UI meant for mobile devices. This is work-in-progress meant to gauge interest, related to but not dependent on #16110, i.e. one can run the "mobile" GUI on desktop as well. You should install qtdeclarative5-dev and qtquickcontrols2-5-dev in addition to usual Qt dependencies and configure with --enable-mobile-gui.

As a proof-of-concept this PR aims to be light, about 400 lines of C++ code that is meant to be a drop-in replacement for bitcoingui with mostly the same public functions and a couple of #ifdefs in bitcoin.cpp. There is also an addition of roleNames to the transactiontablemodel to make it compatible with QML stuff.

Replacement class bitcoinmobilegui hooks up to the exact same signals, models and controllers as bitcoingui. At the moment the mobile GUI presented to the user is dead simple (no multi-wallet, no advanced options) but it has been tested to allow the user to send and receive.

My rationale for making this is the fact that in a couple years phones will be powerful enough to run Core without much hassle. If we add some platform code for

verifying transactions only while in foreground or charging (to preserve battery)
enabling phone to phone transactions (NFC or bluetooth for enhanced privacy)
providing services to other local apps (think Lightning wallets, they could get the blockchain state from Core, no wasting of resources)

and release an official Android package I believe that this has the potential to put a full node in the pockets of millions - empowering users and improving the network.

DrahtBot · 2019-09-16T15:22:36Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

refactor: Apply override specifier consistently #18914 (refactor: Apply override specifier consistently by hebasto)
build: Make the help string for --with-gui configure option unambiguous #18361 (build: Make the help string for --with-gui configure option unambiguous by hebasto)
build: Fix Qt processing of configure script for depends with DEBUG=1 #18298 (build: Fix Qt processing of configure script for depends with DEBUG=1 by hebasto)
build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows #18297 (build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows by hebasto)
net: Add NAT-PMP port forwarding support #18077 (net: Add NAT-PMP port forwarding support by hebasto)
qt: Do not block GUI thread in RPCConsole #17659 (qt: Do not block GUI thread in RPCConsole by hebasto)

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.

BlockMechanic · 2019-09-17T14:30:15Z

Awesome, sorry could not make it home yesterday, fires still spreading! But iwill try tomorrow.

laanwj · 2019-09-18T08:05:39Z

👀
This looks very slick, and having a mobile interface for Bitcoin Core would be neat for projects such as ABCore.
@greenaddress something for you?

ryanofsky · 2019-09-18T14:04:45Z

Whilst I don't think this is something that we'd merge into this repository

Are you saying we wouldn't merge any QML UI into the repository, or that we wouldn't merge this change in its current draft form with ifdefs and the like?

IIUC, one nice thing that QML enables in addition to a mobile UI, is a remotely accessible web UI: https://www.qt.io/blog/2018/11/23/qt-quick-webgl-release-512

promag · 2019-09-18T14:17:43Z

AFAIK considering the current QML state of the art it wouldn't be accepted in core - because JS engine etc.

In Qt6, according to Lars Knoll in https://www.qt.io/blog/2019/08/07/technical-vision-qt-6:

Make JavaScript an optional feature of QML. Having a full JavaScript engine when using QML can complicate things and is an overhead especially when targeting low-end hardware such as microcontrollers. It is however extremely useful in many use cases.

IMHO it's still too early to consider QML since that there are some issues regarding long operations blocking the Qt event loop.

Prototype ACK

sipa · 2019-09-18T16:45:24Z

@promag Is there an actual JS interpreter used at runtime? Some quick googling tells me that with "Qt Quick" the JS is compiled to C++, which seems much less worrisome.

Issues around blocking UI are orthogonal, as they equally apply to the current UI.

This sounds really cool actually. Assuming it's feature complete, I wouldn't object to merging it in as an optional UI module in the repo.

sipa · 2019-09-18T17:34:32Z

When I build with --enable-mobile-gui, the produced bitcoin-qt binary shows an empty non-updating window. Is there anything else needed to enable this?

fanquake · 2019-09-19T00:19:13Z

Are you saying we wouldn't merge any QML UI into the repository, or that we wouldn't merge this change in its current draft form with ifdefs and the like?

What I meant was do we actually want to add another GUI to this repository? If we do, what are the plans / objectives for it?

Would it just exist as a bare-minimum, "here's an example of how you might build a QML based/mobile GUI"? Or would it get built out to add features similar to those mentioned in the PR description, and one day we'd be releasing a Bitcoin Core Android package? If it's the later, is this repository where we want to be doing all that development / having those discussions?

Assuming it's feature complete, I wouldn't object to merging it in as an optional UI module in the repo.

What should we consider feature complete here, basic send/receive functionality? I'm concerned that if it's optional, it wont get built / tested 99% of the time and will likely just break / rot.

promag · 2019-09-19T00:25:00Z

Previous discussions in:

luke-jr · 2019-09-19T00:55:59Z

Regardless of the below, General Concept ACK.

I think it would be best as a separate frontend/repo, though. No need to embed into the main repo just to be split out later...

My rationale for making this is the fact that in a couple years phones will be powerful enough to run Core without much hassle.

We're headed in the opposite direction (it's getting harder, not easier, since technology can't keep pace with the block size).

providing services to other local apps (think Lightning wallets, they could get the blockchain state from Core, no wasting of resources)

We already have that?

release an official Android package

Can this be done via gitian?

jonasschnelli · 2019-09-19T07:27:20Z

Great work!
Support for mobile devices is something we should not dismiss since those platforms (Android/iOS) are fairly popular.

I would not be opposed to merge something like this as an experimental feature – if the risks of breaking the current GUI is at minimum.

A web based JS/CSS frontend, as most desktop and some mobile apps use it today, is something a lot of contributors to this project dislike.

But AFAIK QML/JS compiles to native code...

I think we should follow this.
Eventually merge something like this.
If it will not be maintained, used, followed, remove it (as long as it is marked as experimental).

icota · 2019-09-19T10:23:27Z

What should we consider feature complete here, basic send/receive functionality?

My idea is to eventually have 90% of the features available to the QtWidgets GUI user, but hidden away behind a prompt of some kind so a normal user doesn't need to know or touch anything. I believe most of the users (especially on mobile) just want idiot-proof send/receive so don't want to overwhelm them. Perhaps this PR is the 'dumb' rather than 'mobile' GUI.

providing services to other local apps (think Lightning wallets, they could get the blockchain state from Core, no wasting of resources)

We already have that?

Correct me if I'm mistaken but doesn't RPC have loads of dangerous calls (wallet stuff, etc...)? Can one connect to Core RPC with just a subset for chain monitoring? I was thinking about a 'hardened RPC' for local apps, perhaps even permissioned.

When I build with --enable-mobile-gui, the produced bitcoin-qt binary shows an empty non-updating window. Is there anything else needed to enable this?

If you have qtdeclarative5-dev and qtquickcontrols2-5-dev installed I reckon that should be enough. Care to share your logs when this happens?

BlockMechanic · 2019-09-19T10:35:08Z

@icota see #16916 for the promised PR that contains my working android build. My command is :-

make HOST=i686-linux-android ANDROID_API_LEVEL=24 ANDROID_TARGET_ARCH=x86 ANDROID_TOOLCHAIN_BIN=/opt/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/bin ANDROID_SDK=/opt/android-sdk-linux ANDROID_NDK=/opt/android_ndk -j 4

promag · 2019-09-19T11:13:07Z

Is there anything else needed to enable this?

@sipa which Qt, etc?

luke-jr · 2019-09-19T12:05:03Z

Correct me if I'm mistaken but doesn't RPC have loads of dangerous calls (wallet stuff, etc...)? Can one connect to Core RPC with just a subset for chain monitoring?

RPC whitelisting is a thing now

promag

I think we could start by writing basic developer notes for qml code - component recommendation, how to name properties/vars, order , etc, where to write JS (inlined?), what is acceptable in QML, translations, etc.

In this PR I'd drop all unnecessary stuff, for instance everything related to multimedia.

BlockMechanic · 2019-09-22T14:40:08Z

Also go the depends system to behave, if anyone wants the mods to make it build via depends, let me know.

icota · 2019-09-22T16:19:26Z

I've rebased on top of master and added:

transaction dialog
about pane
settings pane
dark mode

I'll try to do weekly updates like this.

I think we could start by writing basic developer notes for qml code - component recommendation, how to name properties/vars, order , etc, where to write JS (inlined?), what is acceptable in QML, translations, etc.

Probably best to adopt the coding conventions off Qt's website? I've added a link to developer notes.

In this PR I'd drop all unnecessary stuff, for instance everything related to multimedia.

Multimedia is neccessary for camera QR code scanning, this is a placeholder right now but will be enabled as soon as I find out the least painful way to add an image processing dependency like qzxing. I'm also using bits of multimedia to invert the icons colour for dark mode.

icota · 2019-09-30T15:51:13Z

Another week another update. Added:

ability to change display unit
console pane 💻
clipboard handling
internationalisation (system locale used unless otherwise specified - bitcoinTr to reuse the current QtWidgets translations and the standard qsTr for new mobile-specific strings)

hebasto · 2019-10-06T19:22:31Z

Concept ACK.

hebasto · 2020-12-09T03:42:54Z

~~Kindly reminder to reviewers who is interested in QML -- please test and review #19882 :)~~

Bosch-0 · 2020-12-11T08:59:20Z

If a switch to QML is likely going to occur (which I think is the way to go going forward for reasons outlined above) in the future I think it would be good to start working on first principles, ground up, fresh UI designs for the GUI instead of bolting on changes to the current very restrictive widgets. Continuing to do this will just incur technical and design debt that we can't afford to take on.

To save resources I'd like to propose a design freeze of sorts on the current GUI widgets implementation and just focus on adding new protocol changes in a very low fidelity way whilst a complete new design is worked on and ready by the time a technical implementation of QML happens. Design foundations (such as clarity around branding and a design system) would also be established in parallel to designing this fresh new GUI.

We could start by creating a project to track these designs and have an issue posted and pinned to the GUI repo for "WIP: GUI re-designs" that can have a link to active Figma design work being conducted by community members such as myself. Any comments / suggestions / discussions should be posted on that issue, and not on Figma, to keep the discussions here and prevent any designer / developer divide.

Would this PR also not be more relevant in the GUI repo?

hebasto · 2020-12-11T09:01:49Z

Would this PR also not be more relevant in the GUI repo?

Build system changes belong to the main repo.

icota · 2020-12-11T09:39:58Z

To save resources I'd like to propose a design freeze of sorts on the current GUI widgets implementation and just focus on adding new protocol changes in a very low fidelity way whilst a complete new design is worked on and ready by the time a technical implementation of QML happens. Design foundations (such as clarity around branding and a design system) would also be established in parallel to designing this fresh new GUI.

We could start by creating a project to track these designs and have an issue posted and pinned to the GUI repo for "WIP: GUI re-designs" that can have a link to active Figma design work being conducted by community members such as myself. Any comments / suggestions / discussions should be posted on that issue, and not on Figma, to keep the discussions here and prevent any designer / developer divide.

@Bosch-0 I agree that this is the way forward. However, current widgets design freeze might not be wise because we don't know when the QML implementation will be production ready.

I'd like to propose the new design be adaptive to both mobile and desktop. But both of these discussion are for the GUI repo issue.

promag · 2020-12-14T09:34:12Z

@sipa

@promag Is there an actual JS interpreter used at runtime? Some quick googling tells me that with "Qt Quick" the JS is compiled to C++, which seems much less worrisome.

@sipa I think "compiled to C++" needs clarification, there's still runtime execution, see https://stackoverflow.com/a/41287809/1978589.

But in Qt 6, from https://www.qt.io/blog/2019/08/07/technical-vision-qt-6:

Make JavaScript an optional feature of QML. Having a full JavaScript engine when using QML can complicate things and is an overhead especially when targeting low-end hardware such as microcontrollers. It is however extremely useful in many use cases.

hebasto · 2020-12-14T09:45:40Z

@sipa

@promag Is there an actual JS interpreter used at runtime? Some quick googling tells me that with "Qt Quick" the JS is compiled to C++, which seems much less worrisome.

To compile QML source code into the final binaries the Qt Quick Compiler could be used.

promag · 2020-12-14T10:02:42Z

@hebasto

@sipa

@promag Is there an actual JS interpreter used at runtime? Some quick googling tells me that with "Qt Quick" the JS is compiled to C++, which seems much less worrisome.

To compile QML source code into the final binaries the Qt Quick Compiler could be used.

I was already assuming qtquickcompiler would be enabled, but there's still the JVM. See https://stackoverflow.com/a/62393490/1978589.

BlockMechanic · 2020-12-28T09:26:21Z

Has anyone built this using depends for windows and succeeded ? I Am running into an issue :-

2020-12-27T16:26:43Z GUI: qrc:/qml/main.qml:1:1: module "QtQuick" is not installed
2020-12-27T16:26:43Z GUI: qrc:/qml/main.qml:3:1: module "QtQuick.Controls.Material" is not installed
2020-12-27T16:26:43Z GUI: qrc:/qml/main.qml:4:1: module "QtQuick.Layouts" is not installed
2020-12-27T16:26:43Z GUI: qrc:/qml/main.qml:2:1: module "QtQuick.Controls" is not installed
2020-12-27T16:26:43Z GUI: QObject::connect: Cannot connect (null)::copyToClipboard(QString) to BitcoinMobileGUI::setClipboard(QString)
2020-12-27T16:26:43Z GUI: QObject::connect: Cannot connect (null)::changeUnit(int) to BitcoinMobileGUI::setDisplayUnit(int)

This is only on windows so far. Any ideas?

icota · 2020-12-28T10:51:37Z

@BlockMechanic Unless I missed something, depends doesn't include QtQuick (yet) so I don't think it matters which OS you're building on.

BlockMechanic · 2020-12-28T11:05:55Z

@BlockMechanic Unless I missed something, depends doesn't include QtQuick (yet) so I don't think it matters which OS you're building on.

I am using a modified depends based on the PR I made.

BlockMechanic · 2021-01-02T01:10:06Z

build-aux/m4/bitcoin_qt.m4

      CPPFLAGS="$QT_INCLUDES $CPPFLAGS"
    fi
  ])

  BITCOIN_QT_CHECK([AC_CHECK_HEADER([QtPlugin],,BITCOIN_QT_FAIL(QtCore headers missing))])
  BITCOIN_QT_CHECK([AC_CHECK_HEADER([QApplication],, BITCOIN_QT_FAIL(QtGui headers missing))])
  BITCOIN_QT_CHECK([AC_CHECK_HEADER([QLocalSocket],, BITCOIN_QT_FAIL(QtNetwork headers missing))])
+  BITCOIN_QT_CHECK([AC_CHECK_HEADER([QQmlApplicationEngine],, BITCOIN_QT_FAIL(QtQml headers missing))])
+


There are some missing parts to this file that enable static linking. If you compile the depends statically, there is an additional folder 'qml' which contains libs required for statically linking, such as libqtquick2plugin.a .

246774e depends: fix Qt precompiled headers bug (Igor Cota) 8e7ad41 depends: disable Qt Vulkan support on Android (Igor Cota) ba46ada CI: add Android APK build to cirrus (Igor Cota) 7563720 CI: add Android APK build script (Igor Cota) ebfb10c Qt: add Android packaging support (Igor Cota) Pull request description: ![bitcoin-qt](https://user-images.githubusercontent.com/762502/67396157-62f3d000-f5a7-11e9-8a6f-9425823fcd6c.gif) This PR is the third and final piece of the basic Android support puzzle - it depends on bitcoin/bitcoin#16110 and is related to bitcoin/bitcoin#16883. It introduces an `android` directory under `qt` and a simple way to build an Android package of `bitcoin-qt`: 1. Build depends for Android as described in the [README](https://github.com/bitcoin/bitcoin/blob/master/depends/README.md) 2. Configure with one of the resulting prefixes 3. Run `make && make apk` in `src/qt` The resulting APK files will be in `android/build/outputs/apk`. You can install them manually or with [adb](https://developer.android.com/studio/command-line/adb). One can also open the `android` directory in Android Studio for that integrated development and debugging experience. `BitcoinQtActivity` is your starting point. Under the hood makefile `apk` target: 1. Renames the `bitcoin-qt` binary to `libbitcoin-qt.so` and copies it over to a folder under `android/libs` depending on which prefix and corresponding [ABI](https://developer.android.com/ndk/guides/abis.html#sa) `bitcoin-qt` was built for 2. Takes `libc++_shared.so` from the Android NDK and puts in the same place. It [must be included](https://developer.android.com/ndk/guides/cpp-support) in the APK 3. Extracts Qt for Android Java support files from the `qtbase` archive in `depends/sources` to `android/src` There is also just a tiny bit of `ifdef`'d code to make the Qt Widgets menus usable. It's not pretty but it works and is a stepping stone towards bitcoin/bitcoin#16883. ACKs for top commit: MarcoFalke: cr ACK 246774e laanwj: Code review ACK 246774e Tree-SHA512: ba30a746576a167545223c35a51ae60bb0838818779fc152c210f5af1413961b2a6ab6af520ff92cbc8dcd5dcb663e81ca960f021218430c1f76397ed4cead6c

hebasto · 2021-06-13T21:43:04Z

All devs who are interested in the next-gen QML-based GUI for Bitcoin Core, please consider joining the dedicated https://github.com/bitcoin-core/gui-qml repo.

e2104f8 qml: Add stub window (Hennadii Stepanov) cc7f254 qml: Add bitcoin module (Hennadii Stepanov) 8efd330 refactor: Move qwidget and qml common code into the main() function (Hennadii Stepanov) Pull request description: Added basic support for QML to Qt resource system. Suggested the following file directory layout: - `src/qml/` - `src/qml/*.{h|cpp}` - `src/qml/pages/*.qml` - `src/qml/<other resource directories>` ACKs for top commit: promag: ACK e2104f8. Comparing to bitcoin/bitcoin#16883 I prefer this approach especially because we can change the startup without affecting the existing bitcoin-qt. Tree-SHA512: 60d0a519f76f66736c7a6665a0fc39488a2929f934ead6f704577de1337668dfb0931f176462258aad9b41498295cf9bb367d5ed83d4cf4623a563f2314a8989

1155978 build, qt: Do not install *.prl files (Hennadii Stepanov) 763793b build, qt: Fix wrong cross-compiling detection on macOS (Hennadii Stepanov) 3098272 build, qt: Force bootstrap while building linguist tools (Hennadii Stepanov) 689320e build, qt: Drop translations.pro hack (Hennadii Stepanov) 6a1f98f build, qt: Drop lrelease dependency patch (Hennadii Stepanov) 39e561e build, qt: Add linguist_tools list (Hennadii Stepanov) 27d3def build: Use Qt top-level build facilities (Hennadii Stepanov) Pull request description: This PR: - uses Qt top-level build facilities without the need to download all-in-one archive - is based on **BlockMechanic**'s [idea](bitcoin/bitcoin#20600), and is an alternative to #20600 - makes it easy to integrate [new modules](bitcoin/bitcoin#16883) into static builds - has the minimal diff - makes the qt package build process streamlined by dropping some patches and hacks (an alternative to #21420 and #20642) Fixes #18536 (a non-intrusive alternative to #21589 and #19785). Fixes #14648. Fixes #21588 (a non-intrusive alternative to #21591). Required for adding [Wayland support](bitcoin/bitcoin#19950) on Linux. --- **Note for reviewers**: With 9046de8a4cbc3899fed9eae084115f423e7ac5bd from #21995 it is easy to verify that there are no changes in the resulted `qt` package archive on the per commit basis. For example, for `HOST=i686-pc-linux-gnu` no commit in this PR introduces any changes. ACKs for top commit: fanquake: ACK 1155978 Tree-SHA512: 667b06b72cb7ff26d68b9b88e8dddb51084783ca9e3d80b3392710794c1dc7fd77bbcc3ccf4ccece9919d33b9bf8fce13c5059502bd228021dc7c93fdb87ca7a

1155978 build, qt: Do not install *.prl files (Hennadii Stepanov) 763793b build, qt: Fix wrong cross-compiling detection on macOS (Hennadii Stepanov) 3098272 build, qt: Force bootstrap while building linguist tools (Hennadii Stepanov) 689320e build, qt: Drop translations.pro hack (Hennadii Stepanov) 6a1f98f build, qt: Drop lrelease dependency patch (Hennadii Stepanov) 39e561e build, qt: Add linguist_tools list (Hennadii Stepanov) 27d3def build: Use Qt top-level build facilities (Hennadii Stepanov) Pull request description: This PR: - uses Qt top-level build facilities without the need to download all-in-one archive - is based on **BlockMechanic**'s [idea](bitcoin#20600), and is an alternative to bitcoin#20600 - makes it easy to integrate [new modules](bitcoin#16883) into static builds - has the minimal diff - makes the qt package build process streamlined by dropping some patches and hacks (an alternative to bitcoin#21420 and bitcoin#20642) Fixes bitcoin#18536 (a non-intrusive alternative to bitcoin#21589 and bitcoin#19785). Fixes bitcoin#14648. Fixes bitcoin#21588 (a non-intrusive alternative to bitcoin#21591). Required for adding [Wayland support](bitcoin#19950) on Linux. --- **Note for reviewers**: With 9046de8 from bitcoin#21995 it is easy to verify that there are no changes in the resulted `qt` package archive on the per commit basis. For example, for `HOST=i686-pc-linux-gnu` no commit in this PR introduces any changes. ACKs for top commit: fanquake: ACK 1155978 Tree-SHA512: 667b06b72cb7ff26d68b9b88e8dddb51084783ca9e3d80b3392710794c1dc7fd77bbcc3ccf4ccece9919d33b9bf8fce13c5059502bd228021dc7c93fdb87ca7a

DrahtBot · 2021-12-15T11:22:22Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

maflcko · 2021-12-15T11:29:41Z

I guess this is now superseded by https://github.com/bitcoin-core/gui-qml?

hebasto · 2021-12-15T11:36:49Z

I guess this is now superseded by https://github.com/bitcoin-core/gui-qml?

Yeap 🐅

maflcko · 2021-12-15T11:37:52Z

Ok, closing for now.

… to all `$(package)_*_cmds` affbf58 build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564e) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin/bitcoin#22552. Another [bug](bitcoin/bitcoin#22719) in the depends build system has already become a problem at least two times in the past (bitcoin/bitcoin#16883 (comment) and bitcoin/bitcoin#24134). Each time the problem was solved with other means. The initial [solution](bitcoin/bitcoin#19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](bitcoin/bitcoin#24134). The bug is well described in bitcoin/bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin/bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58 Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b

…`$(package)_*_cmds` affbf58 build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564e) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin#22552. Another [bug](bitcoin#22719) in the depends build system has already become a problem at least two times in the past (bitcoin#16883 (comment) and bitcoin#24134). Each time the problem was solved with other means. The initial [solution](bitcoin#19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](bitcoin#24134). The bug is well described in bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58 Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b

fanquake added GUI Needs Conceptual Review labels Sep 16, 2019

Sjors mentioned this pull request Sep 19, 2019

WIP QT Android Build #16916

Closed

promag reviewed Sep 20, 2019

View reviewed changes

icota force-pushed the qt-mobile branch from 09ddc96 to 559e3c5 Compare September 22, 2019 16:04

Sjors mentioned this pull request Sep 23, 2019

QR Code scanner #9913

Closed

laanwj added Android Feature labels Sep 30, 2019

laanwj mentioned this pull request Oct 15, 2019

GUI event loop should be block free #17145

Open

icota mentioned this pull request Oct 23, 2019

Qt: Add Android packaging support #17227

Merged

DrahtBot added the Needs rebase label Oct 26, 2019

hebasto mentioned this pull request Dec 13, 2020

depends: Use Qt top-level build facilities #20641

Merged

BlockMechanic reviewed Jan 2, 2021

View reviewed changes

Bosch-0 mentioned this pull request May 20, 2021

Who is the GUI being designed for? -Continued Discussion- bitcoin-core/gui#45

Closed

michaelfolkson mentioned this pull request Jun 4, 2021

Document goals and limitations of this repo bitcoin-core/gui-qml#1

Closed

hebasto mentioned this pull request Jun 10, 2021

build: split depends Qt into native and target builds #22142

Closed

dongcarl mentioned this pull request Aug 16, 2021

depends: $(package)_*_env settings only apply to the first command #22719

Closed

maflcko closed this Dec 15, 2021

hebasto mentioned this pull request Feb 6, 2022

build: Make $(package)_*_env available to all $(package)_*_cmds #24279

Merged

bitcoin locked and limited conversation to collaborators Dec 15, 2022

WIP: Qt: add QML based mobile GUI #16883

WIP: Qt: add QML based mobile GUI #16883

Conversation

icota commented Sep 16, 2019

DrahtBot commented Sep 16, 2019 • edited Loading

Conflicts

BlockMechanic commented Sep 17, 2019

laanwj commented Sep 18, 2019

ryanofsky commented Sep 18, 2019

promag commented Sep 18, 2019

sipa commented Sep 18, 2019

sipa commented Sep 18, 2019

fanquake commented Sep 19, 2019

promag commented Sep 19, 2019

luke-jr commented Sep 19, 2019

jonasschnelli commented Sep 19, 2019

icota commented Sep 19, 2019

BlockMechanic commented Sep 19, 2019

promag commented Sep 19, 2019

luke-jr commented Sep 19, 2019

promag left a comment

Choose a reason for hiding this comment

BlockMechanic commented Sep 22, 2019

icota commented Sep 22, 2019 • edited Loading

icota commented Sep 30, 2019

hebasto commented Oct 6, 2019

hebasto commented Dec 9, 2020 • edited Loading

Bosch-0 commented Dec 11, 2020

hebasto commented Dec 11, 2020

icota commented Dec 11, 2020 • edited Loading

promag commented Dec 14, 2020

hebasto commented Dec 14, 2020

promag commented Dec 14, 2020

BlockMechanic commented Dec 28, 2020

icota commented Dec 28, 2020

BlockMechanic commented Dec 28, 2020

BlockMechanic Jan 2, 2021

Choose a reason for hiding this comment

hebasto commented Jun 13, 2021

DrahtBot commented Dec 15, 2021

maflcko commented Dec 15, 2021

hebasto commented Dec 15, 2021

maflcko commented Dec 15, 2021

DrahtBot commented Sep 16, 2019 •

edited

Loading

icota commented Sep 22, 2019 •

edited

Loading

hebasto commented Dec 9, 2020 •

edited

Loading

icota commented Dec 11, 2020 •

edited

Loading